From 88779fb14266909fe17358ce1ea54d97271b23d0 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 6 Mar 2025 15:34:29 +0100 Subject: [PATCH 1/3] improve raisesgroup code coverage --- src/_pytest/_code/code.py | 3 ++- src/_pytest/raises_group.py | 7 +------ testing/python/raises.py | 2 +- testing/python/raises_group.py | 29 ++++++++++++++++++++--------- 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/src/_pytest/_code/code.py b/src/_pytest/_code/code.py index b57569dff98..2c872df3008 100644 --- a/src/_pytest/_code/code.py +++ b/src/_pytest/_code/code.py @@ -470,7 +470,8 @@ def stringify_exception( HTTPError = getattr(sys.modules.get("urllib.error", None), "HTTPError", ()) if sys.version_info < (3, 12) and isinstance(exc, HTTPError): notes = [] - else: + else: # pragma: no cover + # exception not related to above bug, reraise raise if not include_subexception_msg and isinstance(exc, BaseExceptionGroup): message = exc.message diff --git a/src/_pytest/raises_group.py b/src/_pytest/raises_group.py index f60bacb7184..d04275ed747 100644 --- a/src/_pytest/raises_group.py +++ b/src/_pytest/raises_group.py @@ -213,7 +213,7 @@ def _parse_exc( self.is_baseexception = True return cast(type[BaseExcT_1], origin_exc) else: - # I kinda think this should be a TypeError... + # TODO: I kinda think this should be a TypeError... raise ValueError( f"Only `ExceptionGroup[Exception]` or `BaseExceptionGroup[BaseExeption]` " f"are accepted as generic types but got `{exc}`. " @@ -424,11 +424,6 @@ def __enter__(self) -> ExceptionInfo[BaseExcT_co_default]: self.excinfo: ExceptionInfo[BaseExcT_co_default] = ExceptionInfo.for_later() return self.excinfo - def expected_type(self) -> str: - if self.expected_exceptions == (): - return "BaseException" - return _exception_type_name(self.expected_exceptions) - # TODO: move common code into superclass def __exit__( self, diff --git a/testing/python/raises.py b/testing/python/raises.py index 5dafef7a78d..24e5ecf22b7 100644 --- a/testing/python/raises.py +++ b/testing/python/raises.py @@ -219,7 +219,7 @@ def __call__(self): elif method == "with_group": with pytest.RaisesGroup(ValueError, allow_unwrapped=True): t() - else: + else: # pragma: no cover raise AssertionError("bad parametrization") # ensure both forms of pytest.raises don't leave exceptions in sys.exc_info() diff --git a/testing/python/raises_group.py b/testing/python/raises_group.py index 2619eb41c1d..0dc2a58a1da 100644 --- a/testing/python/raises_group.py +++ b/testing/python/raises_group.py @@ -458,12 +458,16 @@ def my_check(e: object) -> bool: # pragma: no cover assert RaisesGroup(ValueError, match="bar").matches(exc.value) -def test_RaisesGroup_matches() -> None: +def test_matches() -> None: rg = RaisesGroup(ValueError) assert not rg.matches(None) assert not rg.matches(ValueError()) assert rg.matches(ExceptionGroup("", (ValueError(),))) + re = RaisesExc(ValueError) + assert not re.matches(None) + assert re.matches(ValueError()) + def test_message() -> None: def check_message( @@ -884,11 +888,13 @@ def test_assert_message_nested() -> None: ) +# CI always runs with hypothesis, but this is not a critical test - it overlaps +# with several others @pytest.mark.skipif( "hypothesis" in sys.modules, reason="hypothesis may have monkeypatched _check_repr", ) -def test_check_no_patched_repr() -> None: +def test_check_no_patched_repr() -> None: # pragma: no cover # We make `_check_repr` monkeypatchable to avoid this very ugly and verbose # repr. The other tests that use `check` make use of `_check_repr` so they'll # continue passing in case it is patched - but we have this one test that @@ -1288,15 +1294,20 @@ def test_parametrizing_conditional_raisesgroup( def test_annotated_group() -> None: # repr depends on if exceptiongroup backport is being used or not t = repr(ExceptionGroup[ValueError]) - fail_msg = wrap_escape( - f"Only `ExceptionGroup[Exception]` or `BaseExceptionGroup[BaseExeption]` are accepted as generic types but got `{t}`. As `raises` will catch all instances of the specified group regardless of the generic argument specific nested exceptions has to be checked with `RaisesGroup`." - ) + msg = "Only `ExceptionGroup[Exception]` or `BaseExceptionGroup[BaseExeption]` are accepted as generic types but got `{}`. As `raises` will catch all instances of the specified group regardless of the generic argument specific nested exceptions has to be checked with `RaisesGroup`." + + fail_msg = wrap_escape(msg.format(t)) with pytest.raises(ValueError, match=fail_msg): - with RaisesGroup(ExceptionGroup[ValueError]): - ... # pragma: no cover + RaisesGroup(ExceptionGroup[ValueError]) with pytest.raises(ValueError, match=fail_msg): - with RaisesExc(ExceptionGroup[ValueError]): - ... # pragma: no cover + RaisesExc(ExceptionGroup[ValueError]) + with pytest.raises( + ValueError, + match=wrap_escape(msg.format(repr(BaseExceptionGroup[KeyboardInterrupt]))), + ): + with RaisesExc(BaseExceptionGroup[KeyboardInterrupt]): + raise BaseExceptionGroup("", [KeyboardInterrupt()]) + with RaisesGroup(ExceptionGroup[Exception]): raise ExceptionGroup( "", [ExceptionGroup("", [ValueError(), ValueError(), ValueError()])] From 4c09a6a6264a282f9b182160a912d3c78427dca8 Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 6 Mar 2025 16:38:24 +0100 Subject: [PATCH 2/3] fix coverage of a weird branch in python_api, explicitify match in other test --- testing/python/raises.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/testing/python/raises.py b/testing/python/raises.py index 24e5ecf22b7..333e273db6a 100644 --- a/testing/python/raises.py +++ b/testing/python/raises.py @@ -36,6 +36,19 @@ def test_raises_does_not_allow_none(self): # so we can ignore Mypy telling us that None is invalid. pytest.raises(expected_exception=None) # type: ignore + # it's unclear if this message is helpful, and if it is, should it trigger more + # liberally? Usually you'd get a TypeError here + def test_raises_false_and_arg(self): + with pytest.raises( + ValueError, + match=wrap_escape( + "Expected an exception type or a tuple of exception types, but got `False`. " + "Raising exceptions is already understood as failing the test, so you don't need " + "any special code to say 'this should never raise an exception'." + ), + ): + pytest.raises(False, int) # type: ignore[call-overload] + def test_raises_does_not_allow_empty_tuple(self): with pytest.raises( ValueError, @@ -337,10 +350,15 @@ def __class__(self): def test_raises_context_manager_with_kwargs(self): with pytest.raises(expected_exception=ValueError): raise ValueError - with pytest.raises(TypeError) as excinfo: + with pytest.raises( + TypeError, + match=wrap_escape( + "Unexpected keyword arguments passed to pytest.raises: foo\n" + "Use context-manager form instead?" + ), + ): with pytest.raises(OSError, foo="bar"): # type: ignore[call-overload] pass - assert "Unexpected keyword arguments" in str(excinfo.value) def test_expected_exception_is_not_a_baseexception(self) -> None: with pytest.raises(TypeError) as excinfo: From e96474541dfe4645a2132283a5dcf347e22d6bfb Mon Sep 17 00:00:00 2001 From: jakkdl Date: Thu, 6 Mar 2025 17:55:42 +0100 Subject: [PATCH 3/3] remove comment --- src/_pytest/raises_group.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/_pytest/raises_group.py b/src/_pytest/raises_group.py index d04275ed747..51ab8aafaf4 100644 --- a/src/_pytest/raises_group.py +++ b/src/_pytest/raises_group.py @@ -213,7 +213,6 @@ def _parse_exc( self.is_baseexception = True return cast(type[BaseExcT_1], origin_exc) else: - # TODO: I kinda think this should be a TypeError... raise ValueError( f"Only `ExceptionGroup[Exception]` or `BaseExceptionGroup[BaseExeption]` " f"are accepted as generic types but got `{exc}`. "