Add support to testing.RaisesGroup for catching unwrapped exceptions

docs/source/conf.py

@@ -21,6 +21,7 @@
 import collections.abc
 import os
 import sys
+import types
 from typing import TYPE_CHECKING, cast
 
 if TYPE_CHECKING:
@@ -98,14 +99,22 @@
 # https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html#event-autodoc-process-signature
 def autodoc_process_signature(
     app: Sphinx,
-    what: object,
+    what: str,
     name: str,
     obj: object,
     options: object,
     signature: str,
     return_annotation: str,
 ) -> tuple[str, str]:
     """Modify found signatures to fix various issues."""
+    if name == "trio.testing._raises_group._ExceptionInfo.type":
+        # This has the type "type[E]", which gets resolved into the property itself.
+        # That means Sphinx can't resolve it. Fix the issue by overwriting with a fully-qualified
+        # name.
+        assert isinstance(obj, property), obj
+        assert isinstance(obj.fget, types.FunctionType), obj.fget
+        assert obj.fget.__annotations__["return"] == "type[E]", obj.fget.__annotations__
+        obj.fget.__annotations__["return"] = "type[~trio.testing._raises_group.E]"
     if signature is not None:
         signature = signature.replace("~_contextvars.Context", "~contextvars.Context")
         if name == "trio.lowlevel.RunVar":  # Typevar is not useful here.
@@ -114,6 +123,13 @@ def autodoc_process_signature(
             # Strip the type from the union, make it look like = ...
             signature = signature.replace(" | type[trio._core._local._NoValue]", "")
             signature = signature.replace("<class 'trio._core._local._NoValue'>", "...")
+        if (
+            name in ("trio.testing.RaisesGroup", "trio.testing.Matcher")
+            and "+E" in signature
+        ):
+            # This typevar being covariant isn't handled correctly in some cases, strip the +
+            # and insert the fully-qualified name.
+            signature = signature.replace("+E", "~trio.testing._raises_group.E")
         if "DTLS" in name:
             signature = signature.replace("SSL.Context", "OpenSSL.SSL.Context")
         # Don't specify PathLike[str] | PathLike[bytes], this is just for humans.

newsfragments/2989.bugfix.rst

@@ -0,0 +1 @@
+Fixed a bug where :class:`trio.testing.RaisesGroup` would check the number of exceptions in the raised `ExceptionGroup` before flattening subgroups, leading to incorrectly failed matches with ``strict=False`` (now known as ``flatten_subgroups=True``).

newsfragments/2989.deprecated.rst

@@ -0,0 +1 @@
+Deprecated ``strict`` parameter from :class:`trio.testing.RaisesGroup`, previous functionality of ``strict=False`` is now in ``flatten_subgroups=True``.

newsfragments/2989.feature.rst

@@ -0,0 +1 @@
+:class:`trio.testing.RaisesGroup` can now catch an unwrapped exception with ``unwrapped=True``. This means that the behaviour of :ref:`except* <except_star>` can be fully replicated in combination with ``flatten_subgroups=True``

src/trio/_tests/test_testing_raisesgroup.py

@@ -78,33 +78,77 @@ def test_raises_group() -> None:
         with RaisesGroup(ValueError, SyntaxError):
             raise ExceptionGroup("", (ValueError(),))
 
+
+def test_flatten_subgroups() -> None:
     # loose semantics, as with expect*
-    with RaisesGroup(ValueError, strict=False):
+    with RaisesGroup(ValueError, flatten_subgroups=True):
         raise ExceptionGroup("", (ExceptionGroup("", (ValueError(),)),))
 
+    with RaisesGroup(ValueError, TypeError, flatten_subgroups=True):
+        raise ExceptionGroup("", (ExceptionGroup("", (ValueError(), TypeError())),))
+    with RaisesGroup(ValueError, TypeError, flatten_subgroups=True):
+        raise ExceptionGroup("", [ExceptionGroup("", [ValueError()]), TypeError()])
+
     # mixed loose is possible if you want it to be at least N deep
-    with RaisesGroup(RaisesGroup(ValueError, strict=False)):
+    with RaisesGroup(RaisesGroup(ValueError, flatten_subgroups=True)):
         raise ExceptionGroup("", (ExceptionGroup("", (ValueError(),)),))
-    with RaisesGroup(RaisesGroup(ValueError, strict=False)):
+    with RaisesGroup(RaisesGroup(ValueError, flatten_subgroups=True)):
         raise ExceptionGroup(
             "", (ExceptionGroup("", (ExceptionGroup("", (ValueError(),)),)),)
         )
     with pytest.raises(ExceptionGroup):
-        with RaisesGroup(RaisesGroup(ValueError, strict=False)):
+        with RaisesGroup(RaisesGroup(ValueError, flatten_subgroups=True)):
             raise ExceptionGroup("", (ValueError(),))
 
     # but not the other way around
     with pytest.raises(
         ValueError,
-        match="^You cannot specify a nested structure inside a RaisesGroup with strict=False$",
+        match="^You cannot specify a nested structure inside a RaisesGroup with",
     ):
-        RaisesGroup(RaisesGroup(ValueError), strict=False)
+        RaisesGroup(RaisesGroup(ValueError), flatten_subgroups=True)
+
 
-    # currently not fully identical in behaviour to expect*, which would also catch an unwrapped exception
-    with pytest.raises(ValueError, match="^value error text$"):
-        with RaisesGroup(ValueError, strict=False):
+def test_catch_unwrapped_exceptions() -> None:
+    # Catches lone exceptions with strict=False
+    # just as except* would
+    with RaisesGroup(ValueError, allow_unwrapped=True):
+        raise ValueError
+
+    # expecting multiple unwrapped exceptions is not possible
+    with pytest.raises(
+        ValueError, match="^You cannot specify multiple exceptions with"
+    ):
+        with RaisesGroup(SyntaxError, ValueError, allow_unwrapped=True):
+            ...
+    # if users want one of several exception types they need to use a Matcher
+    # (which the error message suggests)
+    with RaisesGroup(
+        Matcher(check=lambda e: isinstance(e, (SyntaxError, ValueError))),
+        allow_unwrapped=True,
+    ):
+        raise ValueError
+
+    # Unwrapped nested `RaisesGroup` is likely a user error, so we raise an error.
+    with pytest.raises(ValueError, match="has no effect when expecting"):
+        with RaisesGroup(RaisesGroup(ValueError), allow_unwrapped=True):
+            ...
+    # But it *can* be used to check for nesting level +- 1 if they move it to
+    # the nested RaisesGroup. Users should probably use `Matcher`s instead though.
+    with RaisesGroup(RaisesGroup(ValueError, allow_unwrapped=True)):
+        raise ExceptionGroup("", [ExceptionGroup("", [ValueError()])])
+    with RaisesGroup(RaisesGroup(ValueError, allow_unwrapped=True)):
+        raise ExceptionGroup("", [ValueError()])
+
+    # with allow_unwrapped=False (default) it will not be caught
+    with pytest.raises(ValueError, match="value error text"):
+        with RaisesGroup(ValueError):
             raise ValueError("value error text")
 
+    # code coverage
+    with pytest.raises(TypeError):
+        with RaisesGroup(ValueError, allow_unwrapped=True):
+            raise TypeError
+
 
 def test_match() -> None:
     # supports match string
@@ -260,3 +304,12 @@ def test__ExceptionInfo(monkeypatch: pytest.MonkeyPatch) -> None:
     assert excinfo.type is ExceptionGroup
     assert excinfo.value.exceptions[0].args == ("hello",)
     assert isinstance(excinfo.tb, TracebackType)
+
+
+def test_deprecated_strict() -> None:
+    """`strict` has been replaced with `flatten_subgroups`"""
+    # parameter is typed as `None` to also emit a type error if passing anything
+    with pytest.deprecated_call():
+        RaisesGroup(ValueError, strict=False)  # type: ignore[arg-type]
+    with pytest.deprecated_call():
+        RaisesGroup(ValueError, strict=True)  # type: ignore[arg-type]

src/trio/_tests/type_tests/raisesgroup.py

@@ -84,9 +84,9 @@ def check_filenotfound(exc: FileNotFoundError) -> bool:
     Matcher(exception_type=ValueError)
     Matcher(match="regex")
     Matcher(check=check_exc)
-    Matcher(check=check_filenotfound)  # type: ignore
     Matcher(ValueError, match="regex")
     Matcher(FileNotFoundError, check=check_filenotfound)
+    Matcher(check=check_filenotfound)  # type: ignore # not narrowed
     Matcher(match="regex", check=check_exc)
     Matcher(FileNotFoundError, match="regex", check=check_filenotfound)
 
@@ -134,3 +134,28 @@ def check_nested_raisesgroups_matches() -> None:
     # has the same problems as check_nested_raisesgroups_contextmanager
     if RaisesGroup(RaisesGroup(ValueError)).matches(exc):
         assert_type(exc, BaseExceptionGroup[RaisesGroup[ValueError]])
+
+
+def check_multiple_exceptions_1() -> None:
+    a = RaisesGroup(ValueError, ValueError)
+    b = RaisesGroup(Matcher(ValueError), Matcher(ValueError))
+    c = RaisesGroup(ValueError, Matcher(ValueError))
+
+    d: BaseExceptionGroup[ValueError]
+    d = a
+    d = b
+    d = c
+    assert d
+
+
+def check_multiple_exceptions_2() -> None:
+    # This previously failed due to lack of covariance in the TypeVar
+    a = RaisesGroup(Matcher(ValueError), Matcher(TypeError))
+    b = RaisesGroup(Matcher(ValueError), TypeError)
+    c = RaisesGroup(ValueError, TypeError)
+
+    d: BaseExceptionGroup[Exception]
+    d = a
+    d = b
+    d = c
+    assert d

src/trio/testing/_raises_group.py

@@ -2,13 +2,14 @@
 
 import re
 import sys
+import warnings
 from typing import (
     TYPE_CHECKING,
     Callable,
     ContextManager,
     Generic,
-    Iterable,
     Pattern,
+    Sequence,
     TypeVar,
     cast,
     overload,
@@ -29,7 +30,7 @@
 if sys.version_info < (3, 11):
     from exceptiongroup import BaseExceptionGroup
 
-E = TypeVar("E", bound=BaseException)
+E = TypeVar("E", bound=BaseException, covariant=True)
 
 
 @final
@@ -263,8 +264,20 @@ class RaisesGroup(ContextManager[ExceptionInfo[BaseExceptionGroup[E]]], SuperCla
     This works similar to ``pytest.raises``, and a version of it will hopefully be added upstream, after which this can be deprecated and removed. See https://github.com/pytest-dev/pytest/issues/11538
 
 
-    This differs from :ref:`except* <except_star>` in that all specified exceptions must be present, *and no others*. It will similarly not catch exceptions *not* wrapped in an exceptiongroup.
-    If you don't care for the nesting level of the exceptions you can pass ``strict=False``.
+    The catching behaviour differs from :ref:`except* <except_star>` in multiple different ways, being much stricter by default. By using ``allow_unwrapped=True`` and ``flatten_subgroups=True`` you can match ``except*`` fully when expecting a single exception.
+
+    #. All specified exceptions must be present, *and no others*.
+
+       * If you expect a variable number of exceptions you need to use ``pytest.raises(ExceptionGroup)`` and manually check the contained exceptions. Consider making use of :func:`Matcher.matches`.
+
+    #. It will only catch exceptions wrapped in an exceptiongroup by default.
+
+       * With ``allow_unwrapped=True`` you can specify a single expected exception or `Matcher` and it will match the exception even if it is not inside an `ExceptionGroup`. If you expect one of several different exception types you need to use a `Matcher` object.
+
+    #. By default it cares about the full structure with potentially nested `ExceptionGroup`s. You can specify nested `ExceptionGroup`s by passing `RaisesGroup` objects as expected exceptions.
+
+       * With ``flatten_subgroups=True`` it will "flatten" the raised `ExceptionGroup`, extracting all exceptions inside any nested `ExceptionGroup`s, before matching.
+
     It currently does not care about the order of the exceptions, so ``RaisesGroups(ValueError, TypeError)`` is equivalent to ``RaisesGroups(TypeError, ValueError)``.
 
     This class is not as polished as ``pytest.raises``, and is currently not as helpful in e.g. printing diffs when strings don't match, suggesting you use ``re.escape``, etc.
@@ -280,14 +293,19 @@ class RaisesGroup(ContextManager[ExceptionInfo[BaseExceptionGroup[E]]], SuperCla
         with RaisesGroups(RaisesGroups(ValueError)):
             raise ExceptionGroup("", (ExceptionGroup("", (ValueError(),)),))
 
-        with RaisesGroups(ValueError, strict=False):
+        # flatten_subgroups
+        with RaisesGroups(ValueError, flatten_subgroups=True):
             raise ExceptionGroup("", (ExceptionGroup("", (ValueError(),)),))
 
+        # allow_unwrapped
+        with RaisesGroups(ValueError, allow_unwrapped=True):
+            raise ValueError
+
 
     `RaisesGroup.matches` can also be used directly to check a standalone exception group.
 
 
-    This class is also not perfectly smart, e.g. this will likely fail currently::
+    The matching algorithm is greedy, which means cases such as this may fail::
 
         with RaisesGroups(ValueError, Matcher(ValueError, match="hello")):
             raise ExceptionGroup("", (ValueError("hello"), ValueError("goodbye")))
@@ -307,28 +325,60 @@ def __init__(
         self,
         exception: type[E] | Matcher[E] | E,
         *other_exceptions: type[E] | Matcher[E] | E,
-        strict: bool = True,
+        allow_unwrapped: bool = False,
+        flatten_subgroups: bool = False,
         match: str | Pattern[str] | None = None,
         check: Callable[[BaseExceptionGroup[E]], bool] | None = None,
+        strict: None = None,
     ):
         self.expected_exceptions: tuple[type[E] | Matcher[E] | E, ...] = (
             exception,
             *other_exceptions,
         )
-        self.strict = strict
+        self.flatten_subgroups: bool = flatten_subgroups
+        self.allow_unwrapped = allow_unwrapped
         self.match_expr = match
         self.check = check
         self.is_baseexceptiongroup = False
 
+        if strict is not None:
+            warnings.warn(
+                DeprecationWarning(
+                    "`strict=False` has been replaced with `flatten_subgroups=True`"
+                    " with the introduction of `allow_unwrapped` as a parameter."
+                ),
+                stacklevel=2,
+            )
+            self.flatten_subgroups = not strict
+
+        if allow_unwrapped and other_exceptions:
+            raise ValueError(
+                "You cannot specify multiple exceptions with `allow_unwrapped=True.`"
+                " If you want to match one of multiple possible exceptions you should"
+                " use a `Matcher`."
+                " E.g. `Matcher(check=lambda e: isinstance(e, (...)))`"
+            )
+        if allow_unwrapped and isinstance(exception, RaisesGroup):
+            raise ValueError(
+                "`allow_unwrapped=True` has no effect when expecting a `RaisesGroup`."
+                " You might want it in the expected `RaisesGroup`, or"
+                " `flatten_subgroups=True` if you don't care about the structure."
+            )
+
+        # verify `expected_exceptions` and set `self.is_baseexceptiongroup`
         for exc in self.expected_exceptions:
             if isinstance(exc, RaisesGroup):
-                if not strict:
+                if self.flatten_subgroups:
                     raise ValueError(
                         "You cannot specify a nested structure inside a RaisesGroup with"
-                        " strict=False"
+                        " `flatten_subgroups=True`. The parameter will flatten subgroups"
+                        " in the raised exceptiongroup before matching, which would never"
+                        " match a nested structure."
                     )
                 self.is_baseexceptiongroup |= exc.is_baseexceptiongroup
             elif isinstance(exc, Matcher):
+                # The Matcher could match BaseExceptions through the other arguments
+                # but `self.is_baseexceptiongroup` is only used for printing.
                 if exc.exception_type is None:
                     continue
                 # Matcher __init__ assures it's a subclass of BaseException
@@ -348,9 +398,9 @@ def __enter__(self) -> ExceptionInfo[BaseExceptionGroup[E]]:
         return self.excinfo
 
     def _unroll_exceptions(
-        self, exceptions: Iterable[BaseException]
-    ) -> Iterable[BaseException]:
-        """Used in non-strict mode."""
+        self, exceptions: Sequence[BaseException]
+    ) -> Sequence[BaseException]:
+        """Used if `flatten_subgroups=True`."""
         res: list[BaseException] = []
         for exc in exceptions:
             if isinstance(exc, BaseExceptionGroup):
@@ -383,32 +433,38 @@ def matches(
         # maybe have a list of strings logging failed matches, that __exit__ can
         # recursively step through and print on a failing match.
         if not isinstance(exc_val, BaseExceptionGroup):
+            if self.allow_unwrapped:
+                exp_exc = self.expected_exceptions[0]
+                if isinstance(exp_exc, Matcher) and exp_exc.matches(exc_val):
+                    return True
+                if isinstance(exp_exc, type) and isinstance(exc_val, exp_exc):
+                    return True
             return False
-        if len(exc_val.exceptions) != len(self.expected_exceptions):
-            return False
+
         if self.match_expr is not None and not re.search(
             self.match_expr, _stringify_exception(exc_val)
         ):
             return False
         if self.check is not None and not self.check(exc_val):
             return False
+
         remaining_exceptions = list(self.expected_exceptions)
-        actual_exceptions: Iterable[BaseException] = exc_val.exceptions
-        if not self.strict:
+        actual_exceptions: Sequence[BaseException] = exc_val.exceptions
+        if self.flatten_subgroups:
             actual_exceptions = self._unroll_exceptions(actual_exceptions)
 
+        # important to check the length *after* flattening subgroups
+        if len(actual_exceptions) != len(self.expected_exceptions):
+            return False
+
         # it should be possible to get RaisesGroup.matches typed so as not to
-        # need these type: ignores, but I'm not sure that's possible while also having it
+        # need type: ignore, but I'm not sure that's possible while also having it
         # transparent for the end user.
         for e in actual_exceptions:
             for rem_e in remaining_exceptions:
                 if (
                     (isinstance(rem_e, type) and isinstance(e, rem_e))
-                    or (
-                        isinstance(e, BaseExceptionGroup)
-                        and isinstance(rem_e, RaisesGroup)
-                        and rem_e.matches(e)
-                    )
+                    or (isinstance(rem_e, RaisesGroup) and rem_e.matches(e))
                     or (isinstance(rem_e, Matcher) and rem_e.matches(e))
                 ):
                     remaining_exceptions.remove(rem_e)  # type: ignore[arg-type]