Add a "pytest.maybe_raises"

[ikonst]

What's the problem this feature will solve?

Sometimes it's convenient to parameterize both raising and non-raising case, e.g.

@pytest.mark.parameterize(['value', 'exception_expected'], [
    ('good', None),
    ('bogus', ValueError),
])
def test_foobar(value, exception_expected):
    with maybe_raises(exception_expected) as exc_info:
        foobar(
            value,
            constant_value="no need to repeat twice",
        )
    if exc_info:
        assert exc_info.value == 'message'

With pytest as it stands, I'd have to do something more akward like:

if exception_expected:
    with pytest.raises(exception_expected) as exc_info:
        foobar(...)
    assert exc_info.value == 'message'

or if I prefer not to repeat the call:

with pytest.raises(exception_expected) if exception_expected else nullcontext():
    foobar(...)

Describe the solution you'd like

Here's the snippet from my project's test helper module:

E = TypeVar('E', bound=BaseException)


def maybe_raises(
    expected_exception: Union[Type[E], Tuple[Type[E], ...], None],
    *,
    match: Optional[Union[str, Pattern[str]]] = None,
) -> ContextManager[Optional[pytest.ExceptionInfo]]:
    if expected_exception is not None:
        return pytest.raises(expected_exception, match=match)
    else:
        return contextlib.nullcontext()

Alternative Solutions

1. The code above is from my project code, so it's possible to complement pytest rather than add to it. However, since I found this to be a useful pattern, I'd like to propose adding it to pytest.

2. We can make pytest.raises accept an optional exception and return a ContextManager[Optional[pytest.ExceptionInfo]]. As-is it'd be a breaking change, but we could perhaps do this as an @override.

[RonnyPfannschmidt]

Use null content

[ikonst]

1. As it stands, not great for type-checking.

   This code

   from typing import Optional
   
   expected: bool
   ...
   with (pytest.raises(Exception) if expected else contextlib.nullcontext()) as exc_info:
       ...

   results in (mypy)

   error: "object" has no attribute "__enter__"
   error: "object" has no attribute "__exit__"
   

   since the common base for RaisesContext and ContextManager is object.

   Although this might be addressed if pytest.raises is annotated to return typing.ContextManager[E] or so that RaisesContext will be deemed a ContextManager.

2. Clearly more syntax.

On the contrary:

1. What is the concern with e.g. a pytest.raises overload that accepts an optional and yields an optional?
2. Is the implementation complicated?
3. You dislike the API contract it gives users?

[RonnyPfannschmidt]

The api contract is simply unacceptable, there was a lengthy disc before

Im happy to fix the typing, i will absolutely refuse api shitstains like magic boolean flags that change behaviour so strongly it should have a different name to begin with

[ikonst]

Sorry, couldn't find the previous issue, mind linking for posterity? I found #8646 somewhat related.

I'm not sure I follow re: "magic boolean flags". There's no current semantical meaning to pytest.raises(None) . The proposal is either:

1. Give it the semantical meaning of "raises nothing" i.e. "does not raise".
2. Keep it a strict error as done in Improved error when () (empty tuple) is passed to pytest.raises() or pytest.warns() #8646 and introduce a new "pytest.maybe_raises".

I do understand that aversion to (1), as raises(None) and warns(None) are current invalid.
Similarly, I do understand the aversion to (2), since unless it's an obvious common and widely used pattern, there's a whole slew of patterns one could try and add to core pytest.

But whatever it is, it's good to record the decision.

And at the very least, we should address that typing issue.

[RonnyPfannschmidt]

One thing is absolutely in stone, raises itself is only for ensuring exceptions happen

Opening the api of raises itself up to not doing that is horrendous api Design that trades a small convenience for a niche usecase with a unacceptable "forever cost"

The context manager that just does nothing is already null context in the stdlib

[RonnyPfannschmidt]

Additionally the context manager that ignores certain exceptions is also in the stdlib, so there currently really seems to be no reason to expand the api, Only a reason to fix the type information

[Zac-HD]

There's no current semantical meaning to pytest.raises(None) .

This is the result of a fairly long and involved migration, because when that did do something there were three wildly different and common assumptions about what it meant.

Unfortunately we're not going to accept this feature request; you can use nullcontext() and ignore the type issues, write two tests, write an if-statement, write your own helper function, etc.

[ikonst]

write your own helper function

That's in fact what I did, which made me think that could be valuable in pytest, which resulted in this proposal, but obviously there's a high bar for including something in a core library (given that we're then "stuck" maintaining it) and I'm fine not including it.

Related decision in another test helper library:
https://testfixtures.readthedocs.io/en/latest/exceptions.html#exceptions-that-are-conditionally-raised
I assume some people would think this kind of parameterization encourages sloppy tests, and that failure modes should be covered by a distinct test.