[Test Proxy] Add RecordedByProxy decorator and AzureRecordedTestCase#20138
Merged
mccoyp merged 12 commits intoAzure:mainfrom Aug 17, 2021
Merged
[Test Proxy] Add RecordedByProxy decorator and AzureRecordedTestCase#20138mccoyp merged 12 commits intoAzure:mainfrom
mccoyp merged 12 commits intoAzure:mainfrom
Conversation
scbedd
reviewed
Aug 10, 2021
scbedd
reviewed
Aug 10, 2021
| # TODO, get the test-proxy server a real SSL certificate. The issue here is that SSL Certificates are | ||
| # normally associated with a domain name. Need to talk to the //SSLAdmin folks (or someone else) and get | ||
| # a recommendation for how to get a valid SSL Cert for localhost | ||
| kwargs["connection_verify"] = False |
Member
There was a problem hiding this comment.
As discussed. I'll help figure out the trust methodology for the certificate. We should remove this before we merge.
scbedd
reviewed
Aug 10, 2021
tools/azure-sdk-tools/devtools_testutils/aio/proxy_testcase_async.py
Outdated
Show resolved
Hide resolved
scbedd
reviewed
Aug 10, 2021
| from azure_devtools.scenario_tests.utilities import trim_kwargs_from_test_function | ||
|
|
||
| # defaults | ||
| PROXY_URL = "http://localhost:5000" |
Member
There was a problem hiding this comment.
If we're ok with manual start of the proxy, we should probably assume SSL. I'll see if I can figure that out today. If I can, let's get those changes added to this PR and swap this to https.
scbedd
reviewed
Aug 10, 2021
scbedd
approved these changes
Aug 10, 2021
Member
Author
|
/check-enforcer reset |
Contributor
seankane-msft
left a comment
seankane-msft
left a comment
There was a problem hiding this comment.
LGTM, two small comments
Comment on lines
+138
to
+140
| # Adding this for new proxy testcase | ||
| if hasattr(test_class_instance, "scrubber"): | ||
| test_class_instance.scrubber.register_name_pair(resource_name, self.moniker) |
Contributor
There was a problem hiding this comment.
Nit: I would add a log as an else statement here because then the name pair will not be scrubbed from the recordings. With a log at least you are notifying the dev that the object has no scrubber object.
Comment on lines
+68
to
+72
| # Adding this for new proxy testcase | ||
| if hasattr(self.test_class_instance, "scrubber"): | ||
| self.test_class_instance.scrubber.register_name_pair( | ||
| self.real_values[key.lower()], scrubbed_value | ||
| ) |
Contributor
There was a problem hiding this comment.
same as above, might just break out into a small helper function
scbedd
reviewed
Aug 17, 2021
| return os.sep.join(path_components).replace("::", "").replace("\\", "/") | ||
|
|
||
|
|
||
| def get_current_sha(): |
Member
There was a problem hiding this comment.
nit:I believe we can clean this function out.
scbedd
approved these changes
Aug 17, 2021
iscai-msft
added a commit
that referenced
this pull request
Aug 23, 2021
…into header_tuples * 'main' of https://github.com/Azure/azure-sdk-for-python: (104 commits) [Key Vault] Add 7.3-preview support for administration (#20364) Fix Monitor opentelemetry exporter readme issues (#19038) More Renaming in query (#20303) Update CODEOWNERS (#20366) [ServiceBus] update migration guide with message count info (#20360) [rest] change text from a property to a method (#20290) Handle value types for results (#20358) Remove old unused update changelog script (#20357) bump node version (#20353) [AutoRelease] t2-web-2021-08-03-73015 (#20053) Fix query batch processing (#20345) Sync eng/common directory with azure-sdk-tools for PR 1912 (#20340) Increment version for schemaregistry releases (#20326) [SchemaRegistry] prepare avro for release (#20321) Update CHANGELOG.md (#20334) Update main.py (#20332) Removing C:\Git\azure-sdk-tools\eng\common\scripts\FilterPoliCheckResults.ps1 as it was no longer used (#20325) Suppress CredScan warning for test proxy devcert (#20324) [SchemaRegistry] update samples readme (#20323) [Test Proxy] Add RecordedByProxy decorator and AzureRecordedTestCase (#20138) ...
iscai-msft
pushed a commit
that referenced
this pull request
Sep 29, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merges in tools from the integrate-proxy branch so that SDKs can begin migrating tests to the test proxy.