Fix ExceptionGroup traceback filtering of pytest internals

[gesslerpd]

Edited the test to fail on main branch and was able to fix rather than submitting an issue. See sample output of failing test in comment below for reference.

This fix preserves the existing fallback logic for exception groups #9159 and adds traceback filtering only.

[gesslerpd]

Test failed with results such as this previously, the exception groups tracebacks show lots of pytest internal frames that are filtered from other exceptions.

During handling of the above exception, another exception occurred:

  + Exception Group Traceback (most recent call last):
  |   File "...\site-packages\_pytest\runner.py", line 345, in from_call
  |     result: TResult | None = func()
  |                              ~~~~^^
  |   File "...\site-packages\_pytest\runner.py", line 246, in <lambda>
  |     lambda: runtest_hook(item=item, **kwds), when=when, reraise=reraise
  |             ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^
  |   File "...\site-packages\pluggy\_hooks.py", line 513, in __call__
  |     return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
  |            ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |   File "...\site-packages\pluggy\_manager.py", line 120, in _hookexec
  |     return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
  |            ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |   File "...\site-packages\pluggy\_manager.py", line 480, in traced_hookexec
  |     return outcome.get_result()
  |            ~~~~~~~~~~~~~~~~~~^^
  |   File "...\site-packages\pluggy\_result.py", line 100, in get_result
  |     raise exc.with_traceback(exc.__traceback__)
  |   File "...\site-packages\pluggy\_result.py", line 62, in from_call
  |     result = func()
  |   File "...\site-packages\pluggy\_manager.py", line 477, in <lambda>
  |     lambda: oldcall(hook_name, hook_impls, caller_kwargs, firstresult)
  |             ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |   File "...\site-packages\pluggy\_callers.py", line 139, in _multicall
  |     raise exception.with_traceback(exception.__traceback__)
  |   File "...\site-packages\pluggy\_callers.py", line 122, in _multicall
  |     teardown.throw(exception)  # type: ignore[union-attr]
  |     ~~~~~~~~~~~~~~^^^^^^^^^^^
  |   File "...\site-packages\_pytest\logging.py", line 847, in pytest_runtest_call
  |     yield from self._runtest_for(item, "call")
  |   File "...\site-packages\_pytest\logging.py", line 830, in _runtest_for
  |     yield
  |   File "...\site-packages\pluggy\_callers.py", line 122, in _multicall
  |     teardown.throw(exception)  # type: ignore[union-attr]
  |     ~~~~~~~~~~~~~~^^^^^^^^^^^
  |   File "...\site-packages\_pytest\capture.py", line 900, in pytest_runtest_call
  |     return (yield)
  |             ^^^^^
  |   File "...\site-packages\pluggy\_callers.py", line 122, in _multicall
  |     teardown.throw(exception)  # type: ignore[union-attr]
  |     ~~~~~~~~~~~~~~^^^^^^^^^^^
  |   File "...\site-packages\_pytest\skipping.py", line 263, in pytest_runtest_call
  |     return (yield)
  |             ^^^^^
  |   File "...\site-packages\pluggy\_callers.py", line 103, in _multicall
  |     res = hook_impl.function(*args)
  |   File "...\site-packages\_pytest\runner.py", line 178, in pytest_runtest_call
  |     item.runtest()
  |     ~~~~~~~~~~~~^^
  |   File "...\site-packages\_pytest\python.py", line 1663, in runtest
  |     self.ihook.pytest_pyfunc_call(pyfuncitem=self)
  |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
  |   File "...\site-packages\pluggy\_hooks.py", line 513, in __call__
  |     return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
  |            ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |   File "...\site-packages\pluggy\_manager.py", line 120, in _hookexec
  |     return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
  |            ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |   File "...\site-packages\pluggy\_manager.py", line 480, in traced_hookexec
  |     return outcome.get_result()
  |            ~~~~~~~~~~~~~~~~~~^^
  |   File "...\site-packages\pluggy\_result.py", line 100, in get_result
  |     raise exc.with_traceback(exc.__traceback__)
  |   File "...\site-packages\pluggy\_result.py", line 62, in from_call
  |     result = func()
  |   File "...\site-packages\pluggy\_manager.py", line 477, in <lambda>
  |     lambda: oldcall(hook_name, hook_impls, caller_kwargs, firstresult)
  |             ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |   File "...\site-packages\pluggy\_callers.py", line 139, in _multicall
  |     raise exception.with_traceback(exception.__traceback__)
  |   File "...\site-packages\pluggy\_callers.py", line 103, in _multicall
  |     res = hook_impl.function(*args)
  |   File "...\site-packages\_pytest\python.py", line 156, in pytest_pyfunc_call
  |     result = testfunction(**testargs)
  |   File "...\pytest-of-unknown\pytest-67\test_native_exceptiongroup6\test_excgroup.py", line 37, in test
  |     outer("none", "another")
  |     ~~~~~^^^^^^^^^^^^^^^^^^^
  |   File "...\pytest-of-unknown\pytest-67\test_native_exceptiongroup6\test_excgroup.py", line 26, in outer
  |     inner(inner_chain)
  |     ~~~~~^^^^^^^^^^^^^
  |   File "...\pytest-of-unknown\pytest-67\test_native_exceptiongroup6\test_excgroup.py", line 22, in inner
  |     raise BaseExceptionGroup("Oops", excs)
  | BaseExceptionGroup: Oops (2 sub-exceptions)
  +-+---------------- 1 ----------------
    | Traceback (most recent call last):
    |   File "...\pytest-of-unknown\pytest-67\test_native_exceptiongroup6\test_excgroup.py", line 10, in inner
    |     callback()
    |     ~~~~~~~~^^
    |   File "...\pytest-of-unknown\pytest-67\test_native_exceptiongroup6\test_excgroup.py", line 3, in f
    |     def f(): raise ValueError("From f()")
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    | ValueError: From f()
    +---------------- 2 ----------------
    | Traceback (most recent call last):
    |   File "...\pytest-of-unknown\pytest-67\test_native_exceptiongroup6\test_excgroup.py", line 10, in inner
    |     callback()
    |     ~~~~~~~~^^
    |   File "...\pytest-of-unknown\pytest-67\test_native_exceptiongroup6\test_excgroup.py", line 4, in g
    |     def g(): raise BaseException("From g()")
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    | BaseException: From g()
    +------------------------------------
=========================== short test summary info ===========================
FAILED test_excgroup.py::test - BaseExceptionGroup: Oops (2 sub-exceptions)

[gesslerpd]

@Zac-HD @jakkdl you both might have expertise in these areas, the test I edited and fallback logic for exception groups originates in #10209

[jakkdl]

Awesome to see a new pytest contributor! And very happy you're improving my dirty hack for exceptiongroups :)

A note when submitting PR's is to avoid extraneous changes that muddles the diff and makes it harder to see what's actually being changed. The rename of the traceback variable to traceback_, as well as moving the call in _truncate_recursive_traceback to its own method, both qualifies as that.
If you really do think those changes makes the code non-trivially better, they should be in their own commit, so reviewers can view the diff of other commits separately.

This PR is really just ~3-4 lines changed, but it looks much bigger.

Shadowing the traceback module isn't great for sure, so I'm probably in favor of keeping that change.
Adding _repr_exception_group_traceback I'm against. Ideally ReprTraceback should just support exceptiongroups so in the long term the method shouldn't be needed.

[jakkdl]

Have you checked that we always only need to filter the top ExceptionGroup? I guess for exceptions inside the group to have internal frames we'd need to be collecting the group inside pytest itself .. which maybe can happen with some plugin? Or if the test is calling pytest helpers and get an exception in there?

If you don't come up with any reasonable scenario where it'd happen I'd just pop a comment noting that we don't filter any sub-exceptions since they shouldn't have any internal frames.

[nicoddemus]

Thanks a lot @gesslerpd!

I took the liberty of cleaning up the code a bit (the rationale is in the commit message), otherwise looks great to me.

[nicoddemus]

If you don't come up with any reasonable scenario where it'd happen I'd just pop a comment noting that we don't filter any sub-exceptions since they shouldn't have any internal frames.

That's a good point... however I cannot think of a scenario right now where that could happen... 🤔

[gesslerpd]

If you don't come up with any reasonable scenario where it'd happen I'd just pop a comment noting that we don't filter any sub-exceptions since they shouldn't have any internal frames.

The test is parametrized and only fails in 3 cases (6 total because of the backport tests) when the outer_chain="none" and I haven't seen any other cases where the pytest internal frames are included.

[gesslerpd]

@nicoddemus The docs are failing on the typehint refactor not sure best way to fix that (edit: I took it out of the TYPE_CHECKING conditional so docs build can see it, nvm that was a bad idea I now repeated the hint directly just for the doc referencing part of code not optimal but it works)

/home/docs/checkouts/readthedocs.org/user_builds/pytest/checkouts/13380/doc/en/reference/reference.rst:969: WARNING: py:class reference target not found: TracebackFilter [ref.class]

[nicoddemus]

I now repeated the hint directly just for the doc referencing part of code not optimal but it works

Thanks for taking a look, here's the original error for reference:

/home/docs/checkouts/readthedocs.org/user_builds/pytest/checkouts/13380/doc/en/reference/reference.rst:969: WARNING: py:class reference target not found: TracebackFilter [ref.class]

However using the type directly kind defeats the purpose of the alias -- I will attempt to find a workaround later, otherwise I guess we will need to drop the type alias.

[jakkdl]

looking good! Still an ugly hack, but slightly less ugly~ ✨

[Zac-HD]

Thanks!