Skip to content
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

Allow using warnings.warn() with a Warning #11959

Merged
merged 2 commits into from
Feb 16, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ Edison Gustavo Muenz
Edoardo Batini
Edson Tadeu M. Manoel
Eduardo Schettino
Eero Vaher
Eli Boyarski
Elizaveta Shashkova
Éloi Rivard
Expand Down
3 changes: 2 additions & 1 deletion changelog/10865.improvement.rst
Original file line number Diff line number Diff line change
@@ -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 <https://github.com/python/cpython/issues/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 <https://github.com/python/cpython/issues/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.
40 changes: 28 additions & 12 deletions src/_pytest/recwarn.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@
):
return

def found_str():
def found_str() -> str:
return pformat([record.message for record in self], indent=2)

try:
Expand All @@ -341,14 +341,30 @@
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:
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

Check warning on line 361 in src/_pytest/recwarn.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/recwarn.py#L361

Added line #L361 was not covered by tests
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(

Check warning on line 368 in src/_pytest/recwarn.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/recwarn.py#L368

Added line #L368 was not covered by tests
f"Warning must be str or Warning, got {msg!r} (type {type(msg).__name__})"
)
27 changes: 19 additions & 8 deletions testing/test_recwarn.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from typing import List
from typing import Optional
from typing import Type
from typing import Union
import warnings

import pytest
Expand Down Expand Up @@ -546,24 +547,34 @@ def test_it():
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):
Expand Down