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

[CI] Test updates for central sanitizers #35385

Merged
merged 4 commits into from
May 7, 2024
Merged

Conversation

pvaneck
Copy link
Member

@pvaneck pvaneck commented Apr 26, 2024

This PR updates our Python testing framework to adopt the new TestProxy which enables numerous common sanitizers.

Since these new common sanitizers might be considered too aggressive in some cases, a new API was added to devtools_testutils to remove specific sanitizers: remove_batch_sanitizers. With this new method, SDK owners can opt out of certain sanitizers by passing in a list of sanitizer IDs to remove_batch_sanitizers in their conftest.py files. For example:

from devtools_testutils import remove_batch_sanitizers, test_proxy


@pytest.fixture(scope="session", autouse=True)
def add_sanitizers(test_proxy):
    ...
    #  Remove the following body key sanitizer: AZSDK3493: $..name
    remove_batch_sanitizers(["AZSDK3493"])

The list of sanitizers and their IDs can be viewed here. When a sanitizer is opted out of, it's important to be sure that the non-sanitized data is non-sensitive.

Other changes in this PR:

  • Removed pretty much all sanitizers from the devtools_testutils set_common_sanitizers method since these are applied by TestProxy itself.
  • Deprecated the generate_sas function since TestProxy sanitizers will handle sanitization of se, st, and sig URI query parameters. Tests and recordinings using this function should not break with this PR.
  • Update various package conftest.py files to remove certain sanitizers that cause excessive playback failures.
  • Fixed some recordings in some packages and updated the corresponding assets.json files.
  • AI/ML add_sanitizer methods were updated to be session-scoped improving, massively improving testing performance.
  • Some tests marked live-only until they are re-recorded.

@azure-sdk
Copy link
Collaborator

azure-sdk commented Apr 26, 2024

API change check

API changes are not detected in this pull request.

@pvaneck pvaneck mentioned this pull request Apr 26, 2024
@pvaneck pvaneck force-pushed the test-proxy-sanitizer-update branch 4 times, most recently from 9549512 to 9492e67 Compare May 3, 2024 20:56
Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
@pvaneck pvaneck force-pushed the test-proxy-sanitizer-update branch from b161508 to 4fd4360 Compare May 6, 2024 22:44
Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
@pvaneck pvaneck marked this pull request as ready for review May 7, 2024 02:02
Copy link
Member

@mccoyp mccoyp left a comment

Choose a reason for hiding this comment

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

Thank you so much for this work!

@pvaneck
Copy link
Member Author

pvaneck commented May 7, 2024

/check-enforcer override

@pvaneck pvaneck merged commit a61a8e2 into main May 7, 2024
305 of 324 checks passed
@pvaneck pvaneck deleted the test-proxy-sanitizer-update branch May 7, 2024 19:55
@pvaneck pvaneck mentioned this pull request May 7, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants