Bug in 8.0.0 pytest.warns handling

[maread99]

There seems to be a handling error in 8.0.0, when pytest.warns fails to match the message. Seems that the warning class itself is being called towards the end of the handling...

import pytest
import warnings

assert pytest.__version__ == "8.0.0"

class AWarning(UserWarning):
    
    def __init__(self, a, b):
        pass

a, b = 1, 2
with pytest.warns(AWarning, match="not gonna match"):
    warnings.warn(AWarning(a, b))

---------------------------------------------------------------------------
Failed                                    Traceback (most recent call last)
    [... skipping hidden 1 frame]

File ~\...\market_prices\.venv\lib\site-packages\_pytest\outcomes.py:187, in fail(reason, pytrace, msg)
    186 reason = _resolve_msg_to_reason("fail", reason, msg)
--> 187 raise Failed(msg=reason, pytrace=pytrace)

Failed: DID NOT WARN. No warnings of type (<class '__main__.AWarning'>,) matching the regex were emitted.
 Regex: not gonna match
 Emitted warnings: [AWarning(1, 2)].

During handling of the above exception, another exception occurred:

TypeError                                 Traceback (most recent call last)
Cell In[1], line 13
     11 a, b = 1, 2
     12 with pytest.warns(AWarning, match="not gonna match"):
---> 13     warnings.warn(AWarning(a, b))

File ~\...\market_prices\.venv\lib\site-packages\_pytest\recwarn.py:333, in WarningsChecker.__exit__(self, exc_type, exc_val, exc_tb)
    331 for w in self:
    332     if not self.matches(w):
--> 333         warnings.warn_explicit(
    334             str(w.message),
    335             w.message.__class__,  # type: ignore[arg-type]
    336             w.filename,
    337             w.lineno,
    338             module=w.__module__,
    339             source=w.source,
    340         )

TypeError: __init__() missing 1 required positional argument: 'b'

NB: only seems to happen if the Warning class takes more than one argument. For example, if arg b is stripped from all the above then the failure to match is handled as expected.

Also, worked fine in 7.4.4:

import pytest
import warnings

assert pytest.__version__ == "7.4.4"

class AWarning(UserWarning):
    
    def __init__(self, a, b):
        pass

a, b = 1, 2
with pytest.warns(AWarning, match="not gonna match"):
    warnings.warn(AWarning(a, b))

---------------------------------------------------------------------------
Failed                                    Traceback (most recent call last)
Cell In[1], line 13
     11 a, b = 1, 2
     12 with pytest.warns(AWarning, match="not gonna match"):
---> 13     warnings.warn(AWarning(a, b))

    [... skipping hidden 1 frame]

File ~\...\market_prices\.venv\lib\site-packages\_pytest\outcomes.py:198, in fail(reason, pytrace, msg)
    196 __tracebackhide__ = True
    197 reason = _resolve_msg_to_reason("fail", reason, msg)
--> 198 raise Failed(msg=reason, pytrace=pytrace)

Failed: DID NOT WARN. No warnings of type (<class '__main__.AWarning'>,) matching the regex were emitted.
 Regex: not gonna match
 Emitted warnings: [AWarning(1, 2)]

Thanks is advance for looking at this!

OS: 'Windows-10-10.0.22631-SP0'

[The-Compiler]

Also reproducible outside pytest:

>>> w = AWarning(1, 2)
>>> warnings.warn_explicit(message=str(w), category=w.__class__, filename="test.py", lineno=1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: AWarning.__init__() missing 1 required positional argument: 'b'

So while I can't find this documented explicitly anywhere, it looks like Python doesn't expect the signature of a warning's __init__ changing. Not really much pytest can do about that.

[maread99]

Thanks for the quick reply @The-Compiler!

looks like Python doesn't expect the signature of a warning's init changing. Not really much pytest can do about that.

Although pytest handled this fine prior to 8.0,0?

Isn't the following call merely an election on the part of pytest?

warnings.warn_explicit(message=str(w), category=w.__class__, filename="test.py", lineno=1)

I'm not clear on why the implication of Python providing for raising a warning in this way is that warning messages shouldn't be dynamic (which they obviously can be) (?).

Thanks again.

[The-Compiler]

Yes, the call was added in #10937 - but unless others disagree (@reaganjlee @Zac-HD thoughts?), I'd argue that the bug is in your code and not pytest.

[maread99]

So we're saying that the warn_explicit method's signature implies that you shouldn't do the following, which would be considered a bug...(?)

class CustomWarning(UserWarning):

    def __init__(self, a, b):
        self._msg = (
            "This warning message is customised to include reference to"
            f"parameters {a} and {b}"
        )

    def __str__(self):
        return self._msg

Like yourself, I can't find any documentation suggesting that this shouldn't be done...

[The-Compiler]

Indeed - looking at the call pytest does in isolation, I can't see anything that would be wrong with it. The failure happens inside Python's own warn_explicit implementation (or rather, the equivalent C implementation).

That being said, the warn_explicit docs say (emphasis mine):

message must be a string and category a subclass of Warning or message may be a Warning instance, in which case category will be ignored.

So I suppose pytest could still work around this by using message=w, category=None or somesuch instead?

[maread99]

Interesting.

I'm not sure of the purposes for why pytest now makes this call in 8.0. Although, I notice that you now re-raise captured warnings that don't match any passed regex, so I'm guessing that's it (?), in which case passing the instance to 'message' sounds reasonable?

I just find the change in behaviour unexpected.

Regardless of any action taken, or not, thank you for looking at this.

[Zac-HD]

So I suppose pytest could still work around this by using message=w, category=None or somesuch instead?

I think it's worth check that this would have the desired effect, and shipping it in our next patch if so.

I'm aware that re-emitting warnings can be a breaking change and think that missing fewer warnings is worth it, but if we can make a slight change to our use of the stdlib to break less downstream code that'd be nice.

[maread99]

Thank you to everyone who looked at this and arranged for the fix!