From dcf9da92be44e5347fdb559c6717c031daae2b25 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 16 Feb 2024 13:38:55 +0200 Subject: [PATCH 1/2] recwarn: minor style improvements In preparation for next commit. --- src/_pytest/recwarn.py | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/_pytest/recwarn.py b/src/_pytest/recwarn.py index ddb240eb2..5e1fea2a0 100644 --- a/src/_pytest/recwarn.py +++ b/src/_pytest/recwarn.py @@ -314,7 +314,7 @@ class WarningsChecker(WarningsRecorder): ): return - def found_str(): + def found_str() -> str: return pformat([record.message for record in self], indent=2) try: @@ -341,14 +341,19 @@ class WarningsChecker(WarningsRecorder): module=w.__module__, source=w.source, ) - # Check warnings has valid argument type (#10865). - wrn: warnings.WarningMessage - for wrn in self: - self._validate_message(wrn) - @staticmethod - def _validate_message(wrn: Any) -> None: - if not isinstance(msg := wrn.message.args[0], str): - raise TypeError( - f"Warning message must be str, got {msg!r} (type {type(msg).__name__})" - ) + # Currently in Python it is possible to pass other types than an + # `str` message when creating `Warning` instances, however this + # causes an exception when :func:`warnings.filterwarnings` is used + # to filter those warnings. See + # https://github.com/python/cpython/issues/103577 for a discussion. + # While this can be considered a bug in CPython, we put guards in + # pytest as the error message produced without this check in place + # is confusing (#10865). + for w in self: + msg = w.message.args[0] # type: ignore[union-attr] + if isinstance(msg, str): + continue + raise TypeError( + f"Warning message must be str, got {msg!r} (type {type(msg).__name__})" + ) From 0475b1c925d40930172d5e198861864905a2b15d Mon Sep 17 00:00:00 2001 From: Eero Vaher Date: Fri, 9 Feb 2024 21:13:41 +0100 Subject: [PATCH 2/2] Allow using `warnings.warn()` with Warnings Test: `warnings.warn()` expects that its first argument is a `str` or a `Warning`, but since 9454fc38d3636b79ee657d6cacf7477eb8acee52 `pytest.warns()` no longer allows `Warning` instances unless the first argument the `Warning` was initialized with is a `str`. Furthermore, if the `Warning` was created without arguments then `pytest.warns()` raises an unexpected `IndexError`. The new tests reveal the problem. Fix: `pytest.warns()` now allows using `warnings.warn()` with a `Warning` instance, as is required by Python, with one exception. If the warning used is a `UserWarning` that was created by passing it arguments and the first argument was not a `str` then `pytest.raises()` still considers that an error. This is because if an invalid type was used in `warnings.warn()` then Python creates a `UserWarning` anyways and it becomes impossible for `pytest` to figure out if that was done automatically or not. [ran: rebased on previous commit] --- AUTHORS | 1 + changelog/10865.improvement.rst | 3 ++- src/_pytest/recwarn.py | 15 +++++++++++++-- testing/test_recwarn.py | 27 +++++++++++++++++++-------- 4 files changed, 35 insertions(+), 11 deletions(-) diff --git a/AUTHORS b/AUTHORS index 25159b8b0..dce91c4fe 100644 --- a/AUTHORS +++ b/AUTHORS @@ -127,6 +127,7 @@ Edison Gustavo Muenz Edoardo Batini Edson Tadeu M. Manoel Eduardo Schettino +Eero Vaher Eli Boyarski Elizaveta Shashkova Éloi Rivard diff --git a/changelog/10865.improvement.rst b/changelog/10865.improvement.rst index 2c2856dfe..a5ced8e9a 100644 --- a/changelog/10865.improvement.rst +++ b/changelog/10865.improvement.rst @@ -1,2 +1,3 @@ -:func:`pytest.warns` now validates that warning object's ``message`` is of type `str` -- currently in Python it is possible to pass other types than `str` when creating `Warning` instances, however this causes an exception when :func:`warnings.filterwarnings` is used to filter those warnings. See `CPython #103577 `__ for a discussion. +:func:`pytest.warns` now validates that :func:`warnings.warn` was called with a `str` or a `Warning`. +Currently in Python it is possible to use other types, however this causes an exception when :func:`warnings.filterwarnings` is used to filter those warnings (see `CPython #103577 `__ for a discussion). While this can be considered a bug in CPython, we decided to put guards in pytest as the error message produced without this check in place is confusing. diff --git a/src/_pytest/recwarn.py b/src/_pytest/recwarn.py index 5e1fea2a0..9eced36ff 100644 --- a/src/_pytest/recwarn.py +++ b/src/_pytest/recwarn.py @@ -351,9 +351,20 @@ class WarningsChecker(WarningsRecorder): # pytest as the error message produced without this check in place # is confusing (#10865). for w in self: - msg = w.message.args[0] # type: ignore[union-attr] + if type(w.message) is not UserWarning: + # If the warning was of an incorrect type then `warnings.warn()` + # creates a UserWarning. Any other warning must have been specified + # explicitly. + continue + if not w.message.args: + # UserWarning() without arguments must have been specified explicitly. + continue + msg = w.message.args[0] if isinstance(msg, str): continue + # It's possible that UserWarning was explicitly specified, and + # its first argument was not a string. But that case can't be + # distinguished from an invalid type. raise TypeError( - f"Warning message must be str, got {msg!r} (type {type(msg).__name__})" + f"Warning must be str or Warning, got {msg!r} (type {type(msg).__name__})" ) diff --git a/testing/test_recwarn.py b/testing/test_recwarn.py index 1feb3e654..edd4f51b5 100644 --- a/testing/test_recwarn.py +++ b/testing/test_recwarn.py @@ -3,6 +3,7 @@ import sys from typing import List from typing import Optional from typing import Type +from typing import Union import warnings import pytest @@ -546,24 +547,34 @@ class TestWarns: result.assert_outcomes() -def test_raise_type_error_on_non_string_warning() -> None: - """Check pytest.warns validates warning messages are strings (#10865).""" - with pytest.raises(TypeError, match="Warning message must be str"): +def test_raise_type_error_on_invalid_warning() -> None: + """Check pytest.warns validates warning messages are strings (#10865) or + Warning instances (#11959).""" + with pytest.raises(TypeError, match="Warning must be str or Warning"): with pytest.warns(UserWarning): warnings.warn(1) # type: ignore -def test_no_raise_type_error_on_string_warning() -> None: - """Check pytest.warns validates warning messages are strings (#10865).""" - with pytest.warns(UserWarning): - warnings.warn("Warning") +@pytest.mark.parametrize( + "message", + [ + pytest.param("Warning", id="str"), + pytest.param(UserWarning(), id="UserWarning"), + pytest.param(Warning(), id="Warning"), + ], +) +def test_no_raise_type_error_on_valid_warning(message: Union[str, Warning]) -> None: + """Check pytest.warns validates warning messages are strings (#10865) or + Warning instances (#11959).""" + with pytest.warns(Warning): + warnings.warn(message) @pytest.mark.skipif( hasattr(sys, "pypy_version_info"), reason="Not for pypy", ) -def test_raise_type_error_on_non_string_warning_cpython() -> None: +def test_raise_type_error_on_invalid_warning_message_cpython() -> None: # Check that we get the same behavior with the stdlib, at least if filtering # (see https://github.com/python/cpython/issues/103577 for details) with pytest.raises(TypeError):