Pytest: Allow pytest.raises and pytest.warns to be used as decorators

[megabyde]

In addition to pytest.raises and pytest.warns as context managers, it would be very helpful to have them available as decorators since it greatly improves readability of tests:

from pytest import raises

# No redundant scope to catch an exception 
@raises(IndexError)
def test_raises_as_context_manager():
    answers = []
    answers[0] = 42

# Redundant scope introduce by the context manager
def test_raises_as_context_manager():
    answers = []
    with raises(IndexError):
        answers[0] = 42

For example, nose supports decorating tests for exception catching: https://nose.readthedocs.io/en/latest/testing_tools.html

[nicoddemus]

Hi @megabyde thanks for the suggestion.

I'm a little skeptic to be honest on how useful that would be, most code which uses pytest.raises in my experience need to do some setup first. Also IMHO this poses the risk that users will catch exceptions in places they didn't mean to, hiding bugs.

I did a quick look at pytest's own test, and indeed many of them could not make use of this feature. For example:

    def test_unicode_and_str_mixture(self):
        f = capture.CaptureIO()
        if sys.version_info >= (3, 0):
            f.write("\u00f6")
            pytest.raises(TypeError, "f.write(bytes('hello', 'UTF-8'))")
        else:
            f.write(text_type("\u00f6", "UTF-8"))
            f.write("hello")  # bytes
            s = f.getvalue()
            f.close()
            assert isinstance(s, text_type)

Changing it to:

    @pytest.raises(TypeError)
    def test_unicode_and_str_mixture(self):
        f = capture.CaptureIO()
        if sys.version_info >= (3, 0):
            f.write("\u00f6")
            f.write(bytes('hello', 'UTF-8'))
        else:
            f.write(text_type("\u00f6", "UTF-8"))
            f.write("hello")  # bytes
            s = f.getvalue()
            f.close()
            assert isinstance(s, text_type)

Is a bad idea, as now we are capturing TypeError for the entire method.

Some other examples:

@pytest.mark.skipif("sys.version_info >= (3,)", reason="python2 has no buffer")
def test_dontreadfrominput_buffer_python2():
    from _pytest.capture import DontReadFromInput

    f = DontReadFromInput()
    with pytest.raises(AttributeError):
        f.buffer
    f.close()  # just for completeness

def test_parsing_again_fails(self, testdir):
    config = testdir.parseconfig()
    pytest.raises(AssertionError, lambda: config.parse([]))

There are 235 occurrences of pytest.raises on testing so of course I'm not looking at them all, but I'm having a hard time finding any test which using the decorated function looks like a good idea.

[RonnyPfannschmidt]

im closing this as wont-fix since this proposal would mean to move the actual checking/assertion from a specific controlled point to the whole test - which in turn would massively diminish the value of the raises/warns assertions as the cover much more code than the actual supposed point of failure/warning

[megabyde]

I was not proposing to change or remove existing context manager. Instead, I was proposing to complement it with a decorator for people who find it useful. I was already working on a PR.

[nicoddemus]

Hi @megabyde,

I think the main point against your proposal is:

move the actual checking/assertion from a specific controlled point to the whole test

I was already working on a PR.

Sorry you have wasted time on this. 😕

[megabyde]

If you need fine-grained exception checks you will be using a context manager, if you want to cover a whole test you can use a decorator. Having both context manager and decorator doesn't enforce all users to use my proposal, right? We have a lot of users coming to pytest from nose and they've been using decorators for this purpose, so how having two options will hurt anybody?

[blueyed]

@megabyde
It is more code to maintain in the first place.
I also agree that pytest.raises should only be applied to the minimum.

Even with your simple example I do not see the benefit of using a decorator:

# No redundant scope to catch an exception 
@raises(IndexError)
def test_raises_as_context_manager():
    answers = []
    answers[0] = 42

What is bad about having a new scope / context manager here?

[nicoddemus]

We have a lot of users coming to pytest from nose and they've been using decorators for this purpose

@megabyde if the purpose is to facilitate migration from nose to pytest, perhaps we can contribute to nose2pytest to automatically change the code from decorator to context manager?