Record and Playback - Refactoring and Improvements

[HarshaNalluru]

TO DO

Tier 1

• [Estimate: 1] [Priority: normal] Environment variable is not properly replaced [Record and Playback] Environment variable is not properly replaced #6539
• [Estimate: 2] [Priority: high] Bug - replacements in recordings [Recorder] Unknown test failure in playback from storage-queue for a new recording #7108
• [Estimate: 5] [Priority: high] Clean up the current code (Record and playback - Types #4230).
  • Coupling RecorderEnvironmentSetup with the record call [Recorder] Coupling RecorderEnvironmentSetup with the record call #7135
    • And fix the tests [Recorder] Followup of #7083 - Coupling RecorderEnvironmentSetup with the record call #7085
  • Remove the function bunch - setReplaceableVariables, setReplacements, skipQueryParams - [Recorder] Remove function bunch - setReplaceableVariables, setReplacements, skipQueryParams #7133
  • Separate browser src code and node src code to fix rollup warnings
• [Estimate: 4] [Priority: high] timeout doesn't work on browser tests (might be related to the abortSignal problem) [core-http] timeout isn't working on the browser #5747.
  • browser - live
  • browser record
  • browser playback
  • node record
  • ndoe playback
• [Estimate: 4] [Priority: normal] Only record the tests that have changed [Record and Playback] Only record the tests that have changed #5156.
  • Run playback first, and then record the tests with record mode.
  • [Estimate: 3] [Priority: normal] Optimizations on new recordings [Record and Playback] Optimizations on new recordings #4879.
  • [Estimate: 1] [Priority: normal] Ensure that no recordings have time-sensitive information stored. [Recorder] Ensure that no recordings have time-sensitive information stored #6415
• Plan for the tests depending on nock - [Example - app-config, @sadasant for more info]
  • Mostly updating the docs saying that the tests that are already leveraging nock modules and continue to do the same for mocking their tests, they have an option to not use the recorder in the tests that involve nock.
  • For instance, you can keep such tests under one describe block and not use recorder in that test-suite.
• Add tests on the fly (Feel free to add more ideas for testing here)
  • [Estimate: 3] [Priority: normal] Add tests (this should probably be done at the end of the updates) [Common Recorder] Add tests to the common recorder #5160.

Tier 2

• Come to a consensus on the skip
  • What should the recorder.skip() do?
  • What should the this.skip() do?
  • Should not wrap with if blocks IMO
• [Estimate: 2] [Priority: normal] Regenerating all the recordings at once fail because of the tests with this.skip() [Storage] Regenerating all the recordings at once fail because of the tests with this.skip()  #5697.
  • mocha-multi
• [Estimate: 2] [Priority: normal] Skipped tests should have no recordings [test-utils-recorder] beforeEach recording even when the test is skipped #6284.
• [Estimate: 1] [Priority: normal] Update the documentation (this should probably be done at the end of the updates). [Recorder] Update the documentation based on all of the EPIC changes #6296
• [Estimate: 1] [Priority: normal] Update the current libraries using the recorder to use the latest API and follow the guidelines. Re-record when necessary. Remove unnecessary files and code afterwards ([Recorder] Update the current libraries using the recorder to use the latest API #6300).
• Common set of utilities outside of this package? For getUniqueName, etc.
• Better logs for the browser in playback Record and Playback - Improvements to browser console.logs #4205.
  • Bugs with console logs in the browser - [Browser test] console.log weirdness #6475
• Testing the recorder library - [Recorder] Plan to test the recorder library #7145

Tier 3

• [Estimate: 3] [Priority: normal] Zip recordings and add recordings/ to the .gitignore [recorder] Zip recordings and add recordings/ to the .gitignore  #5034.
• [Storage] Increase playback coverage - Providing dummy sas token [Storage] Playback - Providing dummy sas token #4275.

Previously done tasks

Tasks that will make our lives easier [Total estimate: 12]

• [Estimate: 1] [Priority: high] A copy of a single recorded test with a new name, with an accordingly name recorded file, should work as is on play. [Recorder] A copy of a single recorded test with a new name, with an accordingly name recorded file, should work as is on play. #6414 Can't reproduce. Closed the issue.
  [Recorder] Ensure that no recordings have time-sensitive information stored #6415
• [Estimate: 1] [Priority: high] Throw helpful error when getUniqueName() is used with the same label more than once [Common Recorder] Throw helpful error when getUniqueName() is used with the same label more than once #5698. PR: [recorder] Improved getUniqueName #6345
• [Estimate: 1] [Priority: high] Source maps are not properly configured. Tests become really hard to debug. [Recorder] Ensure that source maps can be used to debug errors during record and playback #6298 PR: Separating nyc from the integration tests for Node #6430 (Not related to the recorder)*
• [Estimate: 3] [Priority: high] core-http: abortSignal causes browser tests to skip the rest of the playbacks. [Recorder] Tests with abortSignals cause the other playback tests to be skipped #6297
• [Estimate: 1] [Priority: normal] Follow script guidelines [Recorder] Follow script guidelines #6555 Not needed.

After talking with Jonathan and Jeff, seems most appropriate to fix the issues above before refactoring the current recorder. By the time we finish, we will know if Mike Harder has an update on whether we can move to a centralized recorder.

Other tasks with estimates and considerations

• [Estimate: 1] [Priority: normal] The recording script should clear older built files. [Recorder] Recording should clear previously built files and directories  #6295 (unrelated to the recorder)
• Finalize the behavior of the recording files:
  • [Estimate: 3] [Priority: low] Ensure that source maps can be used to debug errors on browser tests [Recorder] Ensure that source maps can be used to debug errors on browser tests #6429
• Filling out README with instructions [Record and Playback] Filling out README with instructions #4961
• Keyvault - recorder.ts [Common Recorder] Update the KeyVault packages to use the latest API #5157
  • Remove the uniqueString() in favour of recorder.getUniqueName() - [KeyVault] Make sure our repos follow the new recorder guidelines #4872
• Improve .skip() [Common Recorder] Improvements for .skip() #5246
• Import the final recorder into feature/storage for storage([CommonRecorder] Import common recorder in storage packages #5152) and keyvault in master
  • storage-blob
  • storage-file
  • storage-queue
  • Keyvault-keys
  • Keyvault-certificates
  • Keyvault-secrets - @azure/test-utils-recorder in keyvault-secrets package HarshaNalluru/azure-sdk-for-js#15
• Move the existing recorder into "sdk/test-utils/recorder" - Record and Playback - refactoring #4281
• Skip functionality - Record and playback - POC for skip functionality in common recorder #4336
• Export isPlaybackMode, isRecordMode functions
• Adding dummy secrets
• ENV variables should be consistent and coherent across the packages ACCOUNT_NAME is changed to STORAGE_ACCOUNT_NAME
• Logic to filter the secrets
• Make recorder more generic - Record and Playback - refactoring #4281 (comment)
• Use this new recorder package and verify the recorder mechanism in a new branch extending from feature/storage
• Alternative for query-string package [Storage] Alternative for query-string package #4161
• Mocha 6 breaking changes

[ramya-rao-a]

This is needed for #4291

[xirzec]

@HarshaNalluru is there still work to do from this issue?

[HarshaNalluru]

Closed irrelevant ones, kept just a couple of (low priority / good to have) issues open.

Bye bye, EPIC.