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

TST: Added script to enforce usage of match argument for tm.assert_produces_warning #59173

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

chaarvii
Copy link

@chaarvii chaarvii commented Jul 2, 2024

Added a script called enforce_match_arg_in_assert_produces_warning.py which checks that all usages of tm.assert_produces_warning make use of the match argument. This is done to ensure that users are always displayed the correct warning message. Uses of tm.assert_produces_warning(None) have been excluded as no warning is produced in this case.

Furthermore, to ensure that the existing code doesn't fail the pre-commit stage, all files that currently do not use the match argument have been excluded.

Once the PR is merged in, we can open up another issue to shorten the list of "excluded" files.

Copy link
Member

@Aloqeely Aloqeely left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

scripts/enforce_match_arg_in_assert_produces_warning.py Outdated Show resolved Hide resolved
.pre-commit-config.yaml Show resolved Hide resolved
@chaarvii
Copy link
Author

chaarvii commented Jul 5, 2024

@Aloqeely Just fixed the issue with Sequence import. It works now with TYPE_CHECKING. Is there anything else left to be addressed? If not, is it good to merge?

@Aloqeely
Copy link
Member

Aloqeely commented Jul 5, 2024

LGTM. cc @mroeschke

@chaarvii
Copy link
Author

@mroeschke - are we going ahead with this PR? Any reason we’d like to hold it off for now? cc @Aloqeely

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TST: Disallow bare tm.assert_produces_warning
2 participants