-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Catch any warning on warns with no arg passed #8677
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
Changes from all commits
8b2f837
6ae71a2
dbe66d9
24ad886
2414d23
dd8ad3f
3f414d7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
Reducing confusion from `pytest.warns(None)` by: | ||
|
||
- Allowing no arguments to be passed in order to catch any exception (no argument defaults to `Warning`). | ||
- Emit a deprecation warning if passed `None`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
import os | ||
import stat | ||
import sys | ||
import warnings | ||
from pathlib import Path | ||
from typing import Callable | ||
from typing import cast | ||
|
@@ -400,11 +401,13 @@ def test_on_rm_rf_error(self, tmp_path: Path) -> None: | |
assert fn.is_file() | ||
|
||
# ignored function | ||
with pytest.warns(None) as warninfo: | ||
exc_info4 = (None, PermissionError(), None) | ||
on_rm_rf_error(os.open, str(fn), exc_info4, start_path=tmp_path) | ||
assert fn.is_file() | ||
assert not [x.message for x in warninfo] | ||
with warnings.catch_warnings(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this test use a mix of warns/catch_warnings to begin with? imho there should be s single context manager which ensures no warning happens There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would be nice, but I am not sure if it is possible or how to do it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It's a weird test, but I think that Olga's edits are good - we'll remove/change it in 7.0 when the
IMO this is probably served with |
||
warnings.simplefilter("ignore") | ||
with pytest.warns(None) as warninfo: # type: ignore[call-overload] | ||
exc_info4 = (None, PermissionError(), None) | ||
on_rm_rf_error(os.open, str(fn), exc_info4, start_path=tmp_path) | ||
assert fn.is_file() | ||
assert not [x.message for x in warninfo] | ||
|
||
exc_info5 = (None, PermissionError(), None) | ||
on_rm_rf_error(os.unlink, str(fn), exc_info5, start_path=tmp_path) | ||
|
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.
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.
I had to add this again, as mypy was complaining that there is no overload of
warns
that allows for no arguments to be passed. The...
signifies the default argument that is passed in here.