Skip to content

Allow pytest.raises cooperate with ExceptionGroups #11538

Description

@elupus

What's the problem this feature will solve?

Let pytest.raises() handle ExceptionGroup unwrapping, similar to what an "except*" clause would do. The following is valid python code that will catch the ValueError exception:

try:
  raise ExceptionGroup("", [ValueError])
except* ValueError:
  pass

However pytest.raises will not manage this, and will fail with catching the ExceptionGroup instead.

with pytest.raises(ValueError):
  raise ExceptionGroup("", [ValueError])

Describe the solution you'd like

Assert some code throws an exception or an exception as part of an exception group.

anyio/trio will now always wrap exception in ExceptionGroups when raised from taskgroups. This leads to silly checks like: https://github.com/agronholm/anyio/blob/3f1eca1addcd782e2347350a6ddb2ad2b80c6354/tests/test_taskgroups.py#L279C1-L287C31 to unwrap the exceptiongroup.

A basic solution to handle this (without the bells and whistles) would be:

@contextlib.contextmanager
def raises_nested(exc):
    try:
        yield
    except* exc:
        pass
    else:
        raise AssertionError("Did not raise expected exception")

Alternative Solutions

Additional context

Activity

  1. added
    type: proposalproposal for a new feature, often to gather opinions or design the API around the new feature
    on Nov 3, 2023
  2. Zac-HD commented on Nov 3, 2023

    @Zac-HD
    Member

    Related to #10441 which was marked as fixed in #11424 (not yet released). The main blocker here is someone designing an interface which:

    • is convenient to use, and simple in simple cases
    • doesn't only handle simple cases
    • doesn't encourage users to ignore e.g. the nesting depth of their exception groups or the possibility of multiple unrelated errors

    We're generally happy to review suggestions, if you'd like to propose something!

  3. elupus commented on Nov 3, 2023

    @elupus
    Author

    I suggested something above. That test generally matches the semantics of except*

  4. elupus commented on Nov 3, 2023

    @elupus
    Author

    Okey it's simple, it does ignore nest level. But that is one very common need.

    Ps You generally don't use raises () to test for exception chains with the old api. raise A() from b.

  5. jakkdl commented on Nov 22, 2023

    @jakkdl
    Member

    Related to #10441 which was marked as fixed in #11424 (not yet released). The main blocker here is someone designing an interface which:

    • is convenient to use, and simple in simple cases

    • doesn't only handle simple cases

    • doesn't encourage users to ignore e.g. the nesting depth of their exception groups or the possibility of multiple unrelated errors

    We're generally happy to review suggestions, if you'd like to propose something!

    I'm not sure whether this should go in here, or in #10441 (which was maybe closed prematurely), or in a new issue - but when writing a helper for Trio I went through a couple ideas for ways to solve this - and I think my favorite one is introducing a class ExpectedExceptionGroup that pytest.raises can accept an instance of, in place of a type[Exception].

    Example usage:

    from pytest import ExpectedExceptionGroup
    
    with pytest.raises(ExpectedExceptionGroup((ValueError,))):
      ...

    supports nested structures:

    with pytest.raises(ExpectedExceptionGroup(ExpectedExceptionGroup((KeyError,)), RuntimeError)):
      ...

    and works with the current feature of passing a tuple to pytest.raises when you expect one of several errors - though maybe not in a super elegant way if you're nested several layers deep:

    with pytest.raises(
      (
        ExpectedExceptionGroup(KeyError),
        ExpectedExceptionGroup(ValueError),
        RuntimeError,
      )
    ):
      ...

    Can additionally work in keyword args for matching message and/or note.

    with pytest.raises(ExpectedExceptionGroup((KeyError, ValueError), message="foo", note="bar")):
      ...

    and possibly also accept instances of exceptions, to be able to match the message in sub-exceptions

    with pytest.raises(ExpectedExceptionGroup((KeyError(".* very bad"), RuntimeError)):
      ...

    This would fulfill all of the requested bullet points, be a minimal change to the API, and shouldn't be very complicated to implement. To experiment outside of pytest (as requested in #10441 (comment)) should be somewhat doable - if a bit messy*

    *can duplicate the functionality of pytest.raises,
    or monkey patch _pytest.python_api.RaisesContext.__exit__,
    or possibly with https://docs.python.org/3/library/abc.html#abc.ABCMeta.__subclasshook__ to trick https://github.com/pytest-dev/pytest/blob/fdb8bbf15499fc3dfe3547822df7a38fcf103538/src/_pytest/python_api.py#L1017C3-L1017C3 )

  6. RonnyPfannschmidt commented on Nov 22, 2023

    @RonnyPfannschmidt
    Member

    A while back I discussed the idea of matchers with @asottile

    I believe having a matcher as single argument beats making a ever growing complexity of the raises arguments

  7. jakkdl commented on Nov 23, 2023

    @jakkdl
    Member

    raises is quite messy, and would be more so with my suggestion (though most of the complexity would lie in ExpectedExceptionGroup - not in raises), but I think pytest needs support for exception groups sooner rather than later. I'm all for a clean and beautiful matcher argument, but if the above suggestion is acceptable I'll write up a PR.

  8. Zac-HD commented on Nov 27, 2023

    @Zac-HD
    Member

    The extension to pytest.raises() mostly works for me - I like the idea of having a matcher object you pass to raises(), but the interface feels a little off to me.

    We're passing inner exception instances, but interpreting their __str__ (which is usually but not always equal to the message string) as a regex pattern to search by, instead of the usual interpretation as a string. Could we have an alternative form where regex patterns are explicitly passed via match= arguments? I think that having one new keyword-only argument to raises() is going to be easier on users than introducing a new ExpectedExceptionGroup type. Concretely, this might look like:

    with pytest.raises(
        group=(  # asserts that we raise a (subclass of) `BaseExceptionGroup`, with exceptions matching the sequence given:
            pytest.raises(ValueError, match="vmsg"),
            ..., # Ellipsis literal to indicate "one or more exceptions omitted"
            AssertionError,  # also valid, ignores the message
        ),
        match="message",  # interpreted as matching the message (and notes, if any) of the ExceptionGroup
    ):
        raise ExceptionGroup("message", [ValueError("vmsg"), KeyError("kmsg"), AssertionError()])

    Thoughts?

  9. elupus commented on Nov 27, 2023

    @elupus
    Author

    How does this proposal handle the case of matching behaviour of except*, which does not care about nesting level of group?

    try:
      raise ExceptionGroup([ExceptionGroup([ValueError(0])
    except* ValueError:
    try:
      raise ExceptionGroup([ValueError(0])
    except* ValueError:

    The nesting level is an implementation detail that an API user does not care about.

    I would expect to be able to use raises to catch both above cases, same as you catch both cases with the same exception handler.

  10. RonnyPfannschmidt commented on Nov 27, 2023

    @RonnyPfannschmidt
    Member

    @Zac-HD i find the extension quite hard to grasp, unless we externalize the complexity id rather see a new entrypoint to exceptiongroups

    the raises api already is overcomplicated

    what you pass in as group is basically a matcher without being explicit or defined

    i say NO to that type of compexitx, its hard to read, hard to grasp and hard to define propperly

  11. jakkdl commented on Nov 27, 2023

    @jakkdl
    Member

    How does this proposal handle the case of matching behaviour of except*, which does not care about nesting level of group?

    good question - I was not thinking of that use case ~at all, since I was thinking about it from the POV of a library maintainer that cares about the nesting level. I'll see if I can add a kwarg that toggles this.
    If we want to further mirror except* behaviour then

    with pytest.raises(ExpectedExceptionGroup(ValueError)):
      raise ValueError

    should maybe work as well.

    .
    Likewise my current implementation (which you can see at https://github.com/python-trio/trio/blob/84fb527cf9d9a97baf11c7437870f52c6b318992/src/trio/testing/_exceptiongroup_util.py) also checks that exactly the specified exceptions are encountered, and no others, unlike #11424 which will ignore any extraneous errors. I'll try to add a kwarg that toggles that behaviour as well - although I'm unsure if that is a good pattern - if your program is going from raising ExceptionGroup("", (ValueError,)) to raising ExceptionGroup("", (ValueError, SyntaxError)) that's probably something the test suite should make you aware of.

  12. jakkdl commented on Nov 27, 2023

    @jakkdl
    Member

    We're passing inner exception instances, but interpreting their __str__ (which is usually but not always equal to the message string) as a regex pattern to search by, instead of the usual interpretation as a string. Could we have an alternative form where regex patterns are explicitly passed via match= arguments?

    Yeah this isn't great, I encountered problems when using it with an OSError - where str(OSError(5, "foo")) == '[Errno 5] foo' and those [s get interpreted as regex patterns. Checking args doesn't work great with regex's either.

    Though a small wrapper around the exception, instead of instantiating it, seems like a good idea. Could be something like this:

    with pytest.raises(
      ExpectedExceptionGroup(
        ValueError,
        pytest.matcher(ValueError, match=r'foo[bar]'),
        # can add a fully customizable kwarg that takes a callable
        pytest.matcher(OSError, check=lambda x: x.bar == 7),
    
        # it could even make the pos arg skippable
        # e.g. when you want to check `== type` instead of using `isinstance(..., type)`
        pytest.matcher(check=lambda x: type(x) == my_error_generator()),
      )
    ):
  13. jakkdl commented on Nov 27, 2023

    @jakkdl
    Member

    I think that having one new keyword-only argument to raises() is going to be easier on users than introducing a new ExpectedExceptionGroup type.

    Concretely, this might look like:

    with pytest.raises(
        group=(  # asserts that we raise a (subclass of) `BaseExceptionGroup`, with exceptions matching the sequence given:
            pytest.raises(ValueError, match="vmsg"),
            ..., # Ellipsis literal to indicate "one or more exceptions omitted"
            AssertionError,  # also valid, ignores the message
        ),
        match="message",  # interpreted as matching the message (and notes, if any) of the ExceptionGroup
    ):
        raise ExceptionGroup("message", [ValueError("vmsg"), KeyError("kmsg"), AssertionError()])

    Thoughts?

    Seems messy to specify nested exceptiongroups - would be something like this?

    with pytest.raises(
      group=(
        pytest.raises(
          group=(
            pytest.raises(ValueError, match="vmsg"),
          )
        )
      )
    ):
      ...
  14. jakkdl commented on Nov 27, 2023

    @jakkdl
    Member

    If we want to step away from pytest.raises, and also not have a type, - an alternative could be something like

    with pytest.groupraises(
      ValueError,
      pytest.groupraises(
        SyntaxError,
      )
    ):
      ...

    or as a type

    with pytest.RaisedGroup(
      ValueError,
      pytest.RaisedGroup(
        SyntaxError,
      )
    ):
      ...
  15. nicoddemus commented on Dec 2, 2023

    @nicoddemus
    Member

    We're passing inner exception instances, but interpreting their str (which is usually but not always equal to the message string)

    Definitely -1 for me, custom exceptions might not even accept a string at all as argument, a real world example:

    class ReaderError(Exception):
    
        def __init__(self, reader_code: int, cause: Cause, reason: str) -> None: ...

    One thing we should consider is that Python decided to go with a different syntax for "unpacking" exception groups, so we probably should do the same and implement a separate function, say pytest.raises_group. I see some advantages to that:

    • We might decide to go with a simpler implementation initially, for instance only matching a single exception from the group, without worrying about hierarchy, and decide how to expand on that later.
    • We can postpone to decide on more complicated aspects such as how to also match the exceptions against their messages.

    Trying to shoehorn exception group unpacking into pytest.raises seems risky, as we have several concerns to solve (how match should be handled?), plus any future extension to pytest.raises will have to take exception groups into account, and for some extensions it might not even make sense to also support exception groups.

  16. 6 remaining items

  17. jakkdl commented on Dec 14, 2023

    @jakkdl
    Member

    I'm going ahead and adding a helper to Trio for now though, publicly exposing it among with its other test helpers, and we'll see if that garners any feedback from users.
    python-trio/trio#2898

  18. keewis commented on Dec 31, 2023

    @keewis

    If it is, I assume that this isn't:

    with pytest.raises(
      Match(
        (ExceptionGroup, ValueError),
        [
          ...
        ]
      )
    ): ...

    yes, that should raise (but the example before that would make sense to me). Instead, you'd have to use

    with pytest.raises((Match(ExceptionGroup, [...]), ValueError)):
        ...

    The only downside is that sharing the pattern is not as easy.

    For a more crazy thought (so it may be a bad idea, but it would certainly be easier to understand): maybe we can replace the tuple with __or__? I.e. ValueError | RuntimeError currently results in a typing.Union object which raises would need to interpret correctly, but it should in theory also be possible to get Match(...) | ValueError / ValueError | Match(...) / Match(...) | ValueError | Match(...) to work.

    Regarding the nesting: true, this syntax would become hard to read quickly with increased nesting. In this case I think it would make sense to take inspiration from libraries that focus on dealing with trees / graphs. For example, networkx and datatree both can take a nested dict and construct an object from that. For the proposed Match, I'd imagine that to look somewhat like this (the naming could be better, though):

    Match.from_dict({"exceptions": ExceptionGroup, "children": (ValueError, RuntimeError), "match": "pattern"})
    Match.from_dict({"exceptions": (ExceptionGroup1, ValueError), "match": "pattern"})
    Match.from_dict({"exceptions": ExceptionGroup1, "children": [{"exceptions": ExceptionGroup2, "match": "pattern", "children": [ValueError]}, RuntimeError], "match": "pattern"})
    Match.from_dict({"exceptions": (ExceptionGroup1, ExceptionGroup2), "children": [{"exceptions": ExceptionGroup3, "match": "pattern", "children": [ValueError]}, RuntimeError], "match": "pattern"})

    Which might be a bit easier to read even at deeper nesting levels, especially with proper formatting? The tuple in exceptions might also be replaced with a list or the type union from above.

  19. elupus commented on Jan 29, 2024

    @elupus
    Author

    Seems pytest 8 solves this issue. Looks to have taken my preferred route of matching as old raises.
    https://docs.pytest.org/en/stable/how-to/assert.html#assert-matching-exception-groups

  20. jakkdl commented on Jan 29, 2024

    @jakkdl
    Member

    Seems pytest 8 solves this issue. Looks to have taken my preferred route of matching as old raises. https://docs.pytest.org/en/stable/how-to/assert.html#assert-matching-exception-groups

    (Note: I just opened #11882 after reading the linked docs above, in case anybody is having trouble with running that example)

    I think I've voiced my issues with group_contains in some issue/PR, but I think it's very problematic that the following code passes tests:

    import pytest
    class EXTREMELYBADERROR(Exception):
        ...
    
    def test_foo():
        with pytest.raises(ExceptionGroup) as excinfo:
            raise ExceptionGroup("", (ValueError(), EXTREMELYBADERROR()))
        assert excinfo.group_contains(ValueError)

    In the first example in the docs they have a random assert not excinfo.group_contains(TypeError), but you cannot simply list out all exceptions you don't want to get, and I see no simple way to say "assert only this exception and no others".

    If you're using trio, or you're fine with adding it as a test dependency, I recommend using https://trio.readthedocs.io/en/stable/reference-testing.html#exceptiongroup-helpers

  21. jakkdl commented on Jan 31, 2024

    @jakkdl
    Member

    having slept on it, you can make group_contains safe if you check the number of exceptions in the group, and specify the depth.

    with pytest.raises(ExceptionGroup) as excinfo:
        ...
    assert excinfo.group_contains(ValueError, depth=1)
    assert len(excinfo.value.exceptions) == 1

    (that's perhaps something to add to the docs)

    but once you get more than a depth of 1 it quickly gets super messy, especially with multiple expected exceptions.

  22. belm0 commented on Mar 28, 2024

    @belm0

    I've found neither trio.testing.RaisesGroup() nor pytest excinfo.group_contains() helpful, because they only expect an exception group.

    Use case: testing code that may optionally wrap single exceptions with ExceptionGroup or not. For example, the trio library itself has such an option (which can be set globally). I want to test that my package supports both modes.

    Here is my workaround for now:

    _got_value_error = Mock()
    
    with exceptiongroup.catch({ValueError: _got_value_error}):
        ...
    
    _got_value_error.assert_called_once()

    I may be missing something, but it seems important to be able to mirror the actual semantics of except* when verifying raised exceptions...

  23. Zac-HD commented on Mar 28, 2024

    @Zac-HD
    Member

    Note that asyncio.TaskGroup and anyio.TaskGroup always raise ExceptionGroup, and Trio plans to remove the non-strict option after a sufficient deprecation period.

    I agree that matching except* seems useful though... I still haven't found a generally-satisfying interface for this 😕

  24. jakkdl commented on Mar 31, 2024

    @jakkdl
    Member

    with #11656 it would've been possible with

    with pytest.raises(ValueError, ExpectedExceptionGroup(ValueError)):
       ...

    but since pytest devs preferred a separation from pytest.raises with #11671 / trio.testing.RaisesGroup it became more complicated to fully replicate except*.

    One workaround you can do with the current tooling is:

    with pytest.raises(ValueError, ExceptionGroup) as excinfo:
        ...
    assert isinstance(excinfo.value, ValueError) or trio.testing.RaisesGroup(ValueError).matches(excinfo.value)

    But what I probably should do is add a parameter allow_unwrapped (or add it to strict=False), that allows an unwrapped exception to be matched - if there's only one exception specified in the expected exceptiongroup.

  25. jakkdl commented on May 17, 2024

    @jakkdl
    Member

    I agree that matching except* seems useful though... I still haven't found a generally-satisfying interface for this 😕

    With python-trio/trio#2989 merged this will now be possible with trio.testing.RaisesGroup on next release. I decided to split the behaviour into two parameters, allow_unraised and flatten_subgroups, which when both enabled will fully match the behaviour of except*.

    # will catch all the different raises
    with trio.testing.RaisesGroup(ValueError, allow_unwrapped=True, flatten_subgroups=True):
        if ...:
            # handled with allow_unwrapped=True
            raise ValueError()
        elif ...:
            # always handled
            raise ExceptionGroup("", [ValueError()])
        else:
            # handled with flatten_subgroups=True
            raise ExceptionGroup("", [ExceptionGroup("", [ValueError()])])

    (flatten_subgroups was previously called strict (though inverted), but with two different ways of being strict I had to rename and deprecate strict).

    See https://trio.readthedocs.io/en/latest/reference-testing.html#exceptiongroup-helpers for more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    type: proposalproposal for a new feature, often to gather opinions or design the API around the new feature

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions