Skip to content

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

Merged
merged 7 commits into from
May 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ Nicholas Murphy
Niclas Olofsson
Nicolas Delaby
Nikolay Kondratyev
Olga Matoula
Oleg Pidsadnyi
Oleg Sushchenko
Oliver Bestwalter
Expand Down
4 changes: 4 additions & 0 deletions changelog/8645.improvement.rst
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`.
4 changes: 2 additions & 2 deletions doc/en/how-to/capture-warnings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -332,11 +332,11 @@ You can record raised warnings either using func:`pytest.warns` or with
the ``recwarn`` fixture.

To record with func:`pytest.warns` without asserting anything about the warnings,
pass ``None`` as the expected warning type:
pass no arguments as the expected warning type and it will default to a generic Warning:

.. code-block:: python

with pytest.warns(None) as record:
with pytest.warns() as record:
warnings.warn("user", UserWarning)
warnings.warn("runtime", RuntimeWarning)

Expand Down
6 changes: 6 additions & 0 deletions src/_pytest/deprecated.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,12 @@
"see https://docs.pytest.org/en/latest/deprecations.html"
"#py-path-local-arguments-for-hooks-replaced-with-pathlib-path",
)

WARNS_NONE_ARG = PytestDeprecationWarning(
"Passing None to catch any warning has been deprecated, pass no arguments instead:\n"
" Replace pytest.warns(None) by simply pytest.warns()."
)

# You want to make some `__init__` or function "private".
#
# def my_private_function(some, args):
Expand Down
10 changes: 6 additions & 4 deletions src/_pytest/recwarn.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

from _pytest.compat import final
from _pytest.deprecated import check_ispytest
from _pytest.deprecated import WARNS_NONE_ARG
from _pytest.fixtures import fixture
from _pytest.outcomes import fail

Expand Down Expand Up @@ -83,7 +84,7 @@ def deprecated_call(

@overload
def warns(
expected_warning: Optional[Union[Type[Warning], Tuple[Type[Warning], ...]]],
expected_warning: Union[Type[Warning], Tuple[Type[Warning], ...]] = ...,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
expected_warning: Union[Type[Warning], Tuple[Type[Warning], ...]] = ...,
expected_warning: Union[Type[Warning], Tuple[Type[Warning], ...]],

Copy link
Contributor Author

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.

*,
match: Optional[Union[str, Pattern[str]]] = ...,
) -> "WarningsChecker":
Expand All @@ -92,7 +93,7 @@ def warns(

@overload
def warns(
expected_warning: Optional[Union[Type[Warning], Tuple[Type[Warning], ...]]],
expected_warning: Union[Type[Warning], Tuple[Type[Warning], ...]],
func: Callable[..., T],
*args: Any,
**kwargs: Any,
Expand All @@ -101,7 +102,7 @@ def warns(


def warns(
expected_warning: Optional[Union[Type[Warning], Tuple[Type[Warning], ...]]],
expected_warning: Union[Type[Warning], Tuple[Type[Warning], ...]] = Warning,
*args: Any,
match: Optional[Union[str, Pattern[str]]] = None,
**kwargs: Any,
Expand Down Expand Up @@ -232,7 +233,7 @@ def __init__(
self,
expected_warning: Optional[
Union[Type[Warning], Tuple[Type[Warning], ...]]
] = None,
] = Warning,
match_expr: Optional[Union[str, Pattern[str]]] = None,
*,
_ispytest: bool = False,
Expand All @@ -242,6 +243,7 @@ def __init__(

msg = "exceptions must be derived from Warning, not %s"
if expected_warning is None:
warnings.warn(WARNS_NONE_ARG, stacklevel=4)
expected_warning_tup = None
elif isinstance(expected_warning, tuple):
for exc in expected_warning:
Expand Down
12 changes: 12 additions & 0 deletions testing/deprecated_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,3 +178,15 @@ def test_hookproxy_warnings_for_fspath(tmp_path, hooktype, request):
assert l1 < record.lineno < l2

hooks.pytest_ignore_collect(config=request.config, fspath=tmp_path)


def test_warns_none_is_deprecated():
with pytest.warns(
PytestDeprecationWarning,
match=re.escape(
"Passing None to catch any warning has been deprecated, pass no arguments instead:\n "
"Replace pytest.warns(None) by simply pytest.warns()."
),
):
with pytest.warns(None): # type: ignore[call-overload]
pass
14 changes: 13 additions & 1 deletion testing/test_recwarn.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,14 +298,26 @@ def test_record(self) -> None:
assert str(record[0].message) == "user"

def test_record_only(self) -> None:
with pytest.warns(None) as record:
with pytest.warns() as record:
warnings.warn("user", UserWarning)
warnings.warn("runtime", RuntimeWarning)

assert len(record) == 2
assert str(record[0].message) == "user"
assert str(record[1].message) == "runtime"

def test_record_only_none_deprecated_warn(self) -> None:
# This should become an error when WARNS_NONE_ARG is removed in Pytest 7.0
with warnings.catch_warnings():
warnings.simplefilter("ignore")
with pytest.warns(None) as record: # type: ignore[call-overload]
warnings.warn("user", UserWarning)
warnings.warn("runtime", RuntimeWarning)

assert len(record) == 2
assert str(record[0].message) == "user"
assert str(record[1].message) == "runtime"

def test_record_by_subclass(self) -> None:
with pytest.warns(Warning) as record:
warnings.warn("user", UserWarning)
Expand Down
13 changes: 8 additions & 5 deletions testing/test_tmpdir.py
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
Expand Down Expand Up @@ -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():
Copy link
Member

Choose a reason for hiding this comment

The 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
but if it shows tricky to spell different quickly its fine to make that a follow-up task

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.
As I understand we want to discourage folks from using None, but we are maintaining the functionality (and this test) only for backwards-compatibility. I would also be inclined to say that this test case could be removed(!), so maybe not super important to worry about it?

Copy link
Member

Choose a reason for hiding this comment

The 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?

It's a weird test, but I think that Olga's edits are good - we'll remove/change it in 7.0 when the warns(None) deprecation is turned into an error.

imho there should be s single context manager which ensures no warning happens
but if it shows tricky to spell different quickly its fine to make that a follow-up task

IMO this is probably served with with warnings.catch_warnings(): (in the rare times that -Werror isn't enough); and if not it's a separate issue rather than aditional work on this PR.

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)
Expand Down