pytest: RFC: parametrizing conditional raising with pytest.raises

fortunately, if you really want this behaviour you can get it today from the standard library:

if sys.version_info >= (3, 7):
    from contextlib import nullcontext
elif sys.version_info >= (3, 3):
    from contextlib import ExitStack as nullcontext
else:
    from contextlib2 import ExitStack as nullcontext


@pytest.mark.parametrize(
    ('x', 'expected_raises'),
    (
        (True, pytest.raises(ValueError)),
        (False, nullcontext()),
    ),
)
def test(x, expected_raises):
    with expected_raises:
        foo(x)

• nullcontext
• ExitStack
• contextlib2

For anyone looking for a solution to this now before people agree on naming, you can wrap pytest.raises to support None like this:

from contextlib import contextmanager

import pytest


def raises(error):
    """Wrapper around pytest.raises to support None."""
    if error:
        return pytest.raises(error)
    else:
        @contextmanager
        def does_not_raise():
            yield
        return does_not_raise()

Then you can use it with parameterize:

@pytest.mark.parametrize('example_input,expected_error', [
    (3, None),
    (2, None),
    (1, None),
    (0, ZeroDivisionError),
])
def test_division(example_input, expected_error):
    """Test how much I know division."""
    with raises(expected_error):
        assert (6 / example_input) is not None

I think None being considered by pytest.raises as doing nothing seems sensible.

from contextlib import suppress as do_not_raise

@pytest.mark.parametrize('example_input,expectation', [
    (3, do_not_raise()),
    (2, do_not_raise()),
    (1, do_not_raise()),
    (0, raises(ZeroDivisionError)),
])
def test_division(example_input, expectation):
    """Test how much I know division."""
    with expectation:
        assert (6 / example_input) is not None

Test session starts (platform: linux, Python 3.6.6, pytest 3.6.3, pytest-sugar 0.9.1)
rootdir: /home/sik/code/mne-python, inifile: setup.cfg
plugins: smother-0.2, sugar-0.9.1, pudb-0.6, ipdb-0.1, faulthandler-1.5.0, cov-2.5.1

 matplotlib_test.py ✓✓✓✓                                                                  100% ██████████
======================================= slowest 20 test durations =======================================
0.00s setup    matplotlib_test.py::test_division[3-expectation0]
0.00s setup    matplotlib_test.py::test_division[2-expectation1]
0.00s setup    matplotlib_test.py::test_division[1-expectation2]
0.00s teardown matplotlib_test.py::test_division[3-expectation0]
0.00s setup    matplotlib_test.py::test_division[0-expectation3]
0.00s call     matplotlib_test.py::test_division[3-expectation0]
0.00s teardown matplotlib_test.py::test_division[2-expectation1]
0.00s teardown matplotlib_test.py::test_division[1-expectation2]
0.00s call     matplotlib_test.py::test_division[2-expectation1]
0.00s call     matplotlib_test.py::test_division[1-expectation2]
0.00s teardown matplotlib_test.py::test_division[0-expectation3]
0.00s call     matplotlib_test.py::test_division[0-expectation3]

Results (0.08s):
       4 passed

how about something completely different

like a different context manager, so that instead of parameterizing with booleans, we pass different contexts

@pytest.mark.parametrize('inp, expectation', [
    (1, pytest.raises(ValueError),
    (2, pytest.raises(TypeError),
    (3, pytest.wont_raise)])
def test_bar(inp, exception):
    with expectation:
        bar(inp)

Just needed this same functionality but surprised to know that, even though almost all alternatives discussed would be good enough for the job (at least for me), none was implemented.

It seems though that Python 3.7 onwards will have a null context manager that could be used in this case too: https://stackoverflow.com/a/45187287

Have found myself wanting this exact functionality a few times now. Seems there was a PR put together a few months back, but it was closed. 😕 Would be really nice to see something coalesce. What seems to be the blocker to implementing this in some form?

Just my 2c, this is going to get abused

The original example in the issue would really be two tests as it is testing two behaviors (one with acceptable inputs and one with unacceptable (exceptional) inputs)

The reason I say this is going to be abused is we had a similar context manager in testify and found that developers (especially beginners) would add it to every test despite it being a noop. Their thought being that it is better to be explicit that it doesn’t raise but the mistake being that an exception already failed the test.

EDIT (for example):

def test_x():
    with pytest.does_not_raise():
        assert x(13) == 37

when really they wanted just

def test_x():
    assert x(13) == 37

I was planning to put some time on it ~this weekend~ and also modify the warnings. last weekend did not happen. Lets see next 😉

@massich

@pytest.mark.parametrize('example_input,expectation', [
    (3, do_not_raise),
    (2, do_not_raise),
    (1, do_not_raise),
    (0, raises(ZeroDivisionError)),
])
def test_division(example_input, expectation):
    """Test how much I know division."""
    with expectation:
        assert (6 / example_input) is not None

See the mailing list threads: https://mail.python.org/pipermail/pytest-dev/2017-March/thread.html https://mail.python.org/pipermail/pytest-dev/2017-April/thread.html

The main options were:

• with pytest.raises(None):
• with pytest.raises(pytest.NoException): or similar (like the above, but it makes it more explicit, and it won’t be done by accident)
• with pytest.does_not_raise: or similar (which means you need to parametrize the whole “expectation”)

Then it was bikeshedded to death. 😢

(Honestly, I stopped caring. I just started adding stuff to my own testsuite instead of contributing them to pytest. I’d still be happy for this to be added, but if it ends like this, I’m out.)

I’m arguing that making new default value isn’t the best way to do it.

Oh sorry, I wasn’t very clear then. After seeing @RonnyPfannschmidt’s response, I think the best approach is to just provide a new context manager which just ensures nothing is raised. This way users can use it in the parametrization:

@pytest.mark.parametrize('inp, expectation', [
    (-1, pytest.raises(ValueError)),
    (3.5, pytest.raises(TypeError)),
    (5, pytest.does_not_raise),
    (10, pytest.does_not_raise),
])
def test_bar(inp, expectation):
    with expectation:
        validate_positive_integer(inp)

Just to clarify, this does not require changes to pytest.raises at all, only creating does_not_raise which should be very simple.

I provided a full recipe here including contextlib2 if someone wants to copypaste into docs

@Sup3rGeo my problem is it encourages two bad patterns:

1. tests with logic, (abuse of parametrize to test disparate behaviours)
2. tests with unnecessary contexts (noop do_not_raise when it is unnecessary)

is it insufficient to from contextlib import nullcontext as do_not_raise?

Let’s consider the scenarios of having/not having the noop context manager (considering here do_not_raise):

Worst thing that can happen if we DO have do_not_raise: a1 - Proponents of pytest.raises(None) won’t be 100% satisfied. But maybe still 80% satisfied? a2 - Beginners can abuse it. This can happen with a lot of things as well, but just because some users can abuse it, it is not fair to prevent all the other users from having it.

Worst thing that can happen if we DON’T have do_not_raise b1 - Increasing the LOC and maintenance passive where we require to parametrize on exception raise. b2 - Not consistent with warns in terms of noop.

Bottom line: Just implement it - its a small and uncertain harm (I would doubt someone will complain) compared to a big and certain benefit (many happy users).

Friendly nudge. Ran into this problem again and figured I’d put it back on people’s radar. 😉

Clearly this is desirable to many. Any ideas how we can unstick this one? 😄

@arel magic numbers are any time when one uses a general value that encodes specific meaning without naming it

so using None to mean “throws no exception” makes it a magical constant

given that it needs a new name to encode new behaviour, its far more simple to just make a new function with a new name and keeping the logic paths more simple

By the way, something which wasn’t mentioned yet: The problem with having a separate context manager like does_not_raise is that you have to parametrize the whole expectation like so:

@pytest.mark.parametrize('inp, expectation', [
    (1, pytest.raises(ValueError),
    (2, pytest.raises(FooError, match="Match on error message"),
    (3, pytest.does_not_raise)])
def test_bar(inp, expectation):
    with expectation:
        bar(inp)

However, if you want to use other nice features of pytest.raises like match (see above), this gets really cumbersome, and your message is “far away” from the rest of your test.

So my vote would still be for with pytest.raises(pytest.NoException): or the (simpler but less explicit) with pytest.raises(None):, for very much practical rather than ideological reasons 😉

@RonnyPfannschmidt

taking a function, and the suddenly making it work different seems very problematic to me

Yeah I agree, and I think your proposal fits nicely with what @The-Compiler had in mind. @The-Compiler, thoughts?

Here is an example which I think also illustrates @The-Compiler’s use case, taken from pytest-qt:

def context_manager_wait(qtbot, signal, timeout, multiple, raising,
                         should_raise):
    """
    Waiting for signal using context manager API.
    """
    func = qtbot.waitSignals if multiple else qtbot.waitSignal
    if should_raise:
        with pytest.raises(qtbot.SignalTimeoutError):
            with func(signal, timeout, raising=raising) as blocker:
                pass
    else:
        with func(signal, timeout, raising=raising) as blocker:
            pass
    return blocker

This could be changed to:

def context_manager_wait(qtbot, signal, timeout, multiple, raise_expectation,
                         should_raise):
    """
    Waiting for signal using context manager API.
    """
    func = qtbot.waitSignals if multiple else qtbot.waitSignal
    with raise_expectation:
        with func(signal, timeout, raising=raising) as blocker:
            return blocker

This change makes sense only in parameterized tests and parameterized tests make sense only when testing things without side-effects, don’t you think? I mean I don’t see how you would parametrize side-effects test in a way that it’s readable etc. Maybe I’m wrong about that, but If I’m not, then my argument still holds.

I see your point, but are you arguing for us to drop this topic because your solution is enough? In that case I disagree, it is still worth discussing how change pytest.raises (or add a new pytest.no_raise). If on the other hand you are just pointing out an alternative, that’s perfectly valid and welcome, but doesn’t bar the current discussion.