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

RFC: provide warns with a behavior similar to raises #2708

Closed
massich opened this issue Aug 21, 2017 · 1 comment
Closed

RFC: provide warns with a behavior similar to raises #2708

massich opened this issue Aug 21, 2017 · 1 comment
Labels
plugin: warnings related to the warnings builtin plugin type: enhancement new feature or API change, should be merged into features branch

Comments

@massich
Copy link
Contributor

massich commented Aug 21, 2017

When dealing with the messages thrown by warnings (IMHO) it would great if we could use the same calls as for the raises case. As example, this would allow for:

    with warns(RuntimeWarning, message='my runtime warning'):
        warnings.warn("my runtime warning", RuntimeWarning)

    with warns(UserWarning, match=r'must be \d+$'):
        warnings.warn("value must be 42", UserWarning)

I have implemented such behavior by wrapping pytest.warns here in the following manner:

from pytest import warns as _warns
from contextlib import contextmanager
from re import compile

@contextmanager
def warns(expected_warning, message=None, match=None):
    with _warns(expected_warning) as record:
        yield

    wrn_msg = str(record[0].message)
    if message is not None:
        assert_msg = '"{}" is different from "{}"'.format(wrn_msg, message)
        assert wrn_msg == message, assert_msg
    elif match is not None:
        regex = compile(match)
        assert_msg = '"{}" pattern not found in "{}"'.format(match, wrn_msg)
        assert regex.search(wrn_msg) is not None, assert_msg
    else:
        pass

Do you think is a valuable feature? Do you think that a PR implementing such behavior is useful to anyone? I'm sure I'm not the first one to wander why warns has different signature than raises. Therefore I can only guess that is not that easy to integrate it to pytest. But If you think that PR is in order I'll be happy to do it.

@nicoddemus
Copy link
Member

Hi @massich, thanks for writing.

Do you think is a valuable feature?

I think so, this definitely feels like a small oversight. We should take care to reuse the overlapping functionality to avoid introducing bugs and to ensure from now on improvements to one reflect on the other.

One question which is not clear: pytest.raises can catch a single exception (of course, more wouldn't make sense), but pytest.warns can catch multiple warnings. How should pytest.warns behave when multiple warnings are captured by the context manager?

  1. Catch a single warning and tries to match using message or match keyword arguments; if multiple warnings are encountered, fail out.
  2. Catch multiple warnings, and tries to match message or match keyword arguments to any of the captured warnings. If any of them matches, succeed otherwise fail and show the list of captured warnings.

My initial gut feeling is 1) and probably fits the current style of comparing against the records object, but I also see that 2) can be more convenient.

But If you think that PR is in order I'll be happy to do it.

Sure, please go ahead.

@nicoddemus nicoddemus added plugin: warnings related to the warnings builtin plugin type: enhancement new feature or API change, should be merged into features branch labels Sep 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: warnings related to the warnings builtin plugin type: enhancement new feature or API change, should be merged into features branch
Projects
None yet
Development

No branches or pull requests

2 participants