From 62c6f5368995a96c8beff5da1a8c5f1ea30364fc Mon Sep 17 00:00:00 2001 From: Volodymyr Kochetkov Date: Fri, 12 Jan 2024 08:57:36 +0200 Subject: [PATCH 1/6] feat: 10865 --- changelog/10865.bugfix.rst | 1 + src/_pytest/recwarn.py | 8 ++++++++ testing/python/test_warning_attributes.py | 17 +++++++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 changelog/10865.bugfix.rst create mode 100644 testing/python/test_warning_attributes.py diff --git a/changelog/10865.bugfix.rst b/changelog/10865.bugfix.rst new file mode 100644 index 00000000000..0d1f9a9ffcc --- /dev/null +++ b/changelog/10865.bugfix.rst @@ -0,0 +1 @@ +Fix a TypeError in a warning arguments call muted by warnings filter. diff --git a/src/_pytest/recwarn.py b/src/_pytest/recwarn.py index aa58d43b405..2a34730aad5 100644 --- a/src/_pytest/recwarn.py +++ b/src/_pytest/recwarn.py @@ -329,3 +329,11 @@ def found_str(): module=w.__module__, source=w.source, ) + # Check warnings has valid argument type + wrn: warnings.WarningMessage + for wrn in self: + if isinstance(wrn.message, Warning): + if not isinstance(wrn.message.args[0], str): + raise TypeError( + f"Warning message must be str, got {type(wrn.message.args[0])}" + ) diff --git a/testing/python/test_warning_attributes.py b/testing/python/test_warning_attributes.py new file mode 100644 index 00000000000..8335685b41b --- /dev/null +++ b/testing/python/test_warning_attributes.py @@ -0,0 +1,17 @@ +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*"]) From 4806b591fc5b6dea1ae7f98fa9cc583082eb09bd Mon Sep 17 00:00:00 2001 From: Volodymyr Kochetkov Date: Sun, 28 Jan 2024 14:19:41 +0200 Subject: [PATCH 2/6] 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 87909762263..25159b8b07f 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 0d1f9a9ffcc..00000000000 --- 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 00000000000..2c2856dfe96 --- /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 2a34730aad5..5d2a98b2c42 100644 --- a/src/_pytest/recwarn.py +++ b/src/_pytest/recwarn.py @@ -329,11 +329,11 @@ def found_str(): 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 8335685b41b..00000000000 --- 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 5045c781e1f..ca2dbcf5f2f 100644 --- a/testing/test_recwarn.py +++ b/testing/test_recwarn.py @@ -477,3 +477,17 @@ def test_catch_warning_within_raise(self) -> None: 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 From d314ea6c77bd87c0df6a8353d02932ffa8b968de Mon Sep 17 00:00:00 2001 From: Volodymyr Kochetkov Date: Sun, 28 Jan 2024 14:40:44 +0200 Subject: [PATCH 3/6] feat: 10865 add test skip for pypy --- testing/test_recwarn.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/testing/test_recwarn.py b/testing/test_recwarn.py index ca2dbcf5f2f..d3eb010e563 100644 --- a/testing/test_recwarn.py +++ b/testing/test_recwarn.py @@ -2,6 +2,7 @@ from typing import List from typing import Optional from typing import Type +import sys import warnings from _pytest.pytester import Pytester @@ -485,6 +486,12 @@ def test_raise_type_error_on_non_string_warning() -> None: with pytest.warns(UserWarning): warnings.warn(1) # type: ignore + +@pytest.mark.skipif( + hasattr(sys, "pypy_version_info"), + reason="Not for pypy", +) +def test_raise_type_error_on_non_string_warning_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): From 9c4ffb5207f2218c396c6a489a7b9d293fcf13f0 Mon Sep 17 00:00:00 2001 From: Volodymyr Kochetkov Date: Sun, 28 Jan 2024 15:41:10 +0200 Subject: [PATCH 4/6] feat: 10865 add test with valid warning --- testing/test_recwarn.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/testing/test_recwarn.py b/testing/test_recwarn.py index d3eb010e563..b910fbb90ff 100644 --- a/testing/test_recwarn.py +++ b/testing/test_recwarn.py @@ -487,6 +487,12 @@ def test_raise_type_error_on_non_string_warning() -> None: 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.skipif( hasattr(sys, "pypy_version_info"), reason="Not for pypy", From 48ded39b77d1a469263ee4bfd959b9167fa175bb Mon Sep 17 00:00:00 2001 From: Volodymyr Kochetkov Date: Mon, 5 Feb 2024 18:48:13 +0200 Subject: [PATCH 5/6] feat: 10865 fix v2 for codecov --- src/_pytest/recwarn.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/_pytest/recwarn.py b/src/_pytest/recwarn.py index 5d2a98b2c42..62df274bd37 100644 --- a/src/_pytest/recwarn.py +++ b/src/_pytest/recwarn.py @@ -332,8 +332,11 @@ def found_str(): # Check warnings has valid argument type (#10865). wrn: warnings.WarningMessage for wrn in self: - if isinstance(wrn.message, Warning): - if not isinstance(msg := wrn.message.args[0], str): - raise TypeError( - f"Warning message must be str, got {msg!r} (type {type(msg).__name__})" - ) + 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__})" + ) From 79f934ff08daba5e1bae2c9aecc393c3f947469e Mon Sep 17 00:00:00 2001 From: Volodymyr Kochetkov Date: Mon, 5 Feb 2024 19:02:40 +0200 Subject: [PATCH 6/6] feat: 10865 fix conflict --- testing/test_recwarn.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/test_recwarn.py b/testing/test_recwarn.py index b910fbb90ff..e269bd7ddc9 100644 --- a/testing/test_recwarn.py +++ b/testing/test_recwarn.py @@ -1,8 +1,8 @@ # mypy: allow-untyped-defs +import sys from typing import List from typing import Optional from typing import Type -import sys import warnings from _pytest.pytester import Pytester