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

Design discussion: how should we test for expected ExceptionGroups? #10441

Closed
Zac-HD opened this issue Oct 27, 2022 · 10 comments · Fixed by #11424
Closed

Design discussion: how should we test for expected ExceptionGroups? #10441

Zac-HD opened this issue Oct 27, 2022 · 10 comments · Fixed by #11424
Labels
status: help wanted developers would like help from experts on this topic

Comments

@Zac-HD
Copy link
Member

Zac-HD commented Oct 27, 2022

Python 3.11 has been released with the new ExceptionGroup type, and there's a great backport too.

As people start to write code which can raise ExceptionGroups, there's also going to be a corresponding need to test that they raised the expected group of exceptions, and that's where this issue comes in: our existing APIs aren't particularly helpful for this. For example, in the PR to use ExceptionGroups for Pytest teardown errors, I used with pytest.raises(...) to catch the group, but then had to manually unpack the structure and do isinstance() checks by hand.

I'd like to take our time and find the right API, rather than hurrying to any particular conclusion, and that should be based on a volume and diversity of experience that just doesn't exist yet. This issue therefore exists to collect use-cases, design ideas, and discussion, until we're confident that we can ship the right thing for the next decade.

@Zac-HD Zac-HD added the status: help wanted developers would like help from experts on this topic label Oct 27, 2022
@Zac-HD
Copy link
Member Author

Zac-HD commented Oct 27, 2022

One fairly simple use-case is when you have a single error, wrapped in one or more layers of ExceptionGroups - this is also going to be quite common for people using asyncio.TaskGroup() or trio.open_nursery(strict_exception_groups=True).

I think the usual desire will be to simply "unwrap" these as they're conceptually a single error, and ideally with pytest.raises(...) could do that. For correctness it seems important to specify the expected number of layers to unwrap; and we might also want to have a match= pattern for the message at each layer.

I don't even have an API sketch for this, but it seems important both that this simple case be very convenient, but also that whatever approach we use scales smoothly up to more complicated groups of errors.

@RonnyPfannschmidt
Copy link
Member

To me that is something that needs a outside of pytest experiment that can iterate breaking

Its hard to get right, I'd like to use something like the ideas from dirty equals for it

@Zac-HD
Copy link
Member Author

Zac-HD commented Oct 27, 2022

Yeah, I certainly wouldn't want to ship something that hasn't already seen some adoption and iteration, and ideally serious use in some different kinds of code (e.g. web-heavy async, GUI or robotics, etc.).

@Zac-HD
Copy link
Member Author

Zac-HD commented Oct 27, 2022

A more complex case, where the solution currently in our self-tests is kinda inelegant:

raise ExceptionGroup(
    "errors during test teardown (2 sub-exceptions)", 
    [KeyError("from module scope"),
     ExceptionGroup(
        "errors while tearing down <Function test_func> (2 sub-exceptions)",  # might want to test this message
        [TypeError("from function scope 1"), ValueError("from function scope 2")]),
    ])

with pytest.raises(ExceptionGroup, match="errors during test teardown") as e:
ss.teardown_exact(None)
mod, func = e.value.exceptions
assert isinstance(mod, KeyError)
assert isinstance(func.exceptions[0], TypeError) # type: ignore
assert isinstance(func.exceptions[1], ValueError) # type: ignore

That's all I have for now; I'll share more as they're discovered and hope others will too!

@xaviergmail
Copy link

xaviergmail commented Nov 9, 2022

I am also looking forward to an official solution. In the meantime FWIW, I have pieced this together: https://gist.github.com/xaviergmail/90355b7611dcff27a6a0a60297666865

(In my case, I'm testing for exceptiongroups thrown by cattrs)

@lanzz
Copy link
Contributor

lanzz commented Sep 9, 2023

How about providing some assertion helpers in ExceptionInfo, e.g. something like

with pytest.raises(ExceptionGroup) as exc_info:
    tested_function()
assert exc_info.group_contains(RuntimeError, match='expected message')
assert not exc_info.group_contains(ValueError)

with more or less the same call signature as pytest.raises? Could support additional arguments to specify recursion into subgroups? Probably won't be able to deal with strictly matching complicated nested structures, but could cover lots of typical use cases.

@lanzz
Copy link
Contributor

lanzz commented Sep 10, 2023

I've implemented a proof of concept and opened a PR for it

@RonnyPfannschmidt
Copy link
Member

Id like to notes that ExcInfo is a api I'd like to see phased out instead of expanding it's

It's a White elephant that's similar to local path and other parts of pyllib

@nicoddemus
Copy link
Member

nicoddemus commented Sep 10, 2023

Id like to notes that ExcInfo is a api I'd like to see phased out instead of expanding it's

Hmmm phase out and replace it with what? Also note that this would break a huge number of test suites I believe, so we should do it (if we ever do it that is) with utmost care, preferably being backward compatible too.

@RonnyPfannschmidt
Copy link
Member

@nicoddemus i stubled uppon #3399 which has a similar goal but talks of refactoring, lets continue there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help wanted developers would like help from experts on this topic
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants