Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Record and Playback] Skip functionality in common recorder #4898

Merged
merged 2 commits into from
Aug 27, 2019

Conversation

HarshaNalluru
Copy link
Member

@HarshaNalluru HarshaNalluru commented Aug 26, 2019

Skip list to skip a test -

const skip = [	
  // Abort	
  "browsers/aborter/recording_should_abort_after_aborter_timeout.json"	
];

Managing the list of test names would mean that we should frequently update them whenever a test name changes or a new test is added.
The plan is to avoid the skip list becoming stale in the following way.

This PR attempts to provide a .skip() method with the recorder from @azure/test-utils-recorder that can be called from the it block to skip a test.

Let us take a moment to understand what happens with this change.

  • We no longer need to save/update the skip list
  • Test can be skipped by calling recorder.skip("browser"); or recorder.skip("node"); in the it block based on the environment we want the test to be skipped
  • In the record mode, the recording will be generated with the info right from when the recorder is invoked in beforeeach until the skip() method that is invoked in the it block.
    • We need to save(and commit) this recording for the playback mode since the recorder has no way to know that the test is being skipped until the skip() method is called.
      [ A small tradeoff for getting rid of skip list and still add the convenience of invoking the recorder in beforeeach ]

Reference - #4336

@HarshaNalluru HarshaNalluru added the Client This issue points to a problem in the data-plane of the library. label Aug 26, 2019
@HarshaNalluru HarshaNalluru self-assigned this Aug 26, 2019
@HarshaNalluru HarshaNalluru requested a review from sophiajt August 26, 2019 19:04
Copy link
Contributor

@sadasant sadasant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow the feedback and land this! 🌞

@HarshaNalluru HarshaNalluru requested a review from sophiajt August 26, 2019 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants