diff --git a/changelog/14324.bugfix.rst b/changelog/14324.bugfix.rst new file mode 100644 index 00000000000..11c40192b67 --- /dev/null +++ b/changelog/14324.bugfix.rst @@ -0,0 +1 @@ +Fix ``pytest.RaisesGroup`` incorrectly calling the ``check`` callback with contained exceptions instead of only the exception group. diff --git a/src/_pytest/raises.py b/src/_pytest/raises.py index 75eea7d8cc9..ab690310df7 100644 --- a/src/_pytest/raises.py +++ b/src/_pytest/raises.py @@ -1207,14 +1207,35 @@ def matches( reason = ( cast(str, self._fail_reason) + f" on the {type(exception).__name__}" ) + + suggest_subexception_check = False if ( - len(actual_exceptions) == len(self.expected_exceptions) == 1 + self.check is not None + and 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) ): + annotations = getattr(self.check, "__annotations__", {}) + param_names = [name for name in annotations if name != "return"] + if param_names: + param_annotation = annotations[param_names[0]] + + if isinstance(param_annotation, str): + suggest_subexception_check = ( + "ExceptionGroup" not in param_annotation + and "BaseExceptionGroup" not in param_annotation + ) + else: + origin = get_origin(param_annotation) or param_annotation + if isinstance(origin, type): + suggest_subexception_check = not issubclass( + origin, BaseExceptionGroup + ) + + if suggest_subexception_check: self._fail_reason = reason + ( - f", but did return True for the expected {self._repr_expected(expected)}." + f", but the single contained exception matches the expected " + f"{self._repr_expected(expected)}." f" You might want RaisesGroup(RaisesExc({expected.__name__}, check=<...>))" ) else: diff --git a/testing/python/raises_group.py b/testing/python/raises_group.py index e5e3b5cd2dc..ce3bcf14d70 100644 --- a/testing/python/raises_group.py +++ b/testing/python/raises_group.py @@ -412,9 +412,12 @@ def is_exc(e: ExceptionGroup[ValueError]) -> bool: return e is exc is_exc_repr = repr_callable(is_exc) + + # this should pass (same object) with RaisesGroup(ValueError, check=is_exc): raise exc + # this should fail WITHOUT suggestion with ( fails_raises_group( f"check {is_exc_repr} did not return True on the ExceptionGroup" @@ -426,16 +429,29 @@ def is_exc(e: ExceptionGroup[ValueError]) -> bool: def is_value_error(e: BaseException) -> bool: return isinstance(e, ValueError) - # helpful suggestion if the user thinks the check is for the sub-exception + # this should fail WITH suggestion (because check looks like it's for inner 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, but the single contained exception matches the expected ValueError. You might want RaisesGroup(RaisesExc(ValueError, check=<...>))" ), RaisesGroup(ValueError, check=is_value_error), ): raise ExceptionGroup("", (ValueError(),)) +def test_check_called_only_with_group() -> None: + seen = [] + + def check(exc_group: ExceptionGroup[ValueError]) -> bool: + seen.append(type(exc_group)) + return len(exc_group.exceptions) == 1 + + with RaisesGroup(ValueError, match="Main message", check=check): + raise ExceptionGroup("Main message", [ValueError("foo")]) + + assert seen == [ExceptionGroup] + + def test_unwrapped_match_check() -> None: def my_check(e: object) -> bool: # pragma: no cover return True