Recwarn: Fix module when re-emitting unmatched warnings #12898
Recwarn: Fix module when re-emitting unmatched warnings #12898reaganjlee wants to merge 4 commits into
Conversation
psf-chronographer
Bot
added
the
bot:chronographer:provided
for more information, see https://pre-commit.ci
nicoddemus
left a comment
nicoddemus
left a comment
There was a problem hiding this comment.
Thanks @reaganjlee, we appreciate the PR!
Left a comment, but other than that we need a test to ensure this fix does not regress in the future. 👍
| module = next( | ||
| k | ||
| for k, v in sys.modules.items() | ||
| if getattr(v, "__file__", None) == w.filename | ||
| ) |
There was a problem hiding this comment.
We should probably fallback to some string just in case something is very wrong and we cannot match the filename:
| module = next( | |
| k | |
| for k, v in sys.modules.items() | |
| if getattr(v, "__file__", None) == w.filename | |
| ) | |
| module = next( | |
| k | |
| for k, v in sys.modules.items() | |
| if getattr(v, "__file__", None) == w.filename, | |
| None | |
| ) |
I'm assuming None is a valid argument for module=.
| @@ -0,0 +1 @@ | |||
| Fix regression with :func:`pytest.warns` using wrong module when re-emitting unmatched warnings. | |||
|
|
||
| def test_ignore_warning_from_module_a(): | ||
| with pytest.raises(pytest.fail.Exception, match="DID NOT WARN"): | ||
| with pytest.warns(UserWarning, match="module A.") as outer: |
There was a problem hiding this comment.
The test is not really veryfing the new module attribute... if I go in the code and set module = None in warnings.warn_explicit, the test still passes, which makes it unsuitable for a regression test... can you improve the test to check the actual module attribute of the warnings?
|
Closing as this has not seen activity in awhile. Feel free to reopen if you want to work on this again. |
|
Yep, I made a few attempts but couldn't find a proper test that differentiated between the two. Happy if anyone else takes it up! |
2 participants
Fixes #11933
Credit to @Zac-HD for fix!