-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[TestProxyMigration-TA] Rerecord all tests #33931
Conversation
API change check API changes are not detected in this pull request. |
Use the latest changes from unreleased azure-core-test
add unrelease tag in versioning file
@@ -1,47 +0,0 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were some of the tests not replaced but only removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are disabled tests record files
...on-records/TextAnalyticsAsyncClientTest.analyzePiiEntityRecognitionWithDomainFilters[1].json
Show resolved
Hide resolved
.clientId(clientId) | ||
.tenantId(tenantId) | ||
.build(); | ||
credential = new DefaultAzureCredentialBuilder().build(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: The configuration loading before this call is now ignored and can be removed
if (interceptorManager.isPlaybackMode()) { | ||
// since running in playback mode won't have the token credential, so skipping matching it. | ||
interceptorManager.addMatchers(Arrays.asList( | ||
new CustomMatcher().setExcludedHeaders(Arrays.asList("Authorization")))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Authorization
header should be redacted by default and this isn't needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in this case we wanted to set this as with Record we expect the header to be present but with playback we don't set any auth mode at all so are missing even the presence of the header.
|
Rerecord all tests in live mode with the new TestProxy.
(Phase 1) Steps to integrate with Test Proxy:
Use azure-core-test newer than 1.15.0. (If there are bug fixes you need you may need to depend on an unreleased version of the lib.)
Change the Base type of the respective SDK test class from TestBase to TestProxyTestBase
Run tests in record mode.
Validate recordings are as expected and do not contain secrets. Example updated recording
Check in the updated recordings to the main repo
Validate changes by running the Live/ci test pipeline