diff --git a/changelog/14327.bugfix.rst b/changelog/14327.bugfix.rst new file mode 100644 index 00000000000..e1ad3a03468 --- /dev/null +++ b/changelog/14327.bugfix.rst @@ -0,0 +1 @@ +Fixed ``RaisesGroup.matches()`` so group-level ``check=`` callbacks are no longer called again on the first nested exception while building suggestion text. diff --git a/src/_pytest/raises.py b/src/_pytest/raises.py index 75eea7d8cc9..cbd0786a5c7 100644 --- a/src/_pytest/raises.py +++ b/src/_pytest/raises.py @@ -1210,12 +1210,12 @@ def matches( if ( len(actual_exceptions) == len(self.expected_exceptions) == 1 and isinstance(expected := self.expected_exceptions[0], type) - # we explicitly break typing here :) - and self._check_check(actual_exceptions[0]) # type: ignore[arg-type] + and isinstance(actual_exceptions[0], expected) ): self._fail_reason = reason + ( - f", but did return True for the expected {self._repr_expected(expected)}." - f" You might want RaisesGroup(RaisesExc({expected.__name__}, check=<...>))" + f". If you meant to check the sub-exception instead of the group," + f" you might want " + f"RaisesGroup(RaisesExc({expected.__name__}, check=<...>))" ) else: self._fail_reason = reason diff --git a/testing/python/raises_group.py b/testing/python/raises_group.py index e5e3b5cd2dc..ff5b83009f0 100644 --- a/testing/python/raises_group.py +++ b/testing/python/raises_group.py @@ -417,7 +417,7 @@ def is_exc(e: ExceptionGroup[ValueError]) -> bool: with ( fails_raises_group( - f"check {is_exc_repr} did not return True on the ExceptionGroup" + f"check {is_exc_repr} did not return True on the ExceptionGroup. If you meant to check the sub-exception instead of the group, you might want RaisesGroup(RaisesExc(ValueError, check=<...>))" ), RaisesGroup(ValueError, check=is_exc), ): @@ -429,13 +429,49 @@ def is_value_error(e: BaseException) -> bool: # helpful suggestion if the user thinks the check is for the sub-exception with ( fails_raises_group( - f"check {is_value_error} did not return True on the ExceptionGroup, but did return True for the expected ValueError. You might want RaisesGroup(RaisesExc(ValueError, check=<...>))" + f"check {is_value_error} did not return True on the ExceptionGroup. If you meant to check the sub-exception instead of the group, you might want RaisesGroup(RaisesExc(ValueError, check=<...>))" ), RaisesGroup(ValueError, check=is_value_error), ): raise ExceptionGroup("", (ValueError(),)) +def test_check_is_only_called_on_the_group() -> None: + calls: list[type[BaseException]] = [] + + def check(exc_group: ExceptionGroup[ValueError]) -> bool: + calls.append(type(exc_group)) + return False + + with ( + fails_raises_group( + f"check {repr_callable(check)} did not return True on the ExceptionGroup. If you meant to check the sub-exception instead of the group, you might want RaisesGroup(RaisesExc(ValueError, check=<...>))" + ), + RaisesGroup(ValueError, check=check), + ): + raise ExceptionGroup("", (ValueError(),)) + + assert calls == [ExceptionGroup] + + +def test_matches_check_is_only_called_on_the_group() -> None: + calls: list[type[BaseException]] = [] + + def check(exc_group: ExceptionGroup[ValueError]) -> bool: + calls.append(type(exc_group)) + return False + + matcher = RaisesGroup(ValueError, check=check) + + assert not matcher.matches(ExceptionGroup("", (ValueError(),))) + assert matcher.fail_reason == ( + f"check {repr_callable(check)} did not return True on the ExceptionGroup. " + "If you meant to check the sub-exception instead of the group, you might " + "want RaisesGroup(RaisesExc(ValueError, check=<...>))" + ) + assert calls == [ExceptionGroup] + + def test_unwrapped_match_check() -> None: def my_check(e: object) -> bool: # pragma: no cover return True