From 9454fc38d3636b79ee657d6cacf7477eb8acee52 Mon Sep 17 00:00:00 2001 From: whysage <67018871+whysage@users.noreply.github.com> Date: Thu, 8 Feb 2024 02:47:56 +0200 Subject: [PATCH] closes: 10865 Fix muted exception (#11804) * feat: 10865 * feat: 10865 refactor code and tests * feat: 10865 add test skip for pypy * feat: 10865 add test with valid warning * feat: 10865 fix v2 for codecov * feat: 10865 fix conflict --- AUTHORS | 1 + changelog/10865.improvement.rst | 2 ++ src/_pytest/recwarn.py | 11 +++++++++++ testing/test_recwarn.py | 27 +++++++++++++++++++++++++++ 4 files changed, 41 insertions(+) create mode 100644 changelog/10865.improvement.rst 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.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 aa58d43b4..62df274bd 100644 --- a/src/_pytest/recwarn.py +++ b/src/_pytest/recwarn.py @@ -329,3 +329,14 @@ 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__})" + ) diff --git a/testing/test_recwarn.py b/testing/test_recwarn.py index 5045c781e..e269bd7dd 100644 --- a/testing/test_recwarn.py +++ b/testing/test_recwarn.py @@ -1,4 +1,5 @@ # mypy: allow-untyped-defs +import sys from typing import List from typing import Optional from typing import Type @@ -477,3 +478,29 @@ 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 + + +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", +) +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): + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", "test") + warnings.warn(1) # type: ignore