From 30a7a402fcc970809086f637af882a6b546c09a8 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Wed, 6 Dec 2023 17:48:06 +0100 Subject: [PATCH 01/29] Add RaisesGroup, a helper for catching ExceptionGroups in tests --- src/trio/_core/_tests/test_run.py | 103 +++----- .../test_highlevel_open_tcp_listeners.py | 1 + .../_tests/test_highlevel_open_tcp_stream.py | 9 +- src/trio/_tests/test_testing_raisesgroup.py | 242 ++++++++++++++++++ src/trio/testing/__init__.py | 1 + src/trio/testing/_raises_group.py | 231 +++++++++++++++++ 6 files changed, 516 insertions(+), 71 deletions(-) create mode 100644 src/trio/_tests/test_testing_raisesgroup.py create mode 100644 src/trio/testing/_raises_group.py diff --git a/src/trio/_core/_tests/test_run.py b/src/trio/_core/_tests/test_run.py index 5bd98f0e91..d2449da2d6 100644 --- a/src/trio/_core/_tests/test_run.py +++ b/src/trio/_core/_tests/test_run.py @@ -17,10 +17,15 @@ import sniffio from ... import _core -from ..._core._multierror import MultiError, NonBaseMultiError from ..._threads import to_thread_run_sync from ..._timeouts import fail_after, sleep -from ...testing import Sequencer, assert_checkpoints, wait_all_tasks_blocked +from ...testing import ( + Matcher, + RaisesGroup, + Sequencer, + assert_checkpoints, + wait_all_tasks_blocked, +) from .._run import DEADLINE_HEAP_MIN_PRUNE_THRESHOLD from .tutil import ( buggy_pypy_asyncgens, @@ -195,13 +200,8 @@ async def main() -> NoReturn: nursery.start_soon(crasher) raise KeyError - with pytest.raises(MultiError) as excinfo: + with RaisesGroup(ValueError, KeyError): _core.run(main) - print(excinfo.value) - assert {type(exc) for exc in excinfo.value.exceptions} == { - ValueError, - KeyError, - } def test_two_child_crashes() -> None: @@ -213,12 +213,8 @@ async def main() -> None: nursery.start_soon(crasher, KeyError) nursery.start_soon(crasher, ValueError) - with pytest.raises(MultiError) as excinfo: + with RaisesGroup(ValueError, KeyError): _core.run(main) - assert {type(exc) for exc in excinfo.value.exceptions} == { - ValueError, - KeyError, - } async def test_child_crash_wakes_parent() -> None: @@ -437,7 +433,13 @@ async def crasher() -> NoReturn: # KeyError from crasher() with pytest.raises(KeyError): with _core.CancelScope() as outer: - try: + # Since the outer scope became cancelled before the + # nursery block exited, all cancellations inside the + # nursery block continue propagating to reach the + # outer scope. + with RaisesGroup( + _core.Cancelled, _core.Cancelled, _core.Cancelled, KeyError + ) as excinfo: async with _core.open_nursery() as nursery: # Two children that get cancelled by the nursery scope nursery.start_soon(sleep_forever) # t1 @@ -451,20 +453,7 @@ async def crasher() -> NoReturn: # And one that raises a different error nursery.start_soon(crasher) # t4 # and then our __aexit__ also receives an outer Cancelled - except MultiError as multi_exc: - # Since the outer scope became cancelled before the - # nursery block exited, all cancellations inside the - # nursery block continue propagating to reach the - # outer scope. - assert len(multi_exc.exceptions) == 4 - summary: dict[type, int] = {} - for exc in multi_exc.exceptions: - summary.setdefault(type(exc), 0) - summary[type(exc)] += 1 - assert summary == {_core.Cancelled: 3, KeyError: 1} - raise - else: - raise AssertionError("No ExceptionGroup") + raise excinfo.value async def test_precancelled_task() -> None: @@ -784,14 +773,15 @@ async def task2() -> None: RuntimeError, match="which had already been exited" ) as exc_info: await nursery_mgr.__aexit__(*sys.exc_info()) - assert type(exc_info.value.__context__) is NonBaseMultiError - assert len(exc_info.value.__context__.exceptions) == 3 - cancelled_in_context = False - for exc in exc_info.value.__context__.exceptions: - assert isinstance(exc, RuntimeError) - assert "closed before the task exited" in str(exc) - cancelled_in_context |= isinstance(exc.__context__, _core.Cancelled) - assert cancelled_in_context # for the sleep_forever + + subexceptions = ( + Matcher(RuntimeError, match="closed before the task exited"), + ) * 3 + assert RaisesGroup(*subexceptions).matches(exc_info.value.__context__) + assert any( + isinstance(exc.__context__, _core.Cancelled) + for exc in exc_info.value.__context__.exceptions + ) # for the sleep_forever # Trying to exit a cancel scope from an unrelated task raises an error # without affecting any state @@ -945,11 +935,7 @@ async def main() -> None: with pytest.raises(_core.TrioInternalError) as excinfo: _core.run(main) - me = excinfo.value.__cause__ - assert isinstance(me, MultiError) - assert len(me.exceptions) == 2 - for exc in me.exceptions: - assert isinstance(exc, (KeyError, ValueError)) + assert RaisesGroup(KeyError, ValueError).matches(excinfo.value.__cause__) def test_system_task_crash_plus_Cancelled() -> None: @@ -1202,11 +1188,11 @@ async def test_nursery_exception_chaining_doesnt_make_context_loops() -> None: async def crasher() -> NoReturn: raise KeyError - with pytest.raises(MultiError) as excinfo: + with RaisesGroup(ValueError, KeyError) as excinfo: async with _core.open_nursery() as nursery: nursery.start_soon(crasher) raise ValueError - # the MultiError should not have the KeyError or ValueError as context + # the ExceptionGroup should not have the KeyError or ValueError as context assert excinfo.value.__context__ is None @@ -1963,11 +1949,10 @@ async def test_nursery_stop_iteration() -> None: async def fail() -> NoReturn: raise ValueError - with pytest.raises(ExceptionGroup) as excinfo: + with RaisesGroup(StopIteration, ValueError): async with _core.open_nursery() as nursery: nursery.start_soon(fail) raise StopIteration - assert tuple(map(type, excinfo.value.exceptions)) == (StopIteration, ValueError) async def test_nursery_stop_async_iteration() -> None: @@ -2507,13 +2492,9 @@ async def main() -> NoReturn: async with _core.open_nursery(): raise Exception("foo") - with pytest.raises(MultiError) as exc: + with RaisesGroup(Matcher(Exception, match="^foo$")): _core.run(main, strict_exception_groups=True) - assert len(exc.value.exceptions) == 1 - assert type(exc.value.exceptions[0]) is Exception - assert exc.value.exceptions[0].args == ("foo",) - def test_run_strict_exception_groups_nursery_override() -> None: """ @@ -2531,14 +2512,10 @@ async def main() -> NoReturn: async def test_nursery_strict_exception_groups() -> None: """Test that strict exception groups can be enabled on a per-nursery basis.""" - with pytest.raises(MultiError) as exc: + with RaisesGroup(Matcher(Exception, match="^foo$")): async with _core.open_nursery(strict_exception_groups=True): raise Exception("foo") - assert len(exc.value.exceptions) == 1 - assert type(exc.value.exceptions[0]) is Exception - assert exc.value.exceptions[0].args == ("foo",) - async def test_nursery_collapse_strict() -> None: """ @@ -2549,7 +2526,7 @@ async def test_nursery_collapse_strict() -> None: async def raise_error() -> NoReturn: raise RuntimeError("test error") - with pytest.raises(MultiError) as exc: + with RaisesGroup(RuntimeError, RaisesGroup(RuntimeError)): async with _core.open_nursery() as nursery: nursery.start_soon(sleep_forever) nursery.start_soon(raise_error) @@ -2558,13 +2535,6 @@ async def raise_error() -> NoReturn: nursery2.start_soon(raise_error) nursery.cancel_scope.cancel() - exceptions = exc.value.exceptions - assert len(exceptions) == 2 - assert isinstance(exceptions[0], RuntimeError) - assert isinstance(exceptions[1], MultiError) - assert len(exceptions[1].exceptions) == 1 - assert isinstance(exceptions[1].exceptions[0], RuntimeError) - async def test_nursery_collapse_loose() -> None: """ @@ -2575,7 +2545,7 @@ async def test_nursery_collapse_loose() -> None: async def raise_error() -> NoReturn: raise RuntimeError("test error") - with pytest.raises(MultiError) as exc: + with RaisesGroup(RuntimeError, RuntimeError): async with _core.open_nursery() as nursery: nursery.start_soon(sleep_forever) nursery.start_soon(raise_error) @@ -2584,11 +2554,6 @@ async def raise_error() -> NoReturn: nursery2.start_soon(raise_error) nursery.cancel_scope.cancel() - exceptions = exc.value.exceptions - assert len(exceptions) == 2 - assert isinstance(exceptions[0], RuntimeError) - assert isinstance(exceptions[1], RuntimeError) - async def test_cancel_scope_no_cancellederror() -> None: """ diff --git a/src/trio/_tests/test_highlevel_open_tcp_listeners.py b/src/trio/_tests/test_highlevel_open_tcp_listeners.py index 23d6f794e0..b9efc26a72 100644 --- a/src/trio/_tests/test_highlevel_open_tcp_listeners.py +++ b/src/trio/_tests/test_highlevel_open_tcp_listeners.py @@ -327,6 +327,7 @@ async def test_open_tcp_listeners_some_address_families_unavailable( await open_tcp_listeners(80, host="example.org") assert "This system doesn't support" in str(exc_info.value) + # TODO: remove the `else` with strict_exception_groups=True if isinstance(exc_info.value.__cause__, BaseExceptionGroup): for subexc in exc_info.value.__cause__.exceptions: assert "nope" in str(subexc) diff --git a/src/trio/_tests/test_highlevel_open_tcp_stream.py b/src/trio/_tests/test_highlevel_open_tcp_stream.py index 79dd8b0f78..3e27964440 100644 --- a/src/trio/_tests/test_highlevel_open_tcp_stream.py +++ b/src/trio/_tests/test_highlevel_open_tcp_stream.py @@ -16,6 +16,7 @@ reorder_for_rfc_6555_section_5_4, ) from trio.socket import AF_INET, AF_INET6, IPPROTO_TCP, SOCK_STREAM, SocketType +from trio.testing import Matcher, RaisesGroup if TYPE_CHECKING: from trio.testing import MockClock @@ -506,8 +507,12 @@ async def test_all_fail(autojump_clock: MockClock) -> None: expect_error=OSError, ) assert isinstance(exc, OSError) - assert isinstance(exc.__cause__, BaseExceptionGroup) - assert len(exc.__cause__.exceptions) == 4 + + subexceptions = (Matcher(OSError, match="^sorry$"),) * 4 + assert RaisesGroup( + *subexceptions, match="all attempts to connect to test.example.com:80 failed" + ).matches(exc.__cause__) + assert trio.current_time() == (0.1 + 0.2 + 10) assert scenario.connect_times == { "1.1.1.1": 0, diff --git a/src/trio/_tests/test_testing_raisesgroup.py b/src/trio/_tests/test_testing_raisesgroup.py new file mode 100644 index 0000000000..5ceedb6de8 --- /dev/null +++ b/src/trio/_tests/test_testing_raisesgroup.py @@ -0,0 +1,242 @@ +from __future__ import annotations + +import re +import sys +from typing import TYPE_CHECKING + +import pytest + +from trio.testing import Matcher, RaisesGroup + +# TODO: make a public export + +if TYPE_CHECKING: + from typing_extensions import assert_type + +if sys.version_info < (3, 11): + from exceptiongroup import BaseExceptionGroup, ExceptionGroup + + +class TestRaisesGroup: + def test_raises_group(self) -> None: + with pytest.raises( + ValueError, + match="^Invalid argument {exc} must be exception type, Matcher, or RaisesGroup.$", + ): + RaisesGroup(ValueError()) + + with RaisesGroup(ValueError): + raise ExceptionGroup("foo", (ValueError(),)) + + with RaisesGroup(SyntaxError): + with RaisesGroup(ValueError): + raise ExceptionGroup("foo", (SyntaxError(),)) + + # multiple exceptions + with RaisesGroup(ValueError, SyntaxError): + raise ExceptionGroup("foo", (ValueError(), SyntaxError())) + + # order doesn't matter + with RaisesGroup(SyntaxError, ValueError): + raise ExceptionGroup("foo", (ValueError(), SyntaxError())) + + # nested exceptions + with RaisesGroup(RaisesGroup(ValueError)): + raise ExceptionGroup("foo", (ExceptionGroup("bar", (ValueError(),)),)) + + with RaisesGroup( + SyntaxError, + RaisesGroup(ValueError), + RaisesGroup(RuntimeError), + ): + raise ExceptionGroup( + "foo", + ( + SyntaxError(), + ExceptionGroup("bar", (ValueError(),)), + ExceptionGroup("", (RuntimeError(),)), + ), + ) + + # will error if there's excess exceptions + with pytest.raises(ExceptionGroup): + with RaisesGroup(ValueError): + raise ExceptionGroup("", (ValueError(), ValueError())) + + with pytest.raises(ExceptionGroup): + with RaisesGroup(ValueError): + raise ExceptionGroup("", (RuntimeError(), ValueError())) + + # will error if there's missing exceptions + with pytest.raises(ExceptionGroup): + with RaisesGroup(ValueError, ValueError): + raise ExceptionGroup("", (ValueError(),)) + + with pytest.raises(ExceptionGroup): + with RaisesGroup(ValueError, SyntaxError): + raise ExceptionGroup("", (ValueError(),)) + + # loose semantics, as with expect* + with RaisesGroup(ValueError, strict=False): + raise ExceptionGroup("", (ExceptionGroup("", (ValueError(),)),)) + + # mixed loose is possible if you want it to be at least N deep + with RaisesGroup(RaisesGroup(ValueError, strict=True)): + raise ExceptionGroup("", (ExceptionGroup("", (ValueError(),)),)) + with RaisesGroup(RaisesGroup(ValueError, strict=False)): + raise ExceptionGroup( + "", (ExceptionGroup("", (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$", + ): + RaisesGroup(RaisesGroup(ValueError), strict=False) + + # currently not fully identical in behaviour to expect*, which would also catch an unwrapped exception + with pytest.raises(ValueError): + with RaisesGroup(ValueError, strict=False): + raise ValueError + + def test_match(self) -> None: + # supports match string + with RaisesGroup(ValueError, match="bar"): + raise ExceptionGroup("bar", (ValueError(),)) + + with pytest.raises(ExceptionGroup): + with RaisesGroup(ValueError, match="foo"): + raise ExceptionGroup("bar", (ValueError(),)) + + def test_check(self) -> None: + exc = ExceptionGroup("", (ValueError(),)) + with RaisesGroup(ValueError, check=lambda x: x is exc): + raise exc + with pytest.raises(ExceptionGroup): + with RaisesGroup(ValueError, check=lambda x: x is exc): + raise ExceptionGroup("", (ValueError(),)) + + def test_RaisesGroup_matches(self) -> None: + eeg = RaisesGroup(ValueError) + assert not eeg.matches(None) + assert not eeg.matches(ValueError()) + assert eeg.matches(ExceptionGroup("", (ValueError(),))) + + def test_message(self) -> None: + with pytest.raises( + AssertionError, + match=re.escape( + f"DID NOT RAISE any exception, expected ExceptionGroup({ValueError!r},)" + ), + ): + with RaisesGroup(ValueError): + ... + with pytest.raises( + AssertionError, + match=re.escape( + f"DID NOT RAISE any exception, expected ExceptionGroup(ExceptionGroup({ValueError!r},),)" + ), + ): + with RaisesGroup(RaisesGroup(ValueError)): + ... + + def test_matcher(self) -> None: + with pytest.raises( + ValueError, match="^You must specify at least one parameter to match on.$" + ): + Matcher() + + with RaisesGroup(Matcher(ValueError)): + raise ExceptionGroup("", (ValueError(),)) + with pytest.raises(ExceptionGroup): + with RaisesGroup(Matcher(TypeError)): + raise ExceptionGroup("", (ValueError(),)) + + def test_matcher_match(self) -> None: + with RaisesGroup(Matcher(ValueError, "foo")): + raise ExceptionGroup("", (ValueError("foo"),)) + with pytest.raises(ExceptionGroup): + with RaisesGroup(Matcher(ValueError, "foo")): + raise ExceptionGroup("", (ValueError("bar"),)) + + # Can be used without specifying the type + with RaisesGroup(Matcher(match="foo")): + raise ExceptionGroup("", (ValueError("foo"),)) + with pytest.raises(ExceptionGroup): + with RaisesGroup(Matcher(match="foo")): + raise ExceptionGroup("", (ValueError("bar"),)) + + def test_Matcher_check(self) -> None: + def check_oserror_and_errno_is_5(e: BaseException) -> bool: + return isinstance(e, OSError) and e.errno == 5 + + with RaisesGroup(Matcher(check=check_oserror_and_errno_is_5)): + raise ExceptionGroup("", (OSError(5, ""),)) + + # specifying exception_type narrows the parameter type to the callable + def check_errno_is_5(e: OSError) -> bool: + return e.errno == 5 + + with RaisesGroup(Matcher(OSError, check=check_errno_is_5)): + raise ExceptionGroup("", (OSError(5, ""),)) + + with pytest.raises(ExceptionGroup): + with RaisesGroup(Matcher(OSError, check=check_errno_is_5)): + raise ExceptionGroup("", (OSError(6, ""),)) + + if TYPE_CHECKING: + # getting the typing working satisfactory is very tricky + # but with RaisesGroup being seen as a subclass of BaseExceptionGroup + # most end-user cases of checking excinfo.value.foobar should work fine now. + def test_types_0(self) -> None: + _: BaseExceptionGroup[ValueError] = RaisesGroup(ValueError) + _ = RaisesGroup(RaisesGroup(ValueError)) # type: ignore[arg-type] + a: BaseExceptionGroup[BaseExceptionGroup[ValueError]] + a = RaisesGroup(RaisesGroup(ValueError)) + a = BaseExceptionGroup("", (BaseExceptionGroup("", (ValueError(),)),)) + assert a + + def test_types_1(self) -> None: + with RaisesGroup(ValueError) as e: + raise ExceptionGroup("foo", (ValueError(),)) + assert_type(e.value, BaseExceptionGroup[ValueError]) + # assert_type(e.value, RaisesGroup[ValueError]) + + def test_types_2(self) -> None: + exc: ExceptionGroup[ValueError] | ValueError = ExceptionGroup( + "", (ValueError(),) + ) + if RaisesGroup(ValueError).matches(exc): + assert_type(exc, BaseExceptionGroup[ValueError]) + + def test_types_3(self) -> None: + e: BaseExceptionGroup[KeyboardInterrupt] = BaseExceptionGroup( + "", (KeyboardInterrupt(),) + ) + if RaisesGroup(ValueError).matches(e): + assert_type(e, BaseExceptionGroup[ValueError]) + + def test_types_4(self) -> None: + with RaisesGroup(Matcher(ValueError)) as e: + ... + _: BaseExceptionGroup[ValueError] = e.value + assert_type(e.value, BaseExceptionGroup[ValueError]) + + def test_types_5(self) -> None: + with RaisesGroup(RaisesGroup(ValueError)) as excinfo: + raise ExceptionGroup("foo", (ValueError(),)) + _: BaseExceptionGroup[BaseExceptionGroup[ValueError]] = excinfo.value + assert_type( + excinfo.value, + BaseExceptionGroup[RaisesGroup[ValueError]], + ) + print(excinfo.value.exceptions[0].exceptions[0]) + + def test_types_6(self) -> None: + exc: ExceptionGroup[ExceptionGroup[ValueError]] = ExceptionGroup( + "", (ExceptionGroup("", (ValueError(),)),) + ) + if RaisesGroup(RaisesGroup(ValueError)).matches(exc): + # ugly + assert_type(exc, BaseExceptionGroup[RaisesGroup[ValueError]]) diff --git a/src/trio/testing/__init__.py b/src/trio/testing/__init__.py index fa683e1145..5437d6b5dd 100644 --- a/src/trio/testing/__init__.py +++ b/src/trio/testing/__init__.py @@ -24,6 +24,7 @@ memory_stream_pump as memory_stream_pump, ) from ._network import open_stream_to_socket_listener as open_stream_to_socket_listener +from ._raises_group import Matcher, RaisesGroup from ._sequencer import Sequencer as Sequencer from ._trio_test import trio_test as trio_test diff --git a/src/trio/testing/_raises_group.py b/src/trio/testing/_raises_group.py new file mode 100644 index 0000000000..905aea1b58 --- /dev/null +++ b/src/trio/testing/_raises_group.py @@ -0,0 +1,231 @@ +from __future__ import annotations + +import re +import sys +from types import TracebackType +from typing import ( + TYPE_CHECKING, + Callable, + ContextManager, + Generic, + Iterable, + Pattern, + TypeVar, + cast, +) + +from trio._util import final + +if TYPE_CHECKING: + from typing_extensions import TypeGuard + +if sys.version_info < (3, 11): + from exceptiongroup import BaseExceptionGroup + +E = TypeVar("E", bound=BaseException) + + +# minimal version of pytest.ExceptionInfo in case it is not available +@final +class _ExceptionInfo(Generic[E]): + _excinfo: tuple[type[E], E, TracebackType] | None + + def __init__(self, excinfo: tuple[type[E], E, TracebackType] | None): + self._exc_info = excinfo + + def fill_unfilled(self, exc_info: tuple[type[E], E, TracebackType]) -> None: + """Fill an unfilled ExceptionInfo created with ``for_later()``.""" + assert self._excinfo is None, "ExceptionInfo was already filled" + self._excinfo = exc_info + + @classmethod + def for_later(cls) -> _ExceptionInfo[E]: + """Return an unfilled ExceptionInfo.""" + return cls(None) + + +try: + from pytest import ExceptionInfo +except ImportError: + ExceptionInfo = _ExceptionInfo # type: ignore[misc, assignment] + + +# copied from pytest.ExceptionInfo +def _stringify_exception(exc: BaseException) -> str: + return "\n".join( + [ + str(exc), + *getattr(exc, "__notes__", []), + ] + ) + + +@final +class Matcher(Generic[E]): + def __init__( + self, + exception_type: type[E] | None = None, + match: str | Pattern[str] | None = None, + check: Callable[[E], bool] | None = None, + ): + if exception_type is None and match is None and check is None: + raise ValueError("You must specify at least one parameter to match on.") + self.exception_type = exception_type + self.match = match + self.check = check + + def matches(self, exception: E) -> TypeGuard[E]: + if self.exception_type is not None and not isinstance( + exception, self.exception_type + ): + return False + if self.match is not None and not re.search( + self.match, _stringify_exception(exception) + ): + return False + if self.check is not None and not self.check(exception): + return False + return True + + +# typing this has been somewhat of a nightmare, with the primary difficulty making +# the return type of __enter__ correct. Ideally it would function like this +# with RaisesGroup(RaisesGroup(ValueError)) as excinfo: +# ... +# assert_type(excinfo.value, ExceptionGroup[ExceptionGroup[ValueError]]) +# in addition to all the simple cases, but getting all the way to the above seems maybe +# impossible. The type being RaisesGroup[RaisesGroup[ValueError]] is probably also fine, +# as long as I add fake properties corresponding to the properties of exceptiongroup. But +# I had trouble with it handling recursive cases properly. + +# Current solution settles on the above giving BaseExceptionGroup[RaisesGroup[ValueError]], and it not +# being a type error to do `with RaisesGroup(ValueError()): ...` - but that will error on runtime. +if TYPE_CHECKING: + SuperClass = BaseExceptionGroup +else: + SuperClass = Generic + + +@final +class RaisesGroup(ContextManager[ExceptionInfo[BaseExceptionGroup[E]]], SuperClass[E]): + def __init__( + self, + exceptions: type[E] | Matcher[E] | E, + *args: type[E] | Matcher[E] | E, + strict: bool = True, + match: str | Pattern[str] | None = None, + check: Callable[[BaseExceptionGroup[E]], bool] | None = None, + ): + self.expected_exceptions = (exceptions, *args) + self.strict = strict + self.match_expr = match + # message is read-only in BaseExceptionGroup, which we lie to mypy we inherit from + self.check = check + + for exc in self.expected_exceptions: + if not isinstance(exc, (Matcher, RaisesGroup)) and not ( + isinstance(exc, type) and issubclass(exc, BaseException) + ): + raise ValueError( + "Invalid argument {exc} must be exception type, Matcher, or" + " RaisesGroup." + ) + if isinstance(exc, RaisesGroup) and not strict: + raise ValueError( + "You cannot specify a nested structure inside a RaisesGroup with" + " strict=False" + ) + + def __enter__(self) -> ExceptionInfo[BaseExceptionGroup[E]]: + self.excinfo: ExceptionInfo[BaseExceptionGroup[E]] = ExceptionInfo.for_later() + return self.excinfo + + def _unroll_exceptions( + self, exceptions: Iterable[BaseException] + ) -> Iterable[BaseException]: + """Used in non-strict mode.""" + res: list[BaseException] = [] + for exc in exceptions: + if isinstance(exc, BaseExceptionGroup): + res.extend(self._unroll_exceptions(exc.exceptions)) + + else: + res.append(exc) + return res + + def matches( + self, + exc_val: BaseException | None, + ) -> TypeGuard[BaseExceptionGroup[E]]: + if exc_val is None: + return False + # TODO: print/raise why a match fails, in a way that works properly in nested cases + # 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): + 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 = self._unroll_exceptions(actual_exceptions) + + # 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 + # 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, Matcher) + and rem_e.matches(e) # type: ignore[arg-type] + ) + ): + remaining_exceptions.remove(rem_e) # type: ignore[arg-type] + break + else: + return False + return True + + def __exit__( + self, + exc_type: type[BaseException] | None, + exc_val: BaseException | None, + exc_tb: TracebackType | None, + ) -> bool: + __tracebackhide__ = True + assert exc_type is not None, ( + "DID NOT RAISE any exception, expected" + f" ExceptionGroup{self.expected_exceptions!r}" + ) + assert ( + self.excinfo is not None + ), "Internal error - should have been constructed in __enter__" + + if not self.matches(exc_val): + return False + + # Cast to narrow the exception type now that it's verified. + exc_info = cast( + tuple[type[BaseExceptionGroup[E]], BaseExceptionGroup[E], TracebackType], + (exc_type, exc_val, exc_tb), + ) + self.excinfo.fill_unfilled(exc_info) + return True + + def __repr__(self) -> str: + # TODO: [Base]ExceptionGroup + return f"ExceptionGroup{self.expected_exceptions}" From 0d08e19f9a2daeaf84a0bd6cd789c5aacb52def0 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Wed, 6 Dec 2023 18:07:50 +0100 Subject: [PATCH 02/29] fix some CI test errors, now only some grumpy export tests left --- src/trio/testing/__init__.py | 2 +- src/trio/testing/_raises_group.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/trio/testing/__init__.py b/src/trio/testing/__init__.py index 5437d6b5dd..f5dc97f0cd 100644 --- a/src/trio/testing/__init__.py +++ b/src/trio/testing/__init__.py @@ -24,7 +24,7 @@ memory_stream_pump as memory_stream_pump, ) from ._network import open_stream_to_socket_listener as open_stream_to_socket_listener -from ._raises_group import Matcher, RaisesGroup +from ._raises_group import Matcher as Matcher, RaisesGroup as RaisesGroup from ._sequencer import Sequencer as Sequencer from ._trio_test import trio_test as trio_test diff --git a/src/trio/testing/_raises_group.py b/src/trio/testing/_raises_group.py index 905aea1b58..469d7a932f 100644 --- a/src/trio/testing/_raises_group.py +++ b/src/trio/testing/_raises_group.py @@ -2,7 +2,6 @@ import re import sys -from types import TracebackType from typing import ( TYPE_CHECKING, Callable, @@ -17,6 +16,8 @@ from trio._util import final if TYPE_CHECKING: + from types import TracebackType + from typing_extensions import TypeGuard if sys.version_info < (3, 11): @@ -220,7 +221,7 @@ def __exit__( # Cast to narrow the exception type now that it's verified. exc_info = cast( - tuple[type[BaseExceptionGroup[E]], BaseExceptionGroup[E], TracebackType], + "tuple[type[BaseExceptionGroup[E]], BaseExceptionGroup[E], TracebackType]", (exc_type, exc_val, exc_tb), ) self.excinfo.fill_unfilled(exc_info) From 8f1e221558376a3827bbb1645f724e29b26b3231 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Wed, 6 Dec 2023 18:22:15 +0100 Subject: [PATCH 03/29] clarify mixed loose test --- src/trio/_tests/test_testing_raisesgroup.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/trio/_tests/test_testing_raisesgroup.py b/src/trio/_tests/test_testing_raisesgroup.py index 5ceedb6de8..013aceae61 100644 --- a/src/trio/_tests/test_testing_raisesgroup.py +++ b/src/trio/_tests/test_testing_raisesgroup.py @@ -81,12 +81,15 @@ def test_raises_group(self) -> None: raise ExceptionGroup("", (ExceptionGroup("", (ValueError(),)),)) # mixed loose is possible if you want it to be at least N deep - with RaisesGroup(RaisesGroup(ValueError, strict=True)): + with RaisesGroup(RaisesGroup(ValueError, strict=False)): raise ExceptionGroup("", (ExceptionGroup("", (ValueError(),)),)) with RaisesGroup(RaisesGroup(ValueError, strict=False)): raise ExceptionGroup( "", (ExceptionGroup("", (ExceptionGroup("", (ValueError(),)),)),) ) + with pytest.raises(ExceptionGroup): + with RaisesGroup(RaisesGroup(ValueError, strict=False)): + raise ExceptionGroup("", (ValueError(),)) # but not the other way around with pytest.raises( From f7ec07926f845965b28023dc89598db09c36c115 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Wed, 6 Dec 2023 18:22:50 +0100 Subject: [PATCH 04/29] rename variable to fit new class name --- src/trio/_tests/test_testing_raisesgroup.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/trio/_tests/test_testing_raisesgroup.py b/src/trio/_tests/test_testing_raisesgroup.py index 013aceae61..400d018e00 100644 --- a/src/trio/_tests/test_testing_raisesgroup.py +++ b/src/trio/_tests/test_testing_raisesgroup.py @@ -121,10 +121,10 @@ def test_check(self) -> None: raise ExceptionGroup("", (ValueError(),)) def test_RaisesGroup_matches(self) -> None: - eeg = RaisesGroup(ValueError) - assert not eeg.matches(None) - assert not eeg.matches(ValueError()) - assert eeg.matches(ExceptionGroup("", (ValueError(),))) + rg = RaisesGroup(ValueError) + assert not rg.matches(None) + assert not rg.matches(ValueError()) + assert rg.matches(ExceptionGroup("", (ValueError(),))) def test_message(self) -> None: with pytest.raises( From c7c6264f4b42553420fa6164cfbd137254f94fb7 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Sat, 9 Dec 2023 12:57:28 +0100 Subject: [PATCH 05/29] ignore RaisesGroup in test_exports for now --- src/trio/_tests/test_exports.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/trio/_tests/test_exports.py b/src/trio/_tests/test_exports.py index 5c97e76f65..4edf0b0a81 100644 --- a/src/trio/_tests/test_exports.py +++ b/src/trio/_tests/test_exports.py @@ -319,6 +319,10 @@ def lookup_symbol(symbol: str) -> dict[str, str]: if class_name.startswith("_"): # pragma: no cover continue + # ignore class that does dirty tricks + if class_ is trio.testing.RaisesGroup: + continue + # dir() and inspect.getmembers doesn't display properties from the metaclass # also ignore some dunder methods that tend to differ but are of no consequence ignore_names = set(dir(type(class_))) | { From 49af03d3275605ae5e26b69f87d4234e6bb55d35 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Sat, 9 Dec 2023 13:47:40 +0100 Subject: [PATCH 06/29] fix test_export fail --- src/trio/_tests/test_exports.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/trio/_tests/test_exports.py b/src/trio/_tests/test_exports.py index 4edf0b0a81..b51cde2c28 100644 --- a/src/trio/_tests/test_exports.py +++ b/src/trio/_tests/test_exports.py @@ -435,7 +435,9 @@ def lookup_symbol(symbol: str) -> dict[str, str]: if tool == "mypy" and class_ == trio.Nursery: extra.remove("cancel_scope") - # TODO: I'm not so sure about these, but should still be looked at. + # These are (mostly? solely?) *runtime* attributes, often set in + # __init__, which doesn't show up with dir() or inspect.getmembers, + # but we get them in the way we query mypy & jedi EXTRAS = { trio.DTLSChannel: {"peer_address", "endpoint"}, trio.DTLSEndpoint: {"socket", "incoming_packets_buffer"}, @@ -450,6 +452,11 @@ def lookup_symbol(symbol: str) -> dict[str, str]: "send_all_hook", "wait_send_all_might_not_block_hook", }, + trio.testing.Matcher: { + "exception_type", + "match", + "check", + }, } if tool == "mypy" and class_ in EXTRAS: before = len(extra) From 3f630300c725c2b93644174fabae95f086d13baf Mon Sep 17 00:00:00 2001 From: jakkdl Date: Sat, 9 Dec 2023 14:08:25 +0100 Subject: [PATCH 07/29] fix pyright --verifytypes errors --- src/trio/testing/_raises_group.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/trio/testing/_raises_group.py b/src/trio/testing/_raises_group.py index 469d7a932f..127fb42678 100644 --- a/src/trio/testing/_raises_group.py +++ b/src/trio/testing/_raises_group.py @@ -45,10 +45,16 @@ def for_later(cls) -> _ExceptionInfo[E]: return cls(None) -try: +# this may bite users with type checkers not using pytest, but that should +# be rare and give quite obvious errors in tests trying to do so. +if TYPE_CHECKING: from pytest import ExceptionInfo -except ImportError: - ExceptionInfo = _ExceptionInfo # type: ignore[misc, assignment] + +else: + try: + from pytest import ExceptionInfo + except ImportError: + ExceptionInfo = _ExceptionInfo # copied from pytest.ExceptionInfo @@ -117,7 +123,10 @@ def __init__( match: str | Pattern[str] | None = None, check: Callable[[BaseExceptionGroup[E]], bool] | None = None, ): - self.expected_exceptions = (exceptions, *args) + self.expected_exceptions: tuple[type[E] | Matcher[E] | E, ...] = ( + exceptions, + *args, + ) self.strict = strict self.match_expr = match # message is read-only in BaseExceptionGroup, which we lie to mypy we inherit from From 3adc49ac2e3227b58ebccac9cd0ff5c1866dd0b5 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Sat, 9 Dec 2023 14:10:34 +0100 Subject: [PATCH 08/29] un-unittest the tests --- src/trio/_tests/test_testing_raisesgroup.py | 409 ++++++++++---------- 1 file changed, 208 insertions(+), 201 deletions(-) diff --git a/src/trio/_tests/test_testing_raisesgroup.py b/src/trio/_tests/test_testing_raisesgroup.py index 400d018e00..274484aad6 100644 --- a/src/trio/_tests/test_testing_raisesgroup.py +++ b/src/trio/_tests/test_testing_raisesgroup.py @@ -17,229 +17,236 @@ from exceptiongroup import BaseExceptionGroup, ExceptionGroup -class TestRaisesGroup: - def test_raises_group(self) -> None: - with pytest.raises( - ValueError, - match="^Invalid argument {exc} must be exception type, Matcher, or RaisesGroup.$", - ): - RaisesGroup(ValueError()) +def test_raises_group() -> None: + with pytest.raises( + ValueError, + match="^Invalid argument {exc} must be exception type, Matcher, or RaisesGroup.$", + ): + RaisesGroup(ValueError()) + with RaisesGroup(ValueError): + raise ExceptionGroup("foo", (ValueError(),)) + + with RaisesGroup(SyntaxError): with RaisesGroup(ValueError): - raise ExceptionGroup("foo", (ValueError(),)) + raise ExceptionGroup("foo", (SyntaxError(),)) + + # multiple exceptions + with RaisesGroup(ValueError, SyntaxError): + raise ExceptionGroup("foo", (ValueError(), SyntaxError())) + + # order doesn't matter + with RaisesGroup(SyntaxError, ValueError): + raise ExceptionGroup("foo", (ValueError(), SyntaxError())) + + # nested exceptions + with RaisesGroup(RaisesGroup(ValueError)): + raise ExceptionGroup("foo", (ExceptionGroup("bar", (ValueError(),)),)) + + with RaisesGroup( + SyntaxError, + RaisesGroup(ValueError), + RaisesGroup(RuntimeError), + ): + raise ExceptionGroup( + "foo", + ( + SyntaxError(), + ExceptionGroup("bar", (ValueError(),)), + ExceptionGroup("", (RuntimeError(),)), + ), + ) - with RaisesGroup(SyntaxError): - with RaisesGroup(ValueError): - raise ExceptionGroup("foo", (SyntaxError(),)) + # will error if there's excess exceptions + with pytest.raises(ExceptionGroup): + with RaisesGroup(ValueError): + raise ExceptionGroup("", (ValueError(), ValueError())) - # multiple exceptions + with pytest.raises(ExceptionGroup): + with RaisesGroup(ValueError): + raise ExceptionGroup("", (RuntimeError(), ValueError())) + + # will error if there's missing exceptions + with pytest.raises(ExceptionGroup): + with RaisesGroup(ValueError, ValueError): + raise ExceptionGroup("", (ValueError(),)) + + with pytest.raises(ExceptionGroup): with RaisesGroup(ValueError, SyntaxError): - raise ExceptionGroup("foo", (ValueError(), SyntaxError())) + raise ExceptionGroup("", (ValueError(),)) - # order doesn't matter - with RaisesGroup(SyntaxError, ValueError): - raise ExceptionGroup("foo", (ValueError(), SyntaxError())) + # loose semantics, as with expect* + with RaisesGroup(ValueError, strict=False): + raise ExceptionGroup("", (ExceptionGroup("", (ValueError(),)),)) + + # mixed loose is possible if you want it to be at least N deep + with RaisesGroup(RaisesGroup(ValueError, strict=False)): + raise ExceptionGroup("", (ExceptionGroup("", (ValueError(),)),)) + with RaisesGroup(RaisesGroup(ValueError, strict=False)): + raise ExceptionGroup( + "", (ExceptionGroup("", (ExceptionGroup("", (ValueError(),)),)),) + ) + with pytest.raises(ExceptionGroup): + with RaisesGroup(RaisesGroup(ValueError, strict=False)): + raise ExceptionGroup("", (ValueError(),)) - # nested exceptions - with RaisesGroup(RaisesGroup(ValueError)): - raise ExceptionGroup("foo", (ExceptionGroup("bar", (ValueError(),)),)) - - with RaisesGroup( - SyntaxError, - RaisesGroup(ValueError), - RaisesGroup(RuntimeError), - ): - raise ExceptionGroup( - "foo", - ( - SyntaxError(), - ExceptionGroup("bar", (ValueError(),)), - ExceptionGroup("", (RuntimeError(),)), - ), - ) - - # will error if there's excess exceptions - with pytest.raises(ExceptionGroup): - with RaisesGroup(ValueError): - raise ExceptionGroup("", (ValueError(), ValueError())) - - with pytest.raises(ExceptionGroup): - with RaisesGroup(ValueError): - raise ExceptionGroup("", (RuntimeError(), ValueError())) - - # will error if there's missing exceptions - with pytest.raises(ExceptionGroup): - with RaisesGroup(ValueError, ValueError): - raise ExceptionGroup("", (ValueError(),)) - - with pytest.raises(ExceptionGroup): - with RaisesGroup(ValueError, SyntaxError): - raise ExceptionGroup("", (ValueError(),)) - - # loose semantics, as with expect* + # but not the other way around + with pytest.raises( + ValueError, + match="^You cannot specify a nested structure inside a RaisesGroup with strict=False$", + ): + RaisesGroup(RaisesGroup(ValueError), strict=False) + + # currently not fully identical in behaviour to expect*, which would also catch an unwrapped exception + with pytest.raises(ValueError): with RaisesGroup(ValueError, strict=False): - raise ExceptionGroup("", (ExceptionGroup("", (ValueError(),)),)) + raise ValueError - # mixed loose is possible if you want it to be at least N deep - with RaisesGroup(RaisesGroup(ValueError, strict=False)): - raise ExceptionGroup("", (ExceptionGroup("", (ValueError(),)),)) - with RaisesGroup(RaisesGroup(ValueError, strict=False)): - raise ExceptionGroup( - "", (ExceptionGroup("", (ExceptionGroup("", (ValueError(),)),)),) - ) - with pytest.raises(ExceptionGroup): - with RaisesGroup(RaisesGroup(ValueError, strict=False)): - 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$", - ): - RaisesGroup(RaisesGroup(ValueError), strict=False) - - # currently not fully identical in behaviour to expect*, which would also catch an unwrapped exception - with pytest.raises(ValueError): - with RaisesGroup(ValueError, strict=False): - raise ValueError - - def test_match(self) -> None: - # supports match string - with RaisesGroup(ValueError, match="bar"): + +def test_match() -> None: + # supports match string + with RaisesGroup(ValueError, match="bar"): + raise ExceptionGroup("bar", (ValueError(),)) + + with pytest.raises(ExceptionGroup): + with RaisesGroup(ValueError, match="foo"): raise ExceptionGroup("bar", (ValueError(),)) - with pytest.raises(ExceptionGroup): - with RaisesGroup(ValueError, match="foo"): - raise ExceptionGroup("bar", (ValueError(),)) - def test_check(self) -> None: - exc = ExceptionGroup("", (ValueError(),)) +def test_check() -> None: + exc = ExceptionGroup("", (ValueError(),)) + with RaisesGroup(ValueError, check=lambda x: x is exc): + raise exc + with pytest.raises(ExceptionGroup): with RaisesGroup(ValueError, check=lambda x: x is exc): - raise exc - with pytest.raises(ExceptionGroup): - with RaisesGroup(ValueError, check=lambda x: x is exc): - raise ExceptionGroup("", (ValueError(),)) - - def test_RaisesGroup_matches(self) -> None: - rg = RaisesGroup(ValueError) - assert not rg.matches(None) - assert not rg.matches(ValueError()) - assert rg.matches(ExceptionGroup("", (ValueError(),))) - - def test_message(self) -> None: - with pytest.raises( - AssertionError, - match=re.escape( - f"DID NOT RAISE any exception, expected ExceptionGroup({ValueError!r},)" - ), - ): - with RaisesGroup(ValueError): - ... - with pytest.raises( - AssertionError, - match=re.escape( - f"DID NOT RAISE any exception, expected ExceptionGroup(ExceptionGroup({ValueError!r},),)" - ), - ): - with RaisesGroup(RaisesGroup(ValueError)): - ... + raise ExceptionGroup("", (ValueError(),)) + + +def test_RaisesGroup_matches() -> None: + rg = RaisesGroup(ValueError) + assert not rg.matches(None) + assert not rg.matches(ValueError()) + assert rg.matches(ExceptionGroup("", (ValueError(),))) + + +def test_message() -> None: + with pytest.raises( + AssertionError, + match=re.escape( + f"DID NOT RAISE any exception, expected ExceptionGroup({ValueError!r},)" + ), + ): + with RaisesGroup(ValueError): + ... + with pytest.raises( + AssertionError, + match=re.escape( + f"DID NOT RAISE any exception, expected ExceptionGroup(ExceptionGroup({ValueError!r},),)" + ), + ): + with RaisesGroup(RaisesGroup(ValueError)): + ... + - def test_matcher(self) -> None: - with pytest.raises( - ValueError, match="^You must specify at least one parameter to match on.$" - ): - Matcher() +def test_matcher() -> None: + with pytest.raises( + ValueError, match="^You must specify at least one parameter to match on.$" + ): + Matcher() - with RaisesGroup(Matcher(ValueError)): + with RaisesGroup(Matcher(ValueError)): + raise ExceptionGroup("", (ValueError(),)) + with pytest.raises(ExceptionGroup): + with RaisesGroup(Matcher(TypeError)): raise ExceptionGroup("", (ValueError(),)) - with pytest.raises(ExceptionGroup): - with RaisesGroup(Matcher(TypeError)): - raise ExceptionGroup("", (ValueError(),)) - def test_matcher_match(self) -> None: + +def test_matcher_match() -> None: + with RaisesGroup(Matcher(ValueError, "foo")): + raise ExceptionGroup("", (ValueError("foo"),)) + with pytest.raises(ExceptionGroup): with RaisesGroup(Matcher(ValueError, "foo")): - raise ExceptionGroup("", (ValueError("foo"),)) - with pytest.raises(ExceptionGroup): - with RaisesGroup(Matcher(ValueError, "foo")): - raise ExceptionGroup("", (ValueError("bar"),)) + raise ExceptionGroup("", (ValueError("bar"),)) - # Can be used without specifying the type + # Can be used without specifying the type + with RaisesGroup(Matcher(match="foo")): + raise ExceptionGroup("", (ValueError("foo"),)) + with pytest.raises(ExceptionGroup): with RaisesGroup(Matcher(match="foo")): - raise ExceptionGroup("", (ValueError("foo"),)) - with pytest.raises(ExceptionGroup): - with RaisesGroup(Matcher(match="foo")): - raise ExceptionGroup("", (ValueError("bar"),)) + raise ExceptionGroup("", (ValueError("bar"),)) + - def test_Matcher_check(self) -> None: - def check_oserror_and_errno_is_5(e: BaseException) -> bool: - return isinstance(e, OSError) and e.errno == 5 +def test_Matcher_check() -> None: + def check_oserror_and_errno_is_5(e: BaseException) -> bool: + return isinstance(e, OSError) and e.errno == 5 - with RaisesGroup(Matcher(check=check_oserror_and_errno_is_5)): - raise ExceptionGroup("", (OSError(5, ""),)) + with RaisesGroup(Matcher(check=check_oserror_and_errno_is_5)): + raise ExceptionGroup("", (OSError(5, ""),)) - # specifying exception_type narrows the parameter type to the callable - def check_errno_is_5(e: OSError) -> bool: - return e.errno == 5 + # specifying exception_type narrows the parameter type to the callable + def check_errno_is_5(e: OSError) -> bool: + return e.errno == 5 + with RaisesGroup(Matcher(OSError, check=check_errno_is_5)): + raise ExceptionGroup("", (OSError(5, ""),)) + + with pytest.raises(ExceptionGroup): with RaisesGroup(Matcher(OSError, check=check_errno_is_5)): - raise ExceptionGroup("", (OSError(5, ""),)) - - with pytest.raises(ExceptionGroup): - with RaisesGroup(Matcher(OSError, check=check_errno_is_5)): - raise ExceptionGroup("", (OSError(6, ""),)) - - if TYPE_CHECKING: - # getting the typing working satisfactory is very tricky - # but with RaisesGroup being seen as a subclass of BaseExceptionGroup - # most end-user cases of checking excinfo.value.foobar should work fine now. - def test_types_0(self) -> None: - _: BaseExceptionGroup[ValueError] = RaisesGroup(ValueError) - _ = RaisesGroup(RaisesGroup(ValueError)) # type: ignore[arg-type] - a: BaseExceptionGroup[BaseExceptionGroup[ValueError]] - a = RaisesGroup(RaisesGroup(ValueError)) - a = BaseExceptionGroup("", (BaseExceptionGroup("", (ValueError(),)),)) - assert a - - def test_types_1(self) -> None: - with RaisesGroup(ValueError) as e: - raise ExceptionGroup("foo", (ValueError(),)) - assert_type(e.value, BaseExceptionGroup[ValueError]) - # assert_type(e.value, RaisesGroup[ValueError]) - - def test_types_2(self) -> None: - exc: ExceptionGroup[ValueError] | ValueError = ExceptionGroup( - "", (ValueError(),) - ) - if RaisesGroup(ValueError).matches(exc): - assert_type(exc, BaseExceptionGroup[ValueError]) - - def test_types_3(self) -> None: - e: BaseExceptionGroup[KeyboardInterrupt] = BaseExceptionGroup( - "", (KeyboardInterrupt(),) - ) - if RaisesGroup(ValueError).matches(e): - assert_type(e, BaseExceptionGroup[ValueError]) - - def test_types_4(self) -> None: - with RaisesGroup(Matcher(ValueError)) as e: - ... - _: BaseExceptionGroup[ValueError] = e.value - assert_type(e.value, BaseExceptionGroup[ValueError]) - - def test_types_5(self) -> None: - with RaisesGroup(RaisesGroup(ValueError)) as excinfo: - raise ExceptionGroup("foo", (ValueError(),)) - _: BaseExceptionGroup[BaseExceptionGroup[ValueError]] = excinfo.value - assert_type( - excinfo.value, - BaseExceptionGroup[RaisesGroup[ValueError]], - ) - print(excinfo.value.exceptions[0].exceptions[0]) - - def test_types_6(self) -> None: - exc: ExceptionGroup[ExceptionGroup[ValueError]] = ExceptionGroup( - "", (ExceptionGroup("", (ValueError(),)),) - ) - if RaisesGroup(RaisesGroup(ValueError)).matches(exc): - # ugly - assert_type(exc, BaseExceptionGroup[RaisesGroup[ValueError]]) + raise ExceptionGroup("", (OSError(6, ""),)) + + +if TYPE_CHECKING: + # getting the typing working satisfactory is very tricky + # but with RaisesGroup being seen as a subclass of BaseExceptionGroup + # most end-user cases of checking excinfo.value.foobar should work fine now. + def test_types_0() -> None: + _: BaseExceptionGroup[ValueError] = RaisesGroup(ValueError) + _ = RaisesGroup(RaisesGroup(ValueError)) # type: ignore[arg-type] + a: BaseExceptionGroup[BaseExceptionGroup[ValueError]] + a = RaisesGroup(RaisesGroup(ValueError)) + a = BaseExceptionGroup("", (BaseExceptionGroup("", (ValueError(),)),)) + assert a + + def test_types_1() -> None: + with RaisesGroup(ValueError) as e: + raise ExceptionGroup("foo", (ValueError(),)) + assert_type(e.value, BaseExceptionGroup[ValueError]) + # assert_type(e.value, RaisesGroup[ValueError]) + + def test_types_2() -> None: + exc: ExceptionGroup[ValueError] | ValueError = ExceptionGroup( + "", (ValueError(),) + ) + if RaisesGroup(ValueError).matches(exc): + assert_type(exc, BaseExceptionGroup[ValueError]) + + def test_types_3() -> None: + e: BaseExceptionGroup[KeyboardInterrupt] = BaseExceptionGroup( + "", (KeyboardInterrupt(),) + ) + if RaisesGroup(ValueError).matches(e): + assert_type(e, BaseExceptionGroup[ValueError]) + + def test_types_4() -> None: + with RaisesGroup(Matcher(ValueError)) as e: + ... + _: BaseExceptionGroup[ValueError] = e.value + assert_type(e.value, BaseExceptionGroup[ValueError]) + + def test_types_5() -> None: + with RaisesGroup(RaisesGroup(ValueError)) as excinfo: + raise ExceptionGroup("foo", (ValueError(),)) + _: BaseExceptionGroup[BaseExceptionGroup[ValueError]] = excinfo.value + assert_type( + excinfo.value, + BaseExceptionGroup[RaisesGroup[ValueError]], + ) + print(excinfo.value.exceptions[0].exceptions[0]) + + def test_types_6() -> None: + exc: ExceptionGroup[ExceptionGroup[ValueError]] = ExceptionGroup( + "", (ExceptionGroup("", (ValueError(),)),) + ) + if RaisesGroup(RaisesGroup(ValueError)).matches(exc): + # ugly + assert_type(exc, BaseExceptionGroup[RaisesGroup[ValueError]]) From cd9d3a56204b36d342e488ee35868d8d33385d5d Mon Sep 17 00:00:00 2001 From: jakkdl Date: Sat, 9 Dec 2023 14:23:54 +0100 Subject: [PATCH 09/29] add test for _ExceptionInfo (and fix it) --- src/trio/_tests/test_testing_raisesgroup.py | 15 +++++++++++ src/trio/testing/_raises_group.py | 28 +++++++++++++++++++-- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/trio/_tests/test_testing_raisesgroup.py b/src/trio/_tests/test_testing_raisesgroup.py index 274484aad6..87bd36f3b3 100644 --- a/src/trio/_tests/test_testing_raisesgroup.py +++ b/src/trio/_tests/test_testing_raisesgroup.py @@ -2,10 +2,12 @@ import re import sys +from types import TracebackType from typing import TYPE_CHECKING import pytest +import trio from trio.testing import Matcher, RaisesGroup # TODO: make a public export @@ -195,6 +197,19 @@ def check_errno_is_5(e: OSError) -> bool: raise ExceptionGroup("", (OSError(6, ""),)) +def test__ExceptionInfo(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + trio.testing._raises_group, + "ExceptionInfo", + trio.testing._raises_group._ExceptionInfo, + ) + with trio.testing.RaisesGroup(ValueError) as excinfo: + raise ExceptionGroup("", (ValueError("hello"),)) + assert excinfo.type is ExceptionGroup + assert excinfo.value.exceptions[0].args == ("hello",) + assert isinstance(excinfo.tb, TracebackType) + + if TYPE_CHECKING: # getting the typing working satisfactory is very tricky # but with RaisesGroup being seen as a subclass of BaseExceptionGroup diff --git a/src/trio/testing/_raises_group.py b/src/trio/testing/_raises_group.py index 127fb42678..3acf243b76 100644 --- a/src/trio/testing/_raises_group.py +++ b/src/trio/testing/_raises_group.py @@ -32,7 +32,7 @@ class _ExceptionInfo(Generic[E]): _excinfo: tuple[type[E], E, TracebackType] | None def __init__(self, excinfo: tuple[type[E], E, TracebackType] | None): - self._exc_info = excinfo + self._excinfo = excinfo def fill_unfilled(self, exc_info: tuple[type[E], E, TracebackType]) -> None: """Fill an unfilled ExceptionInfo created with ``for_later()``.""" @@ -44,6 +44,30 @@ def for_later(cls) -> _ExceptionInfo[E]: """Return an unfilled ExceptionInfo.""" return cls(None) + @property + def type(self) -> type[E]: + """The exception class.""" + assert ( + self._excinfo is not None + ), ".type can only be used after the context manager exits" + return self._excinfo[0] + + @property + def value(self) -> E: + """The exception value.""" + assert ( + self._excinfo is not None + ), ".value can only be used after the context manager exits" + return self._excinfo[1] + + @property + def tb(self) -> TracebackType: + """The exception raw traceback.""" + assert ( + self._excinfo is not None + ), ".tb can only be used after the context manager exits" + return self._excinfo[2] + # this may bite users with type checkers not using pytest, but that should # be rare and give quite obvious errors in tests trying to do so. @@ -53,7 +77,7 @@ def for_later(cls) -> _ExceptionInfo[E]: else: try: from pytest import ExceptionInfo - except ImportError: + except ImportError: # pragma: no cover ExceptionInfo = _ExceptionInfo From e9688bee3820a1dd4eca682c77f7e951df7e1027 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Sat, 9 Dec 2023 14:53:38 +0100 Subject: [PATCH 10/29] rewrite not to use any in assert, since codecov doesn't like it --- src/trio/_core/_tests/test_run.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/trio/_core/_tests/test_run.py b/src/trio/_core/_tests/test_run.py index d2449da2d6..3dade8f079 100644 --- a/src/trio/_core/_tests/test_run.py +++ b/src/trio/_core/_tests/test_run.py @@ -774,14 +774,21 @@ async def task2() -> None: ) as exc_info: await nursery_mgr.__aexit__(*sys.exc_info()) - subexceptions = ( - Matcher(RuntimeError, match="closed before the task exited"), - ) * 3 - assert RaisesGroup(*subexceptions).matches(exc_info.value.__context__) - assert any( - isinstance(exc.__context__, _core.Cancelled) - for exc in exc_info.value.__context__.exceptions - ) # for the sleep_forever + def no_context(exc: RuntimeError) -> bool: + return exc.__context__ is None + + msg = "closed before the task exited" + subexceptions = ( + Matcher(RuntimeError, match=msg, check=no_context), + Matcher(RuntimeError, match=msg, check=no_context), + # sleep_forever + Matcher( + RuntimeError, + match=msg, + check=lambda x: isinstance(x.__context__, _core.Cancelled), + ), + ) + assert RaisesGroup(*subexceptions).matches(exc_info.value.__context__) # Trying to exit a cancel scope from an unrelated task raises an error # without affecting any state From 97fb79b4f99bcbeca44b738539726e3bee978217 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Sun, 10 Dec 2023 15:15:53 +0100 Subject: [PATCH 11/29] * Split out type tests * Add longer explanation of what the problem with the types are, and expand type tests * Add __new__ to satisfy pyright * replace RaisesGroup.__repr__ with RaisesGroup.expected_type to not lie to repr(), and add support for printing *Base*ExceptionGroup when correct. * add tests for the above --- src/trio/_core/_tests/test_run.py | 4 +- src/trio/_tests/test_testing_raisesgroup.py | 132 ++++++++------------ src/trio/_tests/type_tests/raisesgroup.py | 103 +++++++++++++++ src/trio/testing/_raises_group.py | 69 +++++++--- 4 files changed, 210 insertions(+), 98 deletions(-) create mode 100644 src/trio/_tests/type_tests/raisesgroup.py diff --git a/src/trio/_core/_tests/test_run.py b/src/trio/_core/_tests/test_run.py index 3dade8f079..62b73b0bfc 100644 --- a/src/trio/_core/_tests/test_run.py +++ b/src/trio/_core/_tests/test_run.py @@ -778,7 +778,7 @@ def no_context(exc: RuntimeError) -> bool: return exc.__context__ is None msg = "closed before the task exited" - subexceptions = ( + group = RaisesGroup( Matcher(RuntimeError, match=msg, check=no_context), Matcher(RuntimeError, match=msg, check=no_context), # sleep_forever @@ -788,7 +788,7 @@ def no_context(exc: RuntimeError) -> bool: check=lambda x: isinstance(x.__context__, _core.Cancelled), ), ) - assert RaisesGroup(*subexceptions).matches(exc_info.value.__context__) + assert group.matches(exc_info.value.__context__) # Trying to exit a cancel scope from an unrelated task raises an error # without affecting any state diff --git a/src/trio/_tests/test_testing_raisesgroup.py b/src/trio/_tests/test_testing_raisesgroup.py index 87bd36f3b3..04c4aefaeb 100644 --- a/src/trio/_tests/test_testing_raisesgroup.py +++ b/src/trio/_tests/test_testing_raisesgroup.py @@ -3,20 +3,15 @@ import re import sys from types import TracebackType -from typing import TYPE_CHECKING +from typing import Any import pytest import trio from trio.testing import Matcher, RaisesGroup -# TODO: make a public export - -if TYPE_CHECKING: - from typing_extensions import assert_type - if sys.version_info < (3, 11): - from exceptiongroup import BaseExceptionGroup, ExceptionGroup + from exceptiongroup import ExceptionGroup def test_raises_group() -> None: @@ -132,22 +127,51 @@ def test_RaisesGroup_matches() -> None: def test_message() -> None: - with pytest.raises( - AssertionError, - match=re.escape( - f"DID NOT RAISE any exception, expected ExceptionGroup({ValueError!r},)" - ), - ): - with RaisesGroup(ValueError): - ... - with pytest.raises( - AssertionError, - match=re.escape( - f"DID NOT RAISE any exception, expected ExceptionGroup(ExceptionGroup({ValueError!r},),)" - ), - ): - with RaisesGroup(RaisesGroup(ValueError)): - ... + def check_message(message: str, body: RaisesGroup[Any]) -> None: + with pytest.raises( + AssertionError, + match=f"^DID NOT RAISE any exception, expected {re.escape(message)}$", + ): + with body: + ... + + # basic + check_message("ExceptionGroup(ValueError)", RaisesGroup(ValueError)) + # multiple exceptions + check_message( + "ExceptionGroup(ValueError, ValueError)", RaisesGroup(ValueError, ValueError) + ) + # nested + check_message( + "ExceptionGroup(ExceptionGroup(ValueError))", + RaisesGroup(RaisesGroup(ValueError)), + ) + + # Matcher + check_message( + "ExceptionGroup(Matcher(ValueError, match='my_str'))", + RaisesGroup(Matcher(ValueError, "my_str")), + ) + + # BaseExceptionGroup + check_message( + "BaseExceptionGroup(KeyboardInterrupt)", RaisesGroup(KeyboardInterrupt) + ) + # BaseExceptionGroup with type inside Matcher + check_message( + "BaseExceptionGroup(Matcher(KeyboardInterrupt))", + RaisesGroup(Matcher(KeyboardInterrupt)), + ) + # Base-ness transfers to parent containers + check_message( + "BaseExceptionGroup(BaseExceptionGroup(KeyboardInterrupt))", + RaisesGroup(RaisesGroup(KeyboardInterrupt)), + ) + # but not to child containers + check_message( + "BaseExceptionGroup(BaseExceptionGroup(KeyboardInterrupt), ExceptionGroup(ValueError))", + RaisesGroup(RaisesGroup(KeyboardInterrupt), RaisesGroup(ValueError)), + ) def test_matcher() -> None: @@ -155,6 +179,11 @@ def test_matcher() -> None: ValueError, match="^You must specify at least one parameter to match on.$" ): Matcher() + with pytest.raises( + ValueError, + match=f"^exception_type {re.escape(repr(object))} must be a subclass of BaseException$", + ): + Matcher(object) # type: ignore[type-var] with RaisesGroup(Matcher(ValueError)): raise ExceptionGroup("", (ValueError(),)) @@ -208,60 +237,3 @@ def test__ExceptionInfo(monkeypatch: pytest.MonkeyPatch) -> None: assert excinfo.type is ExceptionGroup assert excinfo.value.exceptions[0].args == ("hello",) assert isinstance(excinfo.tb, TracebackType) - - -if TYPE_CHECKING: - # getting the typing working satisfactory is very tricky - # but with RaisesGroup being seen as a subclass of BaseExceptionGroup - # most end-user cases of checking excinfo.value.foobar should work fine now. - def test_types_0() -> None: - _: BaseExceptionGroup[ValueError] = RaisesGroup(ValueError) - _ = RaisesGroup(RaisesGroup(ValueError)) # type: ignore[arg-type] - a: BaseExceptionGroup[BaseExceptionGroup[ValueError]] - a = RaisesGroup(RaisesGroup(ValueError)) - a = BaseExceptionGroup("", (BaseExceptionGroup("", (ValueError(),)),)) - assert a - - def test_types_1() -> None: - with RaisesGroup(ValueError) as e: - raise ExceptionGroup("foo", (ValueError(),)) - assert_type(e.value, BaseExceptionGroup[ValueError]) - # assert_type(e.value, RaisesGroup[ValueError]) - - def test_types_2() -> None: - exc: ExceptionGroup[ValueError] | ValueError = ExceptionGroup( - "", (ValueError(),) - ) - if RaisesGroup(ValueError).matches(exc): - assert_type(exc, BaseExceptionGroup[ValueError]) - - def test_types_3() -> None: - e: BaseExceptionGroup[KeyboardInterrupt] = BaseExceptionGroup( - "", (KeyboardInterrupt(),) - ) - if RaisesGroup(ValueError).matches(e): - assert_type(e, BaseExceptionGroup[ValueError]) - - def test_types_4() -> None: - with RaisesGroup(Matcher(ValueError)) as e: - ... - _: BaseExceptionGroup[ValueError] = e.value - assert_type(e.value, BaseExceptionGroup[ValueError]) - - def test_types_5() -> None: - with RaisesGroup(RaisesGroup(ValueError)) as excinfo: - raise ExceptionGroup("foo", (ValueError(),)) - _: BaseExceptionGroup[BaseExceptionGroup[ValueError]] = excinfo.value - assert_type( - excinfo.value, - BaseExceptionGroup[RaisesGroup[ValueError]], - ) - print(excinfo.value.exceptions[0].exceptions[0]) - - def test_types_6() -> None: - exc: ExceptionGroup[ExceptionGroup[ValueError]] = ExceptionGroup( - "", (ExceptionGroup("", (ValueError(),)),) - ) - if RaisesGroup(RaisesGroup(ValueError)).matches(exc): - # ugly - assert_type(exc, BaseExceptionGroup[RaisesGroup[ValueError]]) diff --git a/src/trio/_tests/type_tests/raisesgroup.py b/src/trio/_tests/type_tests/raisesgroup.py new file mode 100644 index 0000000000..804588fb9e --- /dev/null +++ b/src/trio/_tests/type_tests/raisesgroup.py @@ -0,0 +1,103 @@ +"""The typing of RaisesGroup involves a lot of deception and lies, since AFAIK what we +actually want to achieve is ~impossible. This is because we specify what we expect with +instances of RaisesGroup and exception classes, but excinfo.value will be instances of +[Base]ExceptionGroup and instances of exceptions. So we need to "translate" from +RaisesGroup to ExceptionGroup. + +The way it currently works is that RaisesGroup[E] corresponds to +ExceptionInfo[BaseExceptionGroup[E]], so the top-level group will be correct. But +RaisesGroup[RaisesGroup[ValueError]] will become +ExceptionInfo[BaseExceptionGroup[RaisesGroup[ValueError]]]. To get around that we specify +RaisesGroup as a subclass of BaseExceptionGroup during type checking - which should mean +that most static type checking for end users should be mostly correct. +""" +from __future__ import annotations + +import sys +from typing import Union + +from trio.testing import Matcher, RaisesGroup +from typing_extensions import assert_type + +if sys.version_info < (3, 11): + from exceptiongroup import BaseExceptionGroup, ExceptionGroup + +# split into functions to isolate the different scopes + + +def check_inheritance_and_assignments() -> None: + # Check inheritance + _: BaseExceptionGroup[ValueError] = RaisesGroup(ValueError) + _ = RaisesGroup(RaisesGroup(ValueError)) # type: ignore + + a: BaseExceptionGroup[BaseExceptionGroup[ValueError]] + a = RaisesGroup(RaisesGroup(ValueError)) + a = BaseExceptionGroup("", (BaseExceptionGroup("", (ValueError(),)),)) + assert a + + +def check_basic_contextmanager() -> None: + # One level of Group is correctly translated - except it's a BaseExceptionGroup + # instead of an ExceptionGroup. + with RaisesGroup(ValueError) as e: + raise ExceptionGroup("foo", (ValueError(),)) + assert_type(e.value, BaseExceptionGroup[ValueError]) + + +def check_basic_matches() -> None: + # check that matches gets rid of the naked ValueError in the union + exc: ExceptionGroup[ValueError] | ValueError = ExceptionGroup("", (ValueError(),)) + if RaisesGroup(ValueError).matches(exc): + assert_type(exc, BaseExceptionGroup[ValueError]) + + +def check_matches_with_different_exception_type() -> None: + # This should probably raise some type error somewhere, since + # ValueError != KeyboardInterrupt + e: BaseExceptionGroup[KeyboardInterrupt] = BaseExceptionGroup( + "", (KeyboardInterrupt(),) + ) + if RaisesGroup(ValueError).matches(e): + assert_type(e, BaseExceptionGroup[ValueError]) + + +def check_matcher_transparent() -> None: + with RaisesGroup(Matcher(ValueError)) as e: + ... + _: BaseExceptionGroup[ValueError] = e.value + assert_type(e.value, BaseExceptionGroup[ValueError]) + + +def check_nested_raisesgroups_contextmanager() -> None: + with RaisesGroup(RaisesGroup(ValueError)) as excinfo: + raise ExceptionGroup("foo", (ValueError(),)) + + # thanks to inheritance this assignment works + _: BaseExceptionGroup[BaseExceptionGroup[ValueError]] = excinfo.value + # and it can mostly be treated like an exceptiongroup + print(excinfo.value.exceptions[0].exceptions[0]) + + # but assert_type reveals the lies + print(type(excinfo.value)) # would print "ExceptionGroup" + # typing says it's a BaseExceptionGroup + assert_type( + excinfo.value, + BaseExceptionGroup[RaisesGroup[ValueError]], + ) + + print(type(excinfo.value.exceptions[0])) # would print "ExceptionGroup" + # but type checkers are utterly confused + assert_type( + excinfo.value.exceptions[0], + Union[RaisesGroup[ValueError], BaseExceptionGroup[RaisesGroup[ValueError]]], + ) + + +def check_nested_raisesgroups_matches() -> None: + """Check nested RaisesGroups with .matches""" + exc: ExceptionGroup[ExceptionGroup[ValueError]] = ExceptionGroup( + "", (ExceptionGroup("", (ValueError(),)),) + ) + # has the same problems as check_nested_raisesgroups_contextmanager + if RaisesGroup(RaisesGroup(ValueError)).matches(exc): + assert_type(exc, BaseExceptionGroup[RaisesGroup[ValueError]]) diff --git a/src/trio/testing/_raises_group.py b/src/trio/testing/_raises_group.py index 3acf243b76..10383f3152 100644 --- a/src/trio/testing/_raises_group.py +++ b/src/trio/testing/_raises_group.py @@ -101,6 +101,10 @@ def __init__( ): if exception_type is None and match is None and check is None: raise ValueError("You must specify at least one parameter to match on.") + if exception_type is not None and not issubclass(exception_type, BaseException): + raise ValueError( + f"exception_type {exception_type} must be a subclass of BaseException" + ) self.exception_type = exception_type self.match = match self.check = check @@ -118,6 +122,15 @@ def matches(self, exception: E) -> TypeGuard[E]: return False return True + def __str__(self) -> str: + reqs = [] + if self.exception_type is not None: + reqs.append(self.exception_type.__name__) + for req, attr in (("match", self.match), ("check", self.check)): + if attr is not None: + reqs.append(f"{req}={attr!r}") + return f'Matcher({", ".join(reqs)})' + # typing this has been somewhat of a nightmare, with the primary difficulty making # the return type of __enter__ correct. Ideally it would function like this @@ -139,6 +152,12 @@ def matches(self, exception: E) -> TypeGuard[E]: @final class RaisesGroup(ContextManager[ExceptionInfo[BaseExceptionGroup[E]]], SuperClass[E]): + # needed for pyright, since BaseExceptionGroup.__new__ takes two arguments + if TYPE_CHECKING: + + def __new__(cls, *args: object, **kwargs: object) -> RaisesGroup[E]: + ... + def __init__( self, exceptions: type[E] | Matcher[E] | E, @@ -153,22 +172,31 @@ def __init__( ) self.strict = strict self.match_expr = match - # message is read-only in BaseExceptionGroup, which we lie to mypy we inherit from self.check = check + self.is_baseexceptiongroup = False for exc in self.expected_exceptions: - if not isinstance(exc, (Matcher, RaisesGroup)) and not ( - isinstance(exc, type) and issubclass(exc, BaseException) - ): + if isinstance(exc, RaisesGroup): + if not strict: + raise ValueError( + "You cannot specify a nested structure inside a RaisesGroup with" + " strict=False" + ) + self.is_baseexceptiongroup |= exc.is_baseexceptiongroup + elif isinstance(exc, Matcher): + if exc.exception_type is None: + continue + # Matcher __init__ assures it's a subclass of BaseException + self.is_baseexceptiongroup |= not issubclass( + exc.exception_type, Exception + ) + elif isinstance(exc, type) and issubclass(exc, BaseException): + self.is_baseexceptiongroup |= not issubclass(exc, Exception) + else: raise ValueError( "Invalid argument {exc} must be exception type, Matcher, or" " RaisesGroup." ) - if isinstance(exc, RaisesGroup) and not strict: - raise ValueError( - "You cannot specify a nested structure inside a RaisesGroup with" - " strict=False" - ) def __enter__(self) -> ExceptionInfo[BaseExceptionGroup[E]]: self.excinfo: ExceptionInfo[BaseExceptionGroup[E]] = ExceptionInfo.for_later() @@ -241,10 +269,9 @@ def __exit__( exc_tb: TracebackType | None, ) -> bool: __tracebackhide__ = True - assert exc_type is not None, ( - "DID NOT RAISE any exception, expected" - f" ExceptionGroup{self.expected_exceptions!r}" - ) + assert ( + exc_type is not None + ), f"DID NOT RAISE any exception, expected {self.expected_type()}" assert ( self.excinfo is not None ), "Internal error - should have been constructed in __enter__" @@ -260,6 +287,16 @@ def __exit__( self.excinfo.fill_unfilled(exc_info) return True - def __repr__(self) -> str: - # TODO: [Base]ExceptionGroup - return f"ExceptionGroup{self.expected_exceptions}" + def expected_type(self) -> str: + subexcs = [] + for e in self.expected_exceptions: + if isinstance(e, Matcher): + subexcs.append(str(e)) + elif isinstance(e, RaisesGroup): + subexcs.append(e.expected_type()) + elif isinstance(e, type): + subexcs.append(e.__name__) + else: # pragma: no cover + raise AssertionError("unknown type") + group_type = "Base" if self.is_baseexceptiongroup else "" + return f"{group_type}ExceptionGroup({', '.join(subexcs)})" From faaebf5677da9d1c6d8ddb7c704b306510f49a4c Mon Sep 17 00:00:00 2001 From: jakkdl Date: Sat, 16 Dec 2023 13:55:45 +0100 Subject: [PATCH 12/29] rewrite another test to use RaisesGroup --- src/trio/_core/_tests/test_run.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/trio/_core/_tests/test_run.py b/src/trio/_core/_tests/test_run.py index 43c738c14d..5596192f7e 100644 --- a/src/trio/_core/_tests/test_run.py +++ b/src/trio/_core/_tests/test_run.py @@ -2019,7 +2019,18 @@ async def test_traceback_frame_removal() -> None: async def my_child_task() -> NoReturn: raise KeyError() - with pytest.raises(ExceptionGroup) as excinfo: # noqa: PT012 + def check_traceback(exc: KeyError) -> bool: + # The top frame in the exception traceback should be inside the child + # task, not trio/contextvars internals. And there's only one frame + # inside the child task, so this will also detect if our frame-removal + # is too eager. + tb = exc.__traceback__ + assert tb is not None + return tb.tb_frame.f_code is my_child_task.__code__ + + expected_exception = Matcher(KeyError, check=check_traceback) + + with RaisesGroup(expected_exception, expected_exception): # Trick: For now cancel/nursery scopes still leave a bunch of tb gunk # behind. But if there's a MultiError, they leave it on the MultiError, # which lets us get a clean look at the KeyError itself. Someday I @@ -2028,15 +2039,6 @@ async def my_child_task() -> NoReturn: async with _core.open_nursery() as nursery: nursery.start_soon(my_child_task) nursery.start_soon(my_child_task) - first_exc = excinfo.value.exceptions[0] - assert isinstance(first_exc, KeyError) - # The top frame in the exception traceback should be inside the child - # task, not trio/contextvars internals. And there's only one frame - # inside the child task, so this will also detect if our frame-removal - # is too eager. - tb = first_exc.__traceback__ - assert tb is not None - assert tb.tb_frame.f_code is my_child_task.__code__ def test_contextvar_support() -> None: From 22d8b5a26497585a76f1d5555a23465428be1173 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 22 Dec 2023 12:39:57 +0100 Subject: [PATCH 13/29] add new classes to docs --- docs/source/reference-testing.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/source/reference-testing.rst b/docs/source/reference-testing.rst index 76ecd4a2d4..89573bab2f 100644 --- a/docs/source/reference-testing.rst +++ b/docs/source/reference-testing.rst @@ -219,3 +219,16 @@ Testing checkpoints .. autofunction:: assert_no_checkpoints :with: + + +ExceptionGroup helpers +---------------------- + +.. autoclass:: RaisesGroup + :members: + +.. autoclass:: Matcher + :members: + +.. autoclass:: trio.testing._raises_group._ExceptionInfo + :members: From 43a51b68ad95e0547a2add20282f5269d33c1794 Mon Sep 17 00:00:00 2001 From: CoolCat467 <52022020+CoolCat467@users.noreply.github.com> Date: Fri, 22 Dec 2023 08:03:53 -0600 Subject: [PATCH 14/29] Fix ruff issues --- src/trio/_tests/test_testing_raisesgroup.py | 4 ++-- src/trio/testing/_raises_group.py | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/trio/_tests/test_testing_raisesgroup.py b/src/trio/_tests/test_testing_raisesgroup.py index 04c4aefaeb..6c4191b911 100644 --- a/src/trio/_tests/test_testing_raisesgroup.py +++ b/src/trio/_tests/test_testing_raisesgroup.py @@ -95,9 +95,9 @@ def test_raises_group() -> None: RaisesGroup(RaisesGroup(ValueError), strict=False) # currently not fully identical in behaviour to expect*, which would also catch an unwrapped exception - with pytest.raises(ValueError): + with pytest.raises(ValueError, match="^value error text$"): with RaisesGroup(ValueError, strict=False): - raise ValueError + raise ValueError("value error text") def test_match() -> None: diff --git a/src/trio/testing/_raises_group.py b/src/trio/testing/_raises_group.py index 10383f3152..12be70f56f 100644 --- a/src/trio/testing/_raises_group.py +++ b/src/trio/testing/_raises_group.py @@ -71,12 +71,13 @@ def tb(self) -> TracebackType: # this may bite users with type checkers not using pytest, but that should # be rare and give quite obvious errors in tests trying to do so. +# Ignoring "incorrect import of pytest", it makes sense in this situation. if TYPE_CHECKING: - from pytest import ExceptionInfo + from pytest import ExceptionInfo # noqa: PT013 else: try: - from pytest import ExceptionInfo + from pytest import ExceptionInfo # noqa: PT013 except ImportError: # pragma: no cover ExceptionInfo = _ExceptionInfo From 3cc15e6c07688677708015f0460e68b3b5a80e97 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 22 Dec 2023 15:10:39 +0100 Subject: [PATCH 15/29] add fix for TracebackType --- docs/source/conf.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/source/conf.py b/docs/source/conf.py index f929c8665d..9746c369d3 100755 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -201,6 +201,9 @@ def add_mapping( # builtins.FrameType. # See https://github.com/sphinx-doc/sphinx/issues/11802 add_mapping("class", "types", "FrameType") + # this *should* work, since sphinx explicitly added a workaround for it + # See https://github.com/sphinx-doc/sphinx/pull/9015 + add_mapping("class", "types", "TracebackType") # new in py3.12, and need target because sphinx is unable to look up # the module of the object if compiling on <3.12 if not hasattr(collections.abc, "Buffer"): From efbccbc45549cae8bcd090be111a2a3706c98b0d Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 22 Dec 2023 15:40:39 +0100 Subject: [PATCH 16/29] cover __str__ of Matcher with no type specified --- src/trio/_tests/test_testing_raisesgroup.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/trio/_tests/test_testing_raisesgroup.py b/src/trio/_tests/test_testing_raisesgroup.py index 6c4191b911..cb14d575de 100644 --- a/src/trio/_tests/test_testing_raisesgroup.py +++ b/src/trio/_tests/test_testing_raisesgroup.py @@ -152,6 +152,10 @@ def check_message(message: str, body: RaisesGroup[Any]) -> None: "ExceptionGroup(Matcher(ValueError, match='my_str'))", RaisesGroup(Matcher(ValueError, "my_str")), ) + check_message( + "ExceptionGroup(Matcher(match='my_str'))", + RaisesGroup(Matcher(match="my_str")), + ) # BaseExceptionGroup check_message( From 1261dc1c88be8effac375a5b84f1ffddb2413bac Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 22 Dec 2023 15:56:34 +0100 Subject: [PATCH 17/29] properly "fix" sphinx+TracebackType --- docs/source/conf.py | 3 --- src/trio/testing/_raises_group.py | 16 +++++++++------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/docs/source/conf.py b/docs/source/conf.py index 9746c369d3..f929c8665d 100755 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -201,9 +201,6 @@ def add_mapping( # builtins.FrameType. # See https://github.com/sphinx-doc/sphinx/issues/11802 add_mapping("class", "types", "FrameType") - # this *should* work, since sphinx explicitly added a workaround for it - # See https://github.com/sphinx-doc/sphinx/pull/9015 - add_mapping("class", "types", "TracebackType") # new in py3.12, and need target because sphinx is unable to look up # the module of the object if compiling on <3.12 if not hasattr(collections.abc, "Buffer"): diff --git a/src/trio/testing/_raises_group.py b/src/trio/testing/_raises_group.py index 12be70f56f..76c4adb3bd 100644 --- a/src/trio/testing/_raises_group.py +++ b/src/trio/testing/_raises_group.py @@ -16,7 +16,9 @@ from trio._util import final if TYPE_CHECKING: - from types import TracebackType + # sphinx will *only* work if we use types.TracebackType, and import + # *inside* TYPE_CHECKING. No other combination works..... + import types from typing_extensions import TypeGuard @@ -29,12 +31,12 @@ # minimal version of pytest.ExceptionInfo in case it is not available @final class _ExceptionInfo(Generic[E]): - _excinfo: tuple[type[E], E, TracebackType] | None + _excinfo: tuple[type[E], E, types.TracebackType] | None - def __init__(self, excinfo: tuple[type[E], E, TracebackType] | None): + def __init__(self, excinfo: tuple[type[E], E, types.TracebackType] | None): self._excinfo = excinfo - def fill_unfilled(self, exc_info: tuple[type[E], E, TracebackType]) -> None: + def fill_unfilled(self, exc_info: tuple[type[E], E, types.TracebackType]) -> None: """Fill an unfilled ExceptionInfo created with ``for_later()``.""" assert self._excinfo is None, "ExceptionInfo was already filled" self._excinfo = exc_info @@ -61,7 +63,7 @@ def value(self) -> E: return self._excinfo[1] @property - def tb(self) -> TracebackType: + def tb(self) -> types.TracebackType: """The exception raw traceback.""" assert ( self._excinfo is not None @@ -267,7 +269,7 @@ def __exit__( self, exc_type: type[BaseException] | None, exc_val: BaseException | None, - exc_tb: TracebackType | None, + exc_tb: types.TracebackType | None, ) -> bool: __tracebackhide__ = True assert ( @@ -282,7 +284,7 @@ def __exit__( # Cast to narrow the exception type now that it's verified. exc_info = cast( - "tuple[type[BaseExceptionGroup[E]], BaseExceptionGroup[E], TracebackType]", + "tuple[type[BaseExceptionGroup[E]], BaseExceptionGroup[E], types.TracebackType]", (exc_type, exc_val, exc_tb), ) self.excinfo.fill_unfilled(exc_info) From ccdb79dac1e009becb05014bdce249f12c16ab14 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 22 Dec 2023 16:01:40 +0100 Subject: [PATCH 18/29] add newsfragment --- newsfragments/2785.feature.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/2785.feature.rst diff --git a/newsfragments/2785.feature.rst b/newsfragments/2785.feature.rst new file mode 100644 index 0000000000..b4b5031589 --- /dev/null +++ b/newsfragments/2785.feature.rst @@ -0,0 +1 @@ +New helper classes: :class:`~.testing.RaisesGroup` and :class:`~.testing.Matcher`. In preparation for changing the default of ``strict_exception_groups`` to `True`, we're introducing a set of helper classes that can be used in place of `pytest.raises ` in tests, to check for an expected `ExceptionGroup`. From 7956848d7ad0b4cb2aa52b972d34238fe6310409 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Sat, 23 Dec 2023 12:12:19 +0100 Subject: [PATCH 19/29] fix url in newsfragment --- newsfragments/2785.feature.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/newsfragments/2785.feature.rst b/newsfragments/2785.feature.rst index b4b5031589..d8a094bc7c 100644 --- a/newsfragments/2785.feature.rst +++ b/newsfragments/2785.feature.rst @@ -1 +1 @@ -New helper classes: :class:`~.testing.RaisesGroup` and :class:`~.testing.Matcher`. In preparation for changing the default of ``strict_exception_groups`` to `True`, we're introducing a set of helper classes that can be used in place of `pytest.raises ` in tests, to check for an expected `ExceptionGroup`. +New helper classes: :class:`~.testing.RaisesGroup` and :class:`~.testing.Matcher`. In preparation for changing the default of ``strict_exception_groups`` to `True`, we're introducing a set of helper classes that can be used in place of `pytest.raises `_ in tests, to check for an expected `ExceptionGroup`. From 4990a7d34d396d78292efe012eb4fffd51fe2128 Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Fri, 29 Dec 2023 18:18:49 +1000 Subject: [PATCH 20/29] Add overloads to Matcher() to precisely check init parameters --- src/trio/_tests/test_testing_raisesgroup.py | 2 +- src/trio/_tests/type_tests/raisesgroup.py | 24 ++++++++++++++ src/trio/testing/_raises_group.py | 36 +++++++++++++++++---- 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/trio/_tests/test_testing_raisesgroup.py b/src/trio/_tests/test_testing_raisesgroup.py index cb14d575de..2a89e5231b 100644 --- a/src/trio/_tests/test_testing_raisesgroup.py +++ b/src/trio/_tests/test_testing_raisesgroup.py @@ -182,7 +182,7 @@ def test_matcher() -> None: with pytest.raises( ValueError, match="^You must specify at least one parameter to match on.$" ): - Matcher() + Matcher() # type: ignore[call-overload] with pytest.raises( ValueError, match=f"^exception_type {re.escape(repr(object))} must be a subclass of BaseException$", diff --git a/src/trio/_tests/type_tests/raisesgroup.py b/src/trio/_tests/type_tests/raisesgroup.py index 804588fb9e..7dee254c5d 100644 --- a/src/trio/_tests/type_tests/raisesgroup.py +++ b/src/trio/_tests/type_tests/raisesgroup.py @@ -61,6 +61,30 @@ def check_matches_with_different_exception_type() -> None: assert_type(e, BaseExceptionGroup[ValueError]) +def check_matcher_init() -> None: + def check_exc(exc: BaseException) -> bool: + return isinstance(exc, ValueError) + + def check_filenotfound(exc: FileNotFoundError) -> bool: + return not exc.filename.endswith(".tmp") + + # Check various combinations of constructor signatures. + # At least 1 arg must be provided. If exception_type is provided, that narrows + # check's argument. + Matcher() # type: ignore + Matcher(ValueError) + Matcher(ValueError, "regex") + Matcher(ValueError, "regex", check_exc) + 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(match="regex", check=check_exc) + Matcher(FileNotFoundError, match="regex", check=check_filenotfound) + + def check_matcher_transparent() -> None: with RaisesGroup(Matcher(ValueError)) as e: ... diff --git a/src/trio/testing/_raises_group.py b/src/trio/testing/_raises_group.py index 76c4adb3bd..bdb6c2c9e5 100644 --- a/src/trio/testing/_raises_group.py +++ b/src/trio/testing/_raises_group.py @@ -11,6 +11,7 @@ Pattern, TypeVar, cast, + overload, ) from trio._util import final @@ -96,6 +97,30 @@ def _stringify_exception(exc: BaseException) -> str: @final class Matcher(Generic[E]): + # At least one of the three parameters must be passed. + @overload + def __init__( + self: Matcher[E], + exception_type: type[E], + match: str | Pattern[str] = ..., + check: Callable[[E], bool] = ..., + ): + ... + + @overload + def __init__( + self: Matcher[BaseException], # Give E a value. + *, + match: str | Pattern[str], + # If exception_type is not provided, check() must do any typechecks itself. + check: Callable[[BaseException], bool] = ..., + ): + ... + + @overload + def __init__(self, *, check: Callable[[BaseException], bool]): + ... + def __init__( self, exception_type: type[E] | None = None, @@ -112,7 +137,7 @@ def __init__( self.match = match self.check = check - def matches(self, exception: E) -> TypeGuard[E]: + def matches(self, exception: BaseException) -> TypeGuard[E]: if self.exception_type is not None and not isinstance( exception, self.exception_type ): @@ -121,7 +146,9 @@ def matches(self, exception: E) -> TypeGuard[E]: self.match, _stringify_exception(exception) ): return False - if self.check is not None and not self.check(exception): + # If exception_type is None check() accepts BaseException. + # If non-none, we have done an isinstance check above. + if self.check is not None and not self.check(cast(E, exception)): return False return True @@ -254,10 +281,7 @@ def matches( and isinstance(rem_e, RaisesGroup) and rem_e.matches(e) ) - or ( - isinstance(rem_e, Matcher) - and rem_e.matches(e) # type: ignore[arg-type] - ) + or (isinstance(rem_e, Matcher) and rem_e.matches(e)) ): remaining_exceptions.remove(rem_e) # type: ignore[arg-type] break From 86287a5c586a100f361564206ed348c3e4a7f294 Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Fri, 29 Dec 2023 18:52:16 +1000 Subject: [PATCH 21/29] Pre-compile strings passed to Matcher(), but unwrap in __str__ --- src/trio/_tests/type_tests/raisesgroup.py | 14 ++++++++++++++ src/trio/testing/_raises_group.py | 20 ++++++++++++++++---- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/trio/_tests/type_tests/raisesgroup.py b/src/trio/_tests/type_tests/raisesgroup.py index 7dee254c5d..83f2656a23 100644 --- a/src/trio/_tests/type_tests/raisesgroup.py +++ b/src/trio/_tests/type_tests/raisesgroup.py @@ -13,6 +13,7 @@ """ from __future__ import annotations +import re import sys from typing import Union @@ -92,6 +93,19 @@ def check_matcher_transparent() -> None: assert_type(e.value, BaseExceptionGroup[ValueError]) +def check_matcher_tostring() -> None: + assert str(Matcher(ValueError)) == "Matcher(ValueError)" + assert str(Matcher(match="[a-z]")) == "Matcher(match='[a-z]')" + pattern_no_flags = re.compile("noflag", 0) + assert str(Matcher(match=pattern_no_flags)) == "Matcher(match='noflag')" + pattern_flags = re.compile("noflag", re.IGNORECASE) + assert str(Matcher(match=pattern_flags)) == f"Matcher(match={pattern_flags!r})" + assert ( + str(Matcher(ValueError, match="re", check=bool)) + == f"Matcher(ValueError, match='re', check={bool!r})" + ) + + def check_nested_raisesgroups_contextmanager() -> None: with RaisesGroup(RaisesGroup(ValueError)) as excinfo: raise ExceptionGroup("foo", (ValueError(),)) diff --git a/src/trio/testing/_raises_group.py b/src/trio/testing/_raises_group.py index bdb6c2c9e5..4521bf9a4b 100644 --- a/src/trio/testing/_raises_group.py +++ b/src/trio/testing/_raises_group.py @@ -95,6 +95,10 @@ def _stringify_exception(exc: BaseException) -> str: ) +# String patterns default to including the unicode flag. +_regex_no_flags = re.compile("").flags + + @final class Matcher(Generic[E]): # At least one of the three parameters must be passed. @@ -134,7 +138,11 @@ def __init__( f"exception_type {exception_type} must be a subclass of BaseException" ) self.exception_type = exception_type - self.match = match + self.match: Pattern[str] | None + if isinstance(match, str): + self.match = re.compile(match) + else: + self.match = match self.check = check def matches(self, exception: BaseException) -> TypeGuard[E]: @@ -156,9 +164,13 @@ def __str__(self) -> str: reqs = [] if self.exception_type is not None: reqs.append(self.exception_type.__name__) - for req, attr in (("match", self.match), ("check", self.check)): - if attr is not None: - reqs.append(f"{req}={attr!r}") + if (match := self.match) is not None: + # If no flags were specified, discard the redundant re.compile() here. + reqs.append( + f"match={match.pattern if match.flags == _regex_no_flags else match!r}" + ) + if self.check is not None: + reqs.append(f"check={self.check!r}") return f'Matcher({", ".join(reqs)})' From 663c12b692e9ed1f35448d7d20cf0ecb5bf3631f Mon Sep 17 00:00:00 2001 From: jakkdl Date: Sat, 30 Dec 2023 11:12:43 +0100 Subject: [PATCH 22/29] update comment, add pyright: ignore --- src/trio/_tests/type_tests/raisesgroup.py | 10 ++++++++-- src/trio/testing/_raises_group.py | 6 +++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/trio/_tests/type_tests/raisesgroup.py b/src/trio/_tests/type_tests/raisesgroup.py index 83f2656a23..6682663b96 100644 --- a/src/trio/_tests/type_tests/raisesgroup.py +++ b/src/trio/_tests/type_tests/raisesgroup.py @@ -33,7 +33,10 @@ def check_inheritance_and_assignments() -> None: a: BaseExceptionGroup[BaseExceptionGroup[ValueError]] a = RaisesGroup(RaisesGroup(ValueError)) - a = BaseExceptionGroup("", (BaseExceptionGroup("", (ValueError(),)),)) + # pyright-ignore due to bug in exceptiongroup + # https://github.com/agronholm/exceptiongroup/pull/101 + # once fixed we'll get errors for unnecessary-pyright-ignore and can clean up + a = BaseExceptionGroup("", (BaseExceptionGroup("", (ValueError(),)),)) # pyright: ignore assert a @@ -133,8 +136,11 @@ def check_nested_raisesgroups_contextmanager() -> None: def check_nested_raisesgroups_matches() -> None: """Check nested RaisesGroups with .matches""" + # pyright-ignore due to bug in exceptiongroup + # https://github.com/agronholm/exceptiongroup/pull/101 + # once fixed we'll get errors for unnecessary-pyright-ignore and can clean up exc: ExceptionGroup[ExceptionGroup[ValueError]] = ExceptionGroup( - "", (ExceptionGroup("", (ValueError(),)),) + "", (ExceptionGroup("", (ValueError(),)),) # pyright: ignore ) # has the same problems as check_nested_raisesgroups_contextmanager if RaisesGroup(RaisesGroup(ValueError)).matches(exc): diff --git a/src/trio/testing/_raises_group.py b/src/trio/testing/_raises_group.py index 4521bf9a4b..509fa3224e 100644 --- a/src/trio/testing/_raises_group.py +++ b/src/trio/testing/_raises_group.py @@ -72,9 +72,9 @@ def tb(self) -> types.TracebackType: return self._excinfo[2] -# this may bite users with type checkers not using pytest, but that should -# be rare and give quite obvious errors in tests trying to do so. -# Ignoring "incorrect import of pytest", it makes sense in this situation. +# This may bite users with type checkers not using pytest, but that should +# be rare and give quite obvious errors at runtime. +# PT013: "incorrect import of pytest". if TYPE_CHECKING: from pytest import ExceptionInfo # noqa: PT013 From 148df6765b16059cae30fceeda1587ed3c807f10 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Sat, 30 Dec 2023 11:26:53 +0100 Subject: [PATCH 23/29] fix formatting, move matcher_tostring test to test_testing_raisesgroup --- src/trio/_tests/test_testing_raisesgroup.py | 13 +++++++++++++ src/trio/_tests/type_tests/raisesgroup.py | 20 ++++---------------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/trio/_tests/test_testing_raisesgroup.py b/src/trio/_tests/test_testing_raisesgroup.py index 2a89e5231b..f9f456c610 100644 --- a/src/trio/_tests/test_testing_raisesgroup.py +++ b/src/trio/_tests/test_testing_raisesgroup.py @@ -230,6 +230,19 @@ def check_errno_is_5(e: OSError) -> bool: raise ExceptionGroup("", (OSError(6, ""),)) +def test_matcher_tostring() -> None: + assert str(Matcher(ValueError)) == "Matcher(ValueError)" + assert str(Matcher(match="[a-z]")) == "Matcher(match='[a-z]')" + pattern_no_flags = re.compile("noflag", 0) + assert str(Matcher(match=pattern_no_flags)) == "Matcher(match='noflag')" + pattern_flags = re.compile("noflag", re.IGNORECASE) + assert str(Matcher(match=pattern_flags)) == f"Matcher(match={pattern_flags!r})" + assert ( + str(Matcher(ValueError, match="re", check=bool)) + == f"Matcher(ValueError, match='re', check={bool!r})" + ) + + def test__ExceptionInfo(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr( trio.testing._raises_group, diff --git a/src/trio/_tests/type_tests/raisesgroup.py b/src/trio/_tests/type_tests/raisesgroup.py index 6682663b96..e00c20d1ba 100644 --- a/src/trio/_tests/type_tests/raisesgroup.py +++ b/src/trio/_tests/type_tests/raisesgroup.py @@ -13,7 +13,6 @@ """ from __future__ import annotations -import re import sys from typing import Union @@ -36,7 +35,9 @@ def check_inheritance_and_assignments() -> None: # pyright-ignore due to bug in exceptiongroup # https://github.com/agronholm/exceptiongroup/pull/101 # once fixed we'll get errors for unnecessary-pyright-ignore and can clean up - a = BaseExceptionGroup("", (BaseExceptionGroup("", (ValueError(),)),)) # pyright: ignore + a = BaseExceptionGroup( + "", (BaseExceptionGroup("", (ValueError(),)),) # pyright: ignore + ) assert a @@ -96,19 +97,6 @@ def check_matcher_transparent() -> None: assert_type(e.value, BaseExceptionGroup[ValueError]) -def check_matcher_tostring() -> None: - assert str(Matcher(ValueError)) == "Matcher(ValueError)" - assert str(Matcher(match="[a-z]")) == "Matcher(match='[a-z]')" - pattern_no_flags = re.compile("noflag", 0) - assert str(Matcher(match=pattern_no_flags)) == "Matcher(match='noflag')" - pattern_flags = re.compile("noflag", re.IGNORECASE) - assert str(Matcher(match=pattern_flags)) == f"Matcher(match={pattern_flags!r})" - assert ( - str(Matcher(ValueError, match="re", check=bool)) - == f"Matcher(ValueError, match='re', check={bool!r})" - ) - - def check_nested_raisesgroups_contextmanager() -> None: with RaisesGroup(RaisesGroup(ValueError)) as excinfo: raise ExceptionGroup("foo", (ValueError(),)) @@ -140,7 +128,7 @@ def check_nested_raisesgroups_matches() -> None: # https://github.com/agronholm/exceptiongroup/pull/101 # once fixed we'll get errors for unnecessary-pyright-ignore and can clean up exc: ExceptionGroup[ExceptionGroup[ValueError]] = ExceptionGroup( - "", (ExceptionGroup("", (ValueError(),)),) # pyright: ignore + "", (ExceptionGroup("", (ValueError(),)),) # pyright: ignore ) # has the same problems as check_nested_raisesgroups_contextmanager if RaisesGroup(RaisesGroup(ValueError)).matches(exc): From c8f7983bd4257d5949490bf9fc3101e861e3314c Mon Sep 17 00:00:00 2001 From: jakkdl Date: Wed, 3 Jan 2024 14:34:52 +0100 Subject: [PATCH 24/29] add docstrings --- src/trio/testing/_raises_group.py | 86 +++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/src/trio/testing/_raises_group.py b/src/trio/testing/_raises_group.py index 509fa3224e..69ef1291a5 100644 --- a/src/trio/testing/_raises_group.py +++ b/src/trio/testing/_raises_group.py @@ -32,6 +32,8 @@ # minimal version of pytest.ExceptionInfo in case it is not available @final class _ExceptionInfo(Generic[E]): + """Minimal re-implementation of pytest.ExceptionInfo, only used if pytest is not available. Supports a subset of its features necessary for functionality of :class:`trio.testing.RaisesGroup` and :class:`trio.testing.Matcher`.""" + _excinfo: tuple[type[E], E, types.TracebackType] | None def __init__(self, excinfo: tuple[type[E], E, types.TracebackType] | None): @@ -101,6 +103,21 @@ def _stringify_exception(exc: BaseException) -> str: @final class Matcher(Generic[E]): + """Helper class to be used together with RaisesGroups when you want to specify requirements on sub-exceptions. Only specifying the type is redundant, and it's also unnecessary when the type is a nested `RaisesGroup` since it supports the same arguments. + The type is checked with `isinstance`, and does not need to be an exact match. If that is wanted you can use the ``check`` parameter. + :meth:`trio.testing.Matcher.matches` can also be used standalone to check individual exceptions. + + Examples:: + + with RaisesGroups(Matcher(ValueError, match="string")) + ... + with RaisesGroups(Matcher(check=lambda x: x.args == (3, "hello"))): + ... + with RaisesGroups(Matcher(check=lambda x: type(x) is ValueError)): + ... + + """ + # At least one of the three parameters must be passed. @overload def __init__( @@ -146,6 +163,23 @@ def __init__( self.check = check def matches(self, exception: BaseException) -> TypeGuard[E]: + """Check if an exception matches the requirements of this Matcher. + + Examples:: + + assert Matcher(ValueError).matches(my_exception): + # is equivalent to + assert isinstance(my_exception, ValueError) + + # this can be useful when checking e.g. the ``__cause__`` of an exception. + with pytest.raises(ValueError) as excinfo: + ... + assert Matcher(SyntaxError, match="foo").matches(excinfo.value.__cause__) + # above line is equivalent to + assert isinstance(excinfo.value.__cause__, SyntaxError) + assert re.search("foo", str(excinfo.value.__cause__) + + """ if self.exception_type is not None and not isinstance( exception, self.exception_type ): @@ -194,6 +228,45 @@ def __str__(self) -> str: @final class RaisesGroup(ContextManager[ExceptionInfo[BaseExceptionGroup[E]]], SuperClass[E]): + """Contextmanager for checking for an expected `ExceptionGroup`. + 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* ` 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``. + 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. + + Examples:: + + with RaisesGroups(ValueError): + raise ExceptionGroup("", (ValueError(),)) + with RaisesGroups(ValueError, ValueError, Matcher(TypeError, match="expected int")): + ... + with RaisesGroups(KeyboardInterrupt, match="hello", check=lambda x: type(x) is BaseExceptionGroup): + ... + with RaisesGroups(RaisesGroups(ValueError)): + raise ExceptionGroup("", (ExceptionGroup("", (ValueError(),)),)) + + with RaisesGroups(ValueError, strict=False): + raise ExceptionGroup("", (ExceptionGroup("", (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:: + + with RaisesGroups(ValueError, Matcher(ValueError, match="hello")): + raise ExceptionGroup("", (ValueError("hello"), ValueError("goodbye"))) + + even though it generally does not care about the order of the exceptions in the group. + To avoid the above you should specify the first ValueError with a Matcher as well. + + It is also not typechecked perfectly, and that's likely not possible with the current approach. Most common usage should work without issue though. + """ + # needed for pyright, since BaseExceptionGroup.__new__ takes two arguments if TYPE_CHECKING: @@ -261,6 +334,19 @@ def matches( self, exc_val: BaseException | None, ) -> TypeGuard[BaseExceptionGroup[E]]: + """Check if an exception matches the requirements of this RaisesGroup. + + Example:: + + with pytest.raises(TypeError) as excinfo: + ... + assert RaisesGroups(ValueError).matches(excinfo.value.__cause__) + # the above line is equivalent to + myexc = excinfo.value.__cause + assert isinstance(myexc, BaseExceptionGroup) + assert len(myexc.exceptions) == 1 + assert isinstance(myexc.exceptions[0], ValueError) + """ if exc_val is None: return False # TODO: print/raise why a match fails, in a way that works properly in nested cases From 9b6618704a2b21dba3631b9af19ae826be5bcc24 Mon Sep 17 00:00:00 2001 From: John Litborn <11260241+jakkdl@users.noreply.github.com> Date: Fri, 5 Jan 2024 12:41:13 +0100 Subject: [PATCH 25/29] Apply suggestions from code review Co-authored-by: EXPLOSION --- newsfragments/2785.feature.rst | 4 +++- src/trio/testing/_raises_group.py | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/newsfragments/2785.feature.rst b/newsfragments/2785.feature.rst index d8a094bc7c..24a334c125 100644 --- a/newsfragments/2785.feature.rst +++ b/newsfragments/2785.feature.rst @@ -1 +1,3 @@ -New helper classes: :class:`~.testing.RaisesGroup` and :class:`~.testing.Matcher`. In preparation for changing the default of ``strict_exception_groups`` to `True`, we're introducing a set of helper classes that can be used in place of `pytest.raises `_ in tests, to check for an expected `ExceptionGroup`. +New helper classes: :class:`~.testing.RaisesGroup` and :class:`~.testing.Matcher`. + +In preparation for changing the default of ``strict_exception_groups`` to `True`, we're introducing a set of helper classes that can be used in place of `pytest.raises `_ in tests, to check for an expected `ExceptionGroup`. diff --git a/src/trio/testing/_raises_group.py b/src/trio/testing/_raises_group.py index 69ef1291a5..9f9188f09b 100644 --- a/src/trio/testing/_raises_group.py +++ b/src/trio/testing/_raises_group.py @@ -275,8 +275,8 @@ def __new__(cls, *args: object, **kwargs: object) -> RaisesGroup[E]: def __init__( self, - exceptions: type[E] | Matcher[E] | E, - *args: type[E] | Matcher[E] | E, + exception: type[E] | Matcher[E] | E, + *other_exceptions: type[E] | Matcher[E] | E, strict: bool = True, match: str | Pattern[str] | None = None, check: Callable[[BaseExceptionGroup[E]], bool] | None = None, @@ -309,7 +309,7 @@ def __init__( self.is_baseexceptiongroup |= not issubclass(exc, Exception) else: raise ValueError( - "Invalid argument {exc} must be exception type, Matcher, or" + f"Invalid argument {exc!r} must be exception type, Matcher, or" " RaisesGroup." ) From d0206559b19351fd5f2a160a4667d33e376d1e9e Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 5 Jan 2024 12:44:31 +0100 Subject: [PATCH 26/29] add comments --- src/trio/testing/_raises_group.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/trio/testing/_raises_group.py b/src/trio/testing/_raises_group.py index 9f9188f09b..f8a1ba4431 100644 --- a/src/trio/testing/_raises_group.py +++ b/src/trio/testing/_raises_group.py @@ -220,8 +220,11 @@ def __str__(self) -> str: # Current solution settles on the above giving BaseExceptionGroup[RaisesGroup[ValueError]], and it not # being a type error to do `with RaisesGroup(ValueError()): ...` - but that will error on runtime. + +# We lie to type checkers that we inherit, so excinfo.value and sub-exceptiongroups can be treated as ExceptionGroups if TYPE_CHECKING: SuperClass = BaseExceptionGroup +# Inheriting at runtime leads to a series of TypeErrors, so we do not want to do that. else: SuperClass = Generic @@ -282,8 +285,8 @@ def __init__( check: Callable[[BaseExceptionGroup[E]], bool] | None = None, ): self.expected_exceptions: tuple[type[E] | Matcher[E] | E, ...] = ( - exceptions, - *args, + exception, + *other_exceptions, ) self.strict = strict self.match_expr = match From 957b4fd035a8a04f4f19c9de503283515d8df1e8 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 5 Jan 2024 13:29:34 +0100 Subject: [PATCH 27/29] switch to tell type checkers that we always use _ExceptionInfo. Add note about the new classes being provisional in the newsfragment --- newsfragments/2785.feature.rst | 1 + src/trio/testing/_raises_group.py | 41 +++++++++++++++++++++++++++---- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/newsfragments/2785.feature.rst b/newsfragments/2785.feature.rst index 24a334c125..834d76be02 100644 --- a/newsfragments/2785.feature.rst +++ b/newsfragments/2785.feature.rst @@ -1,3 +1,4 @@ New helper classes: :class:`~.testing.RaisesGroup` and :class:`~.testing.Matcher`. In preparation for changing the default of ``strict_exception_groups`` to `True`, we're introducing a set of helper classes that can be used in place of `pytest.raises `_ in tests, to check for an expected `ExceptionGroup`. +These are provisional, and only planned to be supplied until there's a good solution in `pytest`. See https://github.com/pytest-dev/pytest/issues/11538 diff --git a/src/trio/testing/_raises_group.py b/src/trio/testing/_raises_group.py index f8a1ba4431..9bed302db0 100644 --- a/src/trio/testing/_raises_group.py +++ b/src/trio/testing/_raises_group.py @@ -17,10 +17,13 @@ from trio._util import final if TYPE_CHECKING: + import builtins + # sphinx will *only* work if we use types.TracebackType, and import # *inside* TYPE_CHECKING. No other combination works..... import types + from _pytest._code.code import ExceptionChainRepr, ReprExceptionInfo, Traceback from typing_extensions import TypeGuard if sys.version_info < (3, 11): @@ -29,7 +32,6 @@ E = TypeVar("E", bound=BaseException) -# minimal version of pytest.ExceptionInfo in case it is not available @final class _ExceptionInfo(Generic[E]): """Minimal re-implementation of pytest.ExceptionInfo, only used if pytest is not available. Supports a subset of its features necessary for functionality of :class:`trio.testing.RaisesGroup` and :class:`trio.testing.Matcher`.""" @@ -73,12 +75,41 @@ def tb(self) -> types.TracebackType: ), ".tb can only be used after the context manager exits" return self._excinfo[2] + def exconly(self, tryshort: bool = False) -> str: + raise NotImplementedError( + "This is a helper method only available if you use RaisesGroup with the pytest package installed" + ) + + def errisinstance( + self, + exc: builtins.type[BaseException] | tuple[builtins.type[BaseException], ...], + ) -> bool: + raise NotImplementedError( + "This is a helper method only available if you use RaisesGroup with the pytest package installed" + ) + + def getrepr( + self, + showlocals: bool = False, + style: str = "long", + abspath: bool = False, + tbfilter: bool | Callable[[_ExceptionInfo[BaseException]], Traceback] = True, + funcargs: bool = False, + truncate_locals: bool = True, + chain: bool = True, + ) -> ReprExceptionInfo | ExceptionChainRepr: + raise NotImplementedError( + "This is a helper method only available if you use RaisesGroup with the pytest package installed" + ) + -# This may bite users with type checkers not using pytest, but that should -# be rare and give quite obvious errors at runtime. -# PT013: "incorrect import of pytest". +# Type checkers are not able to do conditional types depending on installed packages, so +# we've added signatures for all helpers to _ExceptionInfo, and then always use that. +# If this ends up leading to problems, we can resort to always using _ExceptionInfo and +# users that want to use getrepr/errisinstance/exconly can write helpers on their own, or +# we reimplement them ourselves...or get this merged in upstream pytest. if TYPE_CHECKING: - from pytest import ExceptionInfo # noqa: PT013 + ExceptionInfo = _ExceptionInfo else: try: From 614f85a2d4539ca2713a0f7bd78b55f894195229 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 5 Jan 2024 13:55:22 +0100 Subject: [PATCH 28/29] fix broken test --- src/trio/_tests/test_testing_raisesgroup.py | 10 ++++++++-- src/trio/testing/_raises_group.py | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/trio/_tests/test_testing_raisesgroup.py b/src/trio/_tests/test_testing_raisesgroup.py index f9f456c610..9b6b2a6fb6 100644 --- a/src/trio/_tests/test_testing_raisesgroup.py +++ b/src/trio/_tests/test_testing_raisesgroup.py @@ -14,12 +14,18 @@ from exceptiongroup import ExceptionGroup +def wrap_escape(s: str) -> str: + return "^" + re.escape(s) + "$" + + def test_raises_group() -> None: with pytest.raises( ValueError, - match="^Invalid argument {exc} must be exception type, Matcher, or RaisesGroup.$", + match=wrap_escape( + f'Invalid argument "{TypeError()!r}" must be exception type, Matcher, or RaisesGroup.' + ), ): - RaisesGroup(ValueError()) + RaisesGroup(TypeError()) with RaisesGroup(ValueError): raise ExceptionGroup("foo", (ValueError(),)) diff --git a/src/trio/testing/_raises_group.py b/src/trio/testing/_raises_group.py index 9bed302db0..516f71f375 100644 --- a/src/trio/testing/_raises_group.py +++ b/src/trio/testing/_raises_group.py @@ -343,7 +343,7 @@ def __init__( self.is_baseexceptiongroup |= not issubclass(exc, Exception) else: raise ValueError( - f"Invalid argument {exc!r} must be exception type, Matcher, or" + f'Invalid argument "{exc!r}" must be exception type, Matcher, or' " RaisesGroup." ) From 80b70311caa97676ba1200c29e26ea353444ebac Mon Sep 17 00:00:00 2001 From: jakkdl Date: Fri, 5 Jan 2024 14:03:39 +0100 Subject: [PATCH 29/29] fix newsfragment quoting of pytest --- newsfragments/2785.feature.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/newsfragments/2785.feature.rst b/newsfragments/2785.feature.rst index 834d76be02..8dff767e4b 100644 --- a/newsfragments/2785.feature.rst +++ b/newsfragments/2785.feature.rst @@ -1,4 +1,4 @@ New helper classes: :class:`~.testing.RaisesGroup` and :class:`~.testing.Matcher`. In preparation for changing the default of ``strict_exception_groups`` to `True`, we're introducing a set of helper classes that can be used in place of `pytest.raises `_ in tests, to check for an expected `ExceptionGroup`. -These are provisional, and only planned to be supplied until there's a good solution in `pytest`. See https://github.com/pytest-dev/pytest/issues/11538 +These are provisional, and only planned to be supplied until there's a good solution in ``pytest``. See https://github.com/pytest-dev/pytest/issues/11538