From 4806b591fc5b6dea1ae7f98fa9cc583082eb09bd Mon Sep 17 00:00:00 2001 From: Volodymyr Kochetkov Date: Sun, 28 Jan 2024 14:19:41 +0200 Subject: [PATCH] feat: 10865 refactor code and tests --- AUTHORS | 1 + changelog/10865.bugfix.rst | 1 - changelog/10865.improvement.rst | 2 ++ src/_pytest/recwarn.py | 6 +++--- testing/python/test_warning_attributes.py | 17 ----------------- testing/test_recwarn.py | 14 ++++++++++++++ 6 files changed, 20 insertions(+), 21 deletions(-) delete mode 100644 changelog/10865.bugfix.rst create mode 100644 changelog/10865.improvement.rst delete mode 100644 testing/python/test_warning_attributes.py diff --git a/AUTHORS b/AUTHORS index 879097622..25159b8b0 100644 --- a/AUTHORS +++ b/AUTHORS @@ -416,6 +416,7 @@ Vivaan Verma Vlad Dragos Vlad Radziuk Vladyslav Rachek +Volodymyr Kochetkov Volodymyr Piskun Wei Lin Wil Cooley diff --git a/changelog/10865.bugfix.rst b/changelog/10865.bugfix.rst deleted file mode 100644 index 0d1f9a9ff..000000000 --- a/changelog/10865.bugfix.rst +++ /dev/null @@ -1 +0,0 @@ -Fix a TypeError in a warning arguments call muted by warnings filter. diff --git a/changelog/10865.improvement.rst b/changelog/10865.improvement.rst new file mode 100644 index 000000000..2c2856dfe --- /dev/null +++ b/changelog/10865.improvement.rst @@ -0,0 +1,2 @@ +: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. +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 2a34730aa..5d2a98b2c 100644 --- a/src/_pytest/recwarn.py +++ b/src/_pytest/recwarn.py @@ -329,11 +329,11 @@ class WarningsChecker(WarningsRecorder): module=w.__module__, source=w.source, ) - # Check warnings has valid argument type + # Check warnings has valid argument type (#10865). wrn: warnings.WarningMessage for wrn in self: if isinstance(wrn.message, Warning): - if not isinstance(wrn.message.args[0], str): + if not isinstance(msg := wrn.message.args[0], str): raise TypeError( - f"Warning message must be str, got {type(wrn.message.args[0])}" + f"Warning message must be str, got {msg!r} (type {type(msg).__name__})" ) diff --git a/testing/python/test_warning_attributes.py b/testing/python/test_warning_attributes.py deleted file mode 100644 index 8335685b4..000000000 --- a/testing/python/test_warning_attributes.py +++ /dev/null @@ -1,17 +0,0 @@ -from _pytest.pytester import Pytester - - -class TestWarningAttributes: - def test_raise_type_error(self, pytester: Pytester) -> None: - pytester.makepyfile( - """ - import pytest - import warnings - - def test_example_one(): - with pytest.warns(UserWarning): - warnings.warn(1) - """ - ) - result = pytester.runpytest() - result.stdout.fnmatch_lines(["*1 failed*"]) diff --git a/testing/test_recwarn.py b/testing/test_recwarn.py index 5045c781e..ca2dbcf5f 100644 --- a/testing/test_recwarn.py +++ b/testing/test_recwarn.py @@ -477,3 +477,17 @@ class TestWarns: with pytest.raises(ValueError, match="some exception"): warnings.warn("some warning", category=FutureWarning) raise ValueError("some exception") + + +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"): + with pytest.warns(UserWarning): + warnings.warn(1) # type: ignore + + # 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): + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", "test") + warnings.warn(1) # type: ignore