-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Refactor recwarn to use warnings.catch_warnings instead of custom code #2303
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
Conversation
Since we dropped 2.5, we can now use warnings.catch_warnings to do the "catch warnings" magic for us, simplifying the code a bit.
def test_record(self): | ||
with pytest.warns(UserWarning) as record: | ||
warnings.warn("user", UserWarning) | ||
|
||
assert len(record) == 1 | ||
assert str(record[0].message) == "user" | ||
|
||
print(repr(record[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.
This was testing that record
was a RecordedWarning
, which has been replaced by warnings
own tuple.
No longer test for implementation details of recwarn since warnings.catch_warnings has changed significantly in 3.6.
@@ -8,25 +8,19 @@ | |||
def test_recwarn_functional(testdir): | |||
reprec = testdir.inline_runsource(""" | |||
import warnings | |||
oldwarn = warnings.showwarning |
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.
This is test and test_recording
below were testing an implementation detail which is no longer true in 3.6, because catch_warnings
has changed significantly since 3.5
.
@@ -115,19 +114,14 @@ def warns(expected_warning, *args, **kwargs): | |||
return func(*args[1:], **kwargs) | |||
|
|||
|
|||
RecordedWarning = namedtuple('RecordedWarning', ( |
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.
isnt that a breach of api? is it exposed in 3.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.
No, that was added by you in features branch for 3.1, so it has never seen the light of day.
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.
the new behaviour is more consistent and should even match across all python versions
Since we dropped 2.5, we can now use warnings.catch_warnings to do the
"catch warnings" magic for us, simplifying the code a bit.
This is the refactoring I mentioned in #2298, which will ultimately be useful to finish #2072.