[Unified Recorder] Allow passing undefined to sanitizers and other minor changes#19561
Conversation
HarshaNalluru
changed the title
| * A regex. Can be defined as a simple regex replace OR if groupForReplace is set, a subsitution operation. | ||
| */ | ||
| regex: string; | ||
| regex?: string; |
There was a problem hiding this comment.
Is it optional because it is not used in playback mode? if regex is optional, is value optional too?
There was a problem hiding this comment.
This is only meant for convenience while writing the tests. If regex is undefined, the sanitizer won't be added.
For value, typically it is hard-coded and is a non-null value.
If it turned out to be undefined, it is most likely that the user is doing something wrong/unexpected, hence I didn't think of allowing that.
Co-authored-by: Jeremy Meng <yumeng@microsoft.com>
….com/HarshaNalluru/azure-sdk-for-js into harshan/pass-undefined-to-santiizers
| }); | ||
|
|
||
| it.skip("ContinuationSanitizer", async () => { | ||
| // Skipping since the test is failing in the browser |
jeremymeng
left a comment
jeremymeng
left a comment
There was a problem hiding this comment.
Looks good to me! I left a couple minor comments
| replacers.map((replacer: unknown) => | ||
| this.addSanitizer({ | ||
| replacers.map((replacer: RegexSanitizer) => { | ||
| if (!replacer.regex) return; |
There was a problem hiding this comment.
should we throw if it's undefined in live/record mode?
There was a problem hiding this comment.
hmmm, good idea
There was a problem hiding this comment.
only record mode
| actualConnString: string | undefined, | ||
| fakeConnString: string | ||
| ): Promise<void> { | ||
| if (!actualConnString) return; |
There was a problem hiding this comment.
should we throw if it's undefined in live/record mode?
There was a problem hiding this comment.
done
only record mode**
| * If the `TEST_MODE` is not `"playback"`, `delay` is a wrapper for setTimeout that resolves a promise after t milliseconds. | ||
| * | ||
| * @param {number} milliseconds The number of milliseconds to be delayed. | ||
| * @returns {Promise<T>} Resolved promise |
2 participants
Fixes #19559
Fixes #19560
Changes
undefinedas keys in the sanitizer options so that devs don't have to add additional checks if a certain env variable exists in playback.delay