Merge pull request #10921 from bluetech/tb-simplify-2

Fix hidden traceback entries of chained exceptions getting shown

changelog/1904.bugfix.rst

@@ -0,0 +1 @@
+Fixed traceback entries hidden with ``__tracebackhide__ = True`` still being shown for chained exceptions (parts after "... the above exception ..." message).

src/_pytest/_code/code.py

@@ -31,7 +31,6 @@ from typing import Type
 from typing import TYPE_CHECKING
 from typing import TypeVar
 from typing import Union
-from weakref import ref
 
 import pluggy
 
@@ -50,9 +49,9 @@ from _pytest.pathlib import absolutepath
 from _pytest.pathlib import bestrelpath
 
 if TYPE_CHECKING:
+    from typing_extensions import Final
     from typing_extensions import Literal
     from typing_extensions import SupportsIndex
-    from weakref import ReferenceType
 
     _TracebackStyle = Literal["long", "short", "line", "no", "native", "value", "auto"]
 
@@ -194,25 +193,25 @@ class Frame:
 class TracebackEntry:
     """A single entry in a Traceback."""
 
-    __slots__ = ("_rawentry", "_excinfo", "_repr_style")
+    __slots__ = ("_rawentry", "_repr_style")
 
     def __init__(
         self,
         rawentry: TracebackType,
-        excinfo: Optional["ReferenceType[ExceptionInfo[BaseException]]"] = None,
+        repr_style: Optional['Literal["short", "long"]'] = None,
     ) -> None:
-        self._rawentry = rawentry
+        self._rawentry: "Final" = rawentry
-        self._excinfo = excinfo
+        self._repr_style: "Final" = repr_style
-        self._repr_style: Optional['Literal["short", "long"]'] = None
+
+    def with_repr_style(
+        self, repr_style: Optional['Literal["short", "long"]']
+    ) -> "TracebackEntry":
+        return TracebackEntry(self._rawentry, repr_style)
 
     @property
     def lineno(self) -> int:
         return self._rawentry.tb_lineno - 1
 
-    def set_repr_style(self, mode: "Literal['short', 'long']") -> None:
-        assert mode in ("short", "long")
-        self._repr_style = mode
-
     @property
     def frame(self) -> Frame:
         return Frame(self._rawentry.tb_frame)
@@ -272,7 +271,7 @@ class TracebackEntry:
 
     source = property(getsource)
 
-    def ishidden(self) -> bool:
+    def ishidden(self, excinfo: Optional["ExceptionInfo[BaseException]"]) -> bool:
         """Return True if the current frame has a var __tracebackhide__
         resolving to True.
 
@@ -296,7 +295,7 @@ class TracebackEntry:
             else:
                 break
         if tbh and callable(tbh):
-            return tbh(None if self._excinfo is None else self._excinfo())
+            return tbh(excinfo)
         return tbh
 
     def __str__(self) -> str:
@@ -329,16 +328,14 @@ class Traceback(List[TracebackEntry]):
     def __init__(
         self,
         tb: Union[TracebackType, Iterable[TracebackEntry]],
-        excinfo: Optional["ReferenceType[ExceptionInfo[BaseException]]"] = None,
     ) -> None:
         """Initialize from given python traceback object and ExceptionInfo."""
-        self._excinfo = excinfo
         if isinstance(tb, TracebackType):
 
             def f(cur: TracebackType) -> Iterable[TracebackEntry]:
                 cur_: Optional[TracebackType] = cur
                 while cur_ is not None:
-                    yield TracebackEntry(cur_, excinfo=excinfo)
+                    yield TracebackEntry(cur_)
                     cur_ = cur_.tb_next
 
             super().__init__(f(tb))
@@ -378,7 +375,7 @@ class Traceback(List[TracebackEntry]):
                 continue
             if firstlineno is not None and x.frame.code.firstlineno != firstlineno:
                 continue
-            return Traceback(x._rawentry, self._excinfo)
+            return Traceback(x._rawentry)
         return self
 
     @overload
@@ -398,27 +395,27 @@ class Traceback(List[TracebackEntry]):
             return super().__getitem__(key)
 
     def filter(
-        self, fn: Callable[[TracebackEntry], bool] = lambda x: not x.ishidden()
+        self,
+        # TODO(py38): change to positional only.
+        _excinfo_or_fn: Union[
+            "ExceptionInfo[BaseException]",
+            Callable[[TracebackEntry], bool],
+        ],
     ) -> "Traceback":
-        """Return a Traceback instance with certain items removed
+        """Return a Traceback instance with certain items removed.
 
-        fn is a function that gets a single argument, a TracebackEntry
+        If the filter is an `ExceptionInfo`, removes all the ``TracebackEntry``s
-        instance, and should return True when the item should be added
+        which are hidden (see ishidden() above).
-        to the Traceback, False when not.
 
-        By default this removes all the TracebackEntries which are hidden
+        Otherwise, the filter is a function that gets a single argument, a
-        (see ishidden() above).
+        ``TracebackEntry`` instance, and should return True when the item should
+        be added to the ``Traceback``, False when not.
         """
-        return Traceback(filter(fn, self), self._excinfo)
+        if isinstance(_excinfo_or_fn, ExceptionInfo):
-
+            fn = lambda x: not x.ishidden(_excinfo_or_fn)  # noqa: E731
-    def getcrashentry(self) -> Optional[TracebackEntry]:
+        else:
-        """Return last non-hidden traceback entry that lead to the exception of
+            fn = _excinfo_or_fn
-        a traceback, or None if all hidden."""
+        return Traceback(filter(fn, self))
-        for i in range(-1, -len(self) - 1, -1):
-            entry = self[i]
-            if not entry.ishidden():
-                return entry
-        return None
 
     def recursionindex(self) -> Optional[int]:
         """Return the index of the frame/TracebackEntry where recursion originates if
@@ -583,7 +580,7 @@ class ExceptionInfo(Generic[E]):
     def traceback(self) -> Traceback:
         """The traceback."""
         if self._traceback is None:
-            self._traceback = Traceback(self.tb, excinfo=ref(self))
+            self._traceback = Traceback(self.tb)
         return self._traceback
 
     @traceback.setter
@@ -623,19 +620,24 @@ class ExceptionInfo(Generic[E]):
         return isinstance(self.value, exc)
 
     def _getreprcrash(self) -> Optional["ReprFileLocation"]:
-        exconly = self.exconly(tryshort=True)
+        # Find last non-hidden traceback entry that led to the exception of the
-        entry = self.traceback.getcrashentry()
+        # traceback, or None if all hidden.
-        if entry is None:
+        for i in range(-1, -len(self.traceback) - 1, -1):
-            return None
+            entry = self.traceback[i]
+            if not entry.ishidden(self):
                 path, lineno = entry.frame.code.raw.co_filename, entry.lineno
+                exconly = self.exconly(tryshort=True)
                 return ReprFileLocation(path, lineno + 1, exconly)
+        return None
 
     def getrepr(
         self,
         showlocals: bool = False,
         style: "_TracebackStyle" = "long",
         abspath: bool = False,
-        tbfilter: bool = True,
+        tbfilter: Union[
+            bool, Callable[["ExceptionInfo[BaseException]"], Traceback]
+        ] = True,
         funcargs: bool = False,
         truncate_locals: bool = True,
         chain: bool = True,
@@ -652,9 +654,15 @@ class ExceptionInfo(Generic[E]):
         :param bool abspath:
             If paths should be changed to absolute or left unchanged.
 
-        :param bool tbfilter:
+        :param tbfilter:
-            Hide entries that contain a local variable ``__tracebackhide__==True``.
+            A filter for traceback entries.
-            Ignored if ``style=="native"``.
+
+            * If false, don't hide any entries.
+            * If true, hide internal entries and entries that contain a local
+              variable ``__tracebackhide__ = True``.
+            * If a callable, delegates the filtering to the callable.
+
+            Ignored if ``style`` is ``"native"``.
 
         :param bool funcargs:
             Show fixtures ("funcargs" for legacy purposes) per traceback entry.
@@ -719,7 +727,7 @@ class FormattedExcinfo:
     showlocals: bool = False
     style: "_TracebackStyle" = "long"
     abspath: bool = True
-    tbfilter: bool = True
+    tbfilter: Union[bool, Callable[[ExceptionInfo[BaseException]], Traceback]] = True
     funcargs: bool = False
     truncate_locals: bool = True
     chain: bool = True
@@ -881,8 +889,10 @@ class FormattedExcinfo:
 
     def repr_traceback(self, excinfo: ExceptionInfo[BaseException]) -> "ReprTraceback":
         traceback = excinfo.traceback
-        if self.tbfilter:
+        if callable(self.tbfilter):
-            traceback = traceback.filter()
+            traceback = self.tbfilter(excinfo)
+        elif self.tbfilter:
+            traceback = traceback.filter(excinfo)
 
         if isinstance(excinfo.value, RecursionError):
             traceback, extraline = self._truncate_recursive_traceback(traceback)

src/_pytest/nodes.py

@@ -22,6 +22,7 @@ import _pytest._code
 from _pytest._code import getfslineno
 from _pytest._code.code import ExceptionInfo
 from _pytest._code.code import TerminalRepr
+from _pytest._code.code import Traceback
 from _pytest.compat import cached_property
 from _pytest.compat import LEGACY_PATH
 from _pytest.config import Config
@@ -432,8 +433,8 @@ class Node(metaclass=NodeMeta):
         assert current is None or isinstance(current, cls)
         return current
 
-    def _prunetraceback(self, excinfo: ExceptionInfo[BaseException]) -> None:
+    def _traceback_filter(self, excinfo: ExceptionInfo[BaseException]) -> Traceback:
-        pass
+        return excinfo.traceback
 
     def _repr_failure_py(
         self,
@@ -449,10 +450,13 @@ class Node(metaclass=NodeMeta):
                 style = "value"
         if isinstance(excinfo.value, FixtureLookupError):
             return excinfo.value.formatrepr()
+
+        tbfilter: Union[bool, Callable[[ExceptionInfo[BaseException]], Traceback]]
         if self.config.getoption("fulltrace", False):
             style = "long"
+            tbfilter = False
         else:
-            self._prunetraceback(excinfo)
+            tbfilter = self._traceback_filter
             if style == "auto":
                 style = "long"
         # XXX should excinfo.getrepr record all data and toterminal() process it?
@@ -483,7 +487,7 @@ class Node(metaclass=NodeMeta):
             abspath=abspath,
             showlocals=self.config.getoption("showlocals", False),
             style=style,
-            tbfilter=False,  # pruned already, or in --fulltrace mode.
+            tbfilter=tbfilter,
             truncate_locals=truncate_locals,
         )
 
@@ -554,13 +558,14 @@ class Collector(Node):
 
         return self._repr_failure_py(excinfo, style=tbstyle)
 
-    def _prunetraceback(self, excinfo: ExceptionInfo[BaseException]) -> None:
+    def _traceback_filter(self, excinfo: ExceptionInfo[BaseException]) -> Traceback:
         if hasattr(self, "path"):
             traceback = excinfo.traceback
             ntraceback = traceback.cut(path=self.path)
             if ntraceback == traceback:
                 ntraceback = ntraceback.cut(excludepath=tracebackcutdir)
-            excinfo.traceback = ntraceback.filter()
+            return excinfo.traceback.filter(excinfo)
+        return excinfo.traceback
 
 
 def _check_initialpaths_for_relpath(session: "Session", path: Path) -> Optional[str]:

src/_pytest/python.py

@@ -35,6 +35,7 @@ from _pytest._code import filter_traceback
 from _pytest._code import getfslineno
 from _pytest._code.code import ExceptionInfo
 from _pytest._code.code import TerminalRepr
+from _pytest._code.code import Traceback
 from _pytest._io import TerminalWriter
 from _pytest._io.saferepr import saferepr
 from _pytest.compat import ascii_escaped
@@ -1801,7 +1802,7 @@ class Function(PyobjMixin, nodes.Item):
     def setup(self) -> None:
         self._request._fillfixtures()
 
-    def _prunetraceback(self, excinfo: ExceptionInfo[BaseException]) -> None:
+    def _traceback_filter(self, excinfo: ExceptionInfo[BaseException]) -> Traceback:
         if hasattr(self, "_obj") and not self.config.getoption("fulltrace", False):
             code = _pytest._code.Code.from_function(get_real_func(self.obj))
             path, firstlineno = code.path, code.firstlineno
@@ -1813,14 +1814,21 @@ class Function(PyobjMixin, nodes.Item):
                     ntraceback = ntraceback.filter(filter_traceback)
                     if not ntraceback:
                         ntraceback = traceback
+            ntraceback = ntraceback.filter(excinfo)
 
-            excinfo.traceback = ntraceback.filter()
             # issue364: mark all but first and last frames to
             # only show a single-line message for each frame.
             if self.config.getoption("tbstyle", "auto") == "auto":
-                if len(excinfo.traceback) > 2:
+                if len(ntraceback) > 2:
-                    for entry in excinfo.traceback[1:-1]:
+                    ntraceback = Traceback(
-                        entry.set_repr_style("short")
+                        entry
+                        if i == 0 or i == len(ntraceback) - 1
+                        else entry.with_repr_style("short")
+                        for i, entry in enumerate(ntraceback)
+                    )
+
+            return ntraceback
+        return excinfo.traceback
 
     # TODO: Type ignored -- breaks Liskov Substitution.
     def repr_failure(  # type: ignore[override]

src/_pytest/unittest.py

@@ -334,15 +334,16 @@ class TestCaseFunction(Function):
             finally:
                 delattr(self._testcase, self.name)
 
-    def _prunetraceback(
+    def _traceback_filter(
         self, excinfo: _pytest._code.ExceptionInfo[BaseException]
-    ) -> None:
+    ) -> _pytest._code.Traceback:
-        super()._prunetraceback(excinfo)
+        traceback = super()._traceback_filter(excinfo)
-        traceback = excinfo.traceback.filter(
+        ntraceback = traceback.filter(
-            lambda x: not x.frame.f_globals.get("__unittest")
+            lambda x: not x.frame.f_globals.get("__unittest"),
         )
-        if traceback:
+        if not ntraceback:
-            excinfo.traceback = traceback
+            ntraceback = traceback
+        return ntraceback
 
 
 @hookimpl(tryfirst=True)

testing/code/test_excinfo.py

@@ -11,7 +11,7 @@ from typing import Tuple
 from typing import TYPE_CHECKING
 from typing import Union
 
-import _pytest
+import _pytest._code
 import pytest
 from _pytest._code.code import ExceptionChainRepr
 from _pytest._code.code import ExceptionInfo
@@ -186,7 +186,7 @@ class TestTraceback_f_g_h:
 
     def test_traceback_filter(self):
         traceback = self.excinfo.traceback
-        ntraceback = traceback.filter()
+        ntraceback = traceback.filter(self.excinfo)
         assert len(ntraceback) == len(traceback) - 1
 
     @pytest.mark.parametrize(
@@ -217,7 +217,7 @@ class TestTraceback_f_g_h:
 
         excinfo = pytest.raises(ValueError, h)
         traceback = excinfo.traceback
-        ntraceback = traceback.filter()
+        ntraceback = traceback.filter(excinfo)
         print(f"old: {traceback!r}")
         print(f"new: {ntraceback!r}")
 
@@ -290,7 +290,7 @@ class TestTraceback_f_g_h:
         excinfo = pytest.raises(ValueError, fail)
         assert excinfo.traceback.recursionindex() is None
 
-    def test_traceback_getcrashentry(self):
+    def test_getreprcrash(self):
         def i():
             __tracebackhide__ = True
             raise ValueError
@@ -306,15 +306,13 @@ class TestTraceback_f_g_h:
             g()
 
         excinfo = pytest.raises(ValueError, f)
-        tb = excinfo.traceback
+        reprcrash = excinfo._getreprcrash()
-        entry = tb.getcrashentry()
+        assert reprcrash is not None
-        assert entry is not None
         co = _pytest._code.Code.from_function(h)
-        assert entry.frame.code.path == co.path
+        assert reprcrash.path == str(co.path)
-        assert entry.lineno == co.firstlineno + 1
+        assert reprcrash.lineno == co.firstlineno + 1 + 1
-        assert entry.frame.code.name == "h"
 
-    def test_traceback_getcrashentry_empty(self):
+    def test_getreprcrash_empty(self):
         def g():
             __tracebackhide__ = True
             raise ValueError
@@ -324,7 +322,7 @@ class TestTraceback_f_g_h:
             g()
 
         excinfo = pytest.raises(ValueError, f)
-        assert excinfo.traceback.getcrashentry() is None
+        assert excinfo._getreprcrash() is None
 
 
 def test_excinfo_exconly():
@@ -626,7 +624,7 @@ raise ValueError()
         """
         )
         excinfo = pytest.raises(ValueError, mod.func1)
-        excinfo.traceback = excinfo.traceback.filter()
+        excinfo.traceback = excinfo.traceback.filter(excinfo)
         p = FormattedExcinfo()
         reprtb = p.repr_traceback_entry(excinfo.traceback[-1])
 
@@ -659,7 +657,7 @@ raise ValueError()
         """
         )
         excinfo = pytest.raises(ValueError, mod.func1, "m" * 90, 5, 13, "z" * 120)
-        excinfo.traceback = excinfo.traceback.filter()
+        excinfo.traceback = excinfo.traceback.filter(excinfo)
         entry = excinfo.traceback[-1]
         p = FormattedExcinfo(funcargs=True)
         reprfuncargs = p.repr_args(entry)
@@ -686,7 +684,7 @@ raise ValueError()
         """
         )
         excinfo = pytest.raises(ValueError, mod.func1, "a", "b", c="d")
-        excinfo.traceback = excinfo.traceback.filter()
+        excinfo.traceback = excinfo.traceback.filter(excinfo)
         entry = excinfo.traceback[-1]
         p = FormattedExcinfo(funcargs=True)
         reprfuncargs = p.repr_args(entry)
@@ -960,7 +958,7 @@ raise ValueError()
         """
         )
         excinfo = pytest.raises(ValueError, mod.f)
-        excinfo.traceback = excinfo.traceback.filter()
+        excinfo.traceback = excinfo.traceback.filter(excinfo)
         repr = excinfo.getrepr()
         repr.toterminal(tw_mock)
         assert tw_mock.lines[0] == ""
@@ -994,7 +992,7 @@ raise ValueError()
         )
         excinfo = pytest.raises(ValueError, mod.f)
         tmp_path.joinpath("mod.py").unlink()
-        excinfo.traceback = excinfo.traceback.filter()
+        excinfo.traceback = excinfo.traceback.filter(excinfo)
         repr = excinfo.getrepr()
         repr.toterminal(tw_mock)
         assert tw_mock.lines[0] == ""
@@ -1026,7 +1024,7 @@ raise ValueError()
         )
         excinfo = pytest.raises(ValueError, mod.f)
         tmp_path.joinpath("mod.py").write_text("asdf")
-        excinfo.traceback = excinfo.traceback.filter()
+        excinfo.traceback = excinfo.traceback.filter(excinfo)
         repr = excinfo.getrepr()
         repr.toterminal(tw_mock)
         assert tw_mock.lines[0] == ""
@@ -1123,9 +1121,11 @@ raise ValueError()
         """
         )
         excinfo = pytest.raises(ValueError, mod.f)
-        excinfo.traceback = excinfo.traceback.filter()
+        excinfo.traceback = excinfo.traceback.filter(excinfo)
-        excinfo.traceback[1].set_repr_style("short")
+        excinfo.traceback = _pytest._code.Traceback(
-        excinfo.traceback[2].set_repr_style("short")
+            entry if i not in (1, 2) else entry.with_repr_style("short")
+            for i, entry in enumerate(excinfo.traceback)
+        )
         r = excinfo.getrepr(style="long")
         r.toterminal(tw_mock)
         for line in tw_mock.lines:
@@ -1391,7 +1391,7 @@ raise ValueError()
         with pytest.raises(TypeError) as excinfo:
             mod.f()
         # previously crashed with `AttributeError: list has no attribute get`
-        excinfo.traceback.filter()
+        excinfo.traceback.filter(excinfo)
 
 
 @pytest.mark.parametrize("style", ["short", "long"])
@@ -1603,3 +1603,48 @@ def test_all_entries_hidden(pytester: Pytester, tbstyle: str) -> None:
         result.stdout.fnmatch_lines(["*ZeroDivisionError: division by zero"])
     if tbstyle not in ("line", "native"):
         result.stdout.fnmatch_lines(["All traceback entries are hidden.*"])
+
+
+def test_hidden_entries_of_chained_exceptions_are_not_shown(pytester: Pytester) -> None:
+    """Hidden entries of chained exceptions are not shown (#1904)."""
+    p = pytester.makepyfile(
+        """
+        def g1():
+            __tracebackhide__ = True
+            str.does_not_exist
+
+        def f3():
+            __tracebackhide__ = True
+            1 / 0
+
+        def f2():
+            try:
+                f3()
+            except Exception:
+                g1()
+
+        def f1():
+            __tracebackhide__ = True
+            f2()
+
+        def test():
+            f1()
+        """
+    )
+    result = pytester.runpytest(str(p), "--tb=short")
+    assert result.ret == 1
+    result.stdout.fnmatch_lines(
+        [
+            "*.py:11: in f2",
+            "    f3()",
+            "E   ZeroDivisionError: division by zero",
+            "",
+            "During handling of the above exception, another exception occurred:",
+            "*.py:20: in test",
+            "    f1()",
+            "*.py:13: in f2",
+            "    g1()",
+            "E   AttributeError:*'does_not_exist'",
+        ],
+        consecutive=True,
+    )

testing/python/collect.py

@@ -1003,9 +1003,9 @@ class TestTracebackCutting:
         with pytest.raises(pytest.skip.Exception) as excinfo:
             pytest.skip("xxx")
         assert excinfo.traceback[-1].frame.code.name == "skip"
-        assert excinfo.traceback[-1].ishidden()
+        assert excinfo.traceback[-1].ishidden(excinfo)
         assert excinfo.traceback[-2].frame.code.name == "test_skip_simple"
-        assert not excinfo.traceback[-2].ishidden()
+        assert not excinfo.traceback[-2].ishidden(excinfo)
 
     def test_traceback_argsetup(self, pytester: Pytester) -> None:
         pytester.makeconftest(