-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
pytest.warns checks for subclass relationship rather than class equality #2166
pytest.warns checks for subclass relationship rather than class equality #2166
Conversation
56528f0
to
c7ab6a2
Compare
c7ab6a2
to
6173120
Compare
Thanks @lesteve for the PR, appreciate it! I rebased your branch into the latest |
@@ -238,6 +238,16 @@ def test_record_only(self): | |||
assert str(record[0].message) == "user" | |||
assert str(record[1].message) == "runtime" | |||
|
|||
def test_record_by_subclass(self): |
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.
Could you please also add a test which checks for warning subclasses when the user passes more than one warning to pytest.warns
? For example:
class Warn1(UserWarning): pass
class Warn2(RuntimeWarning): pass
with pytest.warns((UserWarning, RuntimeWarning)):
warnings.warn("w1", Warn1)
warnings.warn("w2", Warn2)
assert len(record) == 2
...
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.
Done
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.
Thanks!
babbff4
to
fa418c9
Compare
Not at all, I think this is a really nice recent addition in GitHub that maintainers can push into PR branches! |
@@ -238,6 +238,16 @@ def test_record_only(self): | |||
assert str(record[0].message) == "user" | |||
assert str(record[1].message) == "runtime" | |||
|
|||
def test_record_by_subclass(self): |
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.
Thanks!
@@ -216,7 +216,8 @@ def __exit__(self, *exc_info): | |||
# only check if we're not currently handling an exception | |||
if all(a is None for a in exc_info): | |||
if self.expected_warning is not None: | |||
if not any(r.category in self.expected_warning for r in self): | |||
if not any(issubclass(r.category, exp_warning) for |
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.
lets use issubclass with the propper classinfo support here, no need for own loops, please check the docs
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.
Good point!
rather than class equality. This makes it more similar to pytest.raises.
ae4a5be
to
0229896
Compare
Thanks a lot @lesteve! |
Great, thanks a lot! |
This makes it more similar to pytest.raises. Fix #2151.