Add support to testing.RaisesGroup for catching unwrapped exceptions

[jakkdl]

• Deprecates the strict parameter, renaming it to flatten_subgroups.
• Adds the allow_unwrapped parameter, adding the possibility to more closely mirror except*. This is a feature requested in https://github.com/pytest-dev/pytest/issues/11538#issuecomment-2024361910 by @belm0
• Fixes a bug I noticed while reading the code, where e.g.

with RaisesGroup(ValueError, TypeError, strict=False):
  raise ExceptionGroup("", [ExceptionGroup("", [ValueError(), TypeError()])])

would fail due to it checking if the number of exceptions was correct before flattening the structure of the raised exceptiongroup.

~~I also found an unrelated issue where mypy&pyright are not able to deduce the type when passing multiple Matcher objects matching against different exceptions to RaisesGroup. (or Matcher+class, but two classes work). Fiddled around a little bit but wasn't able to quickly find a fix, and explicitly setting [Exception] isn't especially onerous.~~ EDIT: this was due to TypeVar lacking covariance.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.63%. Comparing base (ccd40e1) to head (6ef442b).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2989   +/-   ##
=======================================
  Coverage   99.63%   99.63%           
=======================================
  Files         120      120           
  Lines       17710    17798   +88     
  Branches     3179     3198   +19     
=======================================
+ Hits        17645    17733   +88     
  Misses         46       46           
  Partials       19       19           

Files	Coverage Δ
src/trio/_core/_run.py	99.34% <ø> (ø)
src/trio/_core/_unbounded_queue.py	100.00% <ø> (ø)
src/trio/_deprecate.py	100.00% <100.00%> (ø)
src/trio/_highlevel_open_tcp_listeners.py	100.00% <ø> (ø)
src/trio/_tests/test_deprecate.py	100.00% <100.00%> (ø)
src/trio/_tests/test_testing_raisesgroup.py	100.00% <100.00%> (ø)
src/trio/_threads.py	100.00% <ø> (ø)
src/trio/testing/_raises_group.py	100.00% <100.00%> (ø)

[jakkdl]

~~Oh, turns out the unrelated typing error does work with pyright even with the overloads, and it's solely a mypy thing. I'll try to get a minimal repro and/or see if it's a known mypy issue and/or report it.~~ EDIT: fixed by making TypeVar covariant

[jakkdl]

@TeamSpen210 after adding covariant=True to the TypeVar the docs/source/typevars.py fails to resolve it:

/home/h/Git/trio/looser_excgroups/src/trio/testing/__init__.py:docstring of trio.testing.RaisesGroup:1: WARNING: py:obj reference target not found: typing.Callable[[BaseExceptionGroup[+E]], bool] | None
/home/h/Git/trio/looser_excgroups/src/trio/testing/__init__.py:docstring of trio.testing.Matcher:1: WARNING: py:class reference target not found: type[+E] | None
/home/h/Git/trio/looser_excgroups/src/trio/testing/_raises_group.py:docstring of trio.testing._raises_group._ExceptionInfo.type:1: WARNING: py:class reference target not found: type[+E]

I'm a bit surprised as covariant typevars exist in a few other places in the codebase without issue. Unless you have any opinions I'll try do some ugly workaround where I either add a plussed [copy] to the dicts in identify_typevars, or strip +s from the target in lookup_reference

EDIT: nope, wasn't that easy to work around...

[jakkdl]

I can make a theoretical case for splitting strict=False into two separate flags - one that allows unwrapped exceptions, and one that flattens nested exceptiongroups. But I suspect the added burden of having to set both might outweigh the gain from somebody that actually wants them split:

# they want this to pass
with RaisesGroup(ValueError, strict=False):
    raise ExceptionGroup("outer", [ExceptionGroup("inner", [ValueError()])])
# but this to fail
with RaisesGroup(ValueError, strict=False):
    raise ValueError

Of course they can just do

with RaisesGroup(..., strict=False) as e:
    ...
assert isinstance(e, ExceptionGroup)

but that's easy to forget and could silently introduce changes in behavior.

One, probably better, case in favor of splitting out allow_unwrapped=True is that we can raise an error in RaisesGroup.__init__ if specifying several exceptions + allow_unwrapped=True. That's both easier to implement and faster to debug for end-users than trying to resolve that with a message on a failed catch https://github.com/python-trio/trio/pull/2989#discussion_r1565979668

Since typing with RaisesGroup(ValueError, allow_unwrapped=True, flatten=True) is a mouthful we (or leave that for pytest) could add convenience aliases, or just rely on end-users to write their own thin wrappers that change the defaults.

[TeamSpen210]

Fixed the typevar-related issues, but now there's something else in history.rst. Not fully sure why the +E is only happening with this typevar, but I found a workaround. By using autodoc_process_signatures() I could insert the fully qualified name for the var, allowing Sphinx to find it.

[jakkdl]

Seems like nobody else had opinions on strict vs allow_unwrapped+flatten_subgroups. The ability to raise a ValueError on users doing RaisesGroup(SyntaxError, TypeError, allow_unwrapped=True) with a helpful message on what else to do convinced me that this is the way to go, even if it will be more verbose when you want to fully emulate except* (i.e. you expect a single exception, but don't care if it's unwrapped, or in any level of nesting).

[jakkdl]

bumping exceptiongroup so whoever reviews the next dep bump don't have to spend any time tracking down why type tests start ""failing"" (i.e. no longer xfail)

[jakkdl]

Looks good. I think I saw a few regexes reading through the changes that could still have end specifiers added, but I think it should be ok.

The missing end specifiers are intended because I didn't bother including the full error message.

I fixed it so RaisesGroup now supports it on ExceptionGroup - previously it failed since it included "1 sub-exception" in the message it matched against, since

>>> str(ExceptionGroup('foo', [ValueError()]))
'foo (1 sub-exception)'

[jakkdl]

oh sorry for the re-request, didn't get notified about your review

[jakkdl]

oh sorry for the re-request, didn't get notified about your review

fixed everything now though, so re-review request is warranted

[jakkdl]

I encountered some issues with typing for functions passed to the check argument, and thought about handling that in a separate pull request - but it got so thorny that I'll just add the failing tests now and maaaybe address it in a separate PR once I hear from https://github.com/python/mypy/issues/17251

[A5rocks]

Also re: your mypy bug, my impression was that it just ignores __new__ in favor of __init__ if it's available.

[jakkdl]

Also re: your mypy bug, my impression was that it just ignores __new__ in favor of __init__ if it's available.

scrolling through https://github.com/python/mypy/blob/master/test-data/unit/check-classes.test I guess that is the current state of things with mypy. But after sleeping on it I think I have a better way of resolving it - but that's for a new PR. It's finally time to merge this one :rocket: