closes: 10865 Fix muted exception

[whysage]

closes #10865

[Zac-HD]

Thanks for volunteering to take this on, @whysage! The implementation looks good; I've just suggested a tweak to the message and a test approach which I hope will get us full coverage of the diff.

[Zac-HD]

I think we could simplify this test by running it inline, something like:

import warnings

import pytest

def test_raise_type_error_on_non_string_warning():
    with pytest.raises(TypeError, match="Warning message must be str"):
        with pytest.warns(UserWarning):
            warnings.warn(1)

    # 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.simplefilter("always")
            warnings.warn(1)

[nicoddemus]

Agreed, also:

• Please put this test in test_recwarn.py instead of creating a new file.
• Add a -> None type annotation.
• Add a docstring like Check pytest.warns validates warning messages are strings (#10865).

[whysage]

thanks, updated

[nicoddemus]

Thanks @whysage!

Left a few comments as well. 👍

[nicoddemus]

Suggested change

	Fix a TypeError in a warning arguments call muted by warnings filter.
	: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.
	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.

However I'm not sure this should be a bugfix, I would argue this is more of an improvement.

[whysage]

thanks, updated

[nicoddemus]

Suggested change

	# Check warnings has valid argument type
	# Check warnings has valid argument type (#10865).

[whysage]

thanks, updated

[nicoddemus]

Agreed, also:

• Please put this test in test_recwarn.py instead of creating a new file.
• Add a -> None type annotation.
• Add a docstring like Check pytest.warns validates warning messages are strings (#10865).

[whysage]

Thanks for volunteering to take this on, @whysage! The implementation looks good; I've just suggested a tweak to the message and a test approach which I hope will get us full coverage of the diff.

HI!
Thanks for the review.
I updated the code and still no luck with coverage
line 336 here https://app.codecov.io/gh/pytest-dev/pytest/pull/11804
is marked as not covered, but it is definitely entered by the test
def test_raise_type_error_on_non_string_warning

Any ideas how to fix this?

[whysage]

Hi all!
Updated to fix codecov issue
please review
@nicoddemus
@Zac-HD

[Zac-HD]

Looks great! Thanks for bearing with us through the process @whysage, we really appreciate your contribution 🥰

Comments addressed.

[zanieb]

Thank you!