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_CHECKING
 from typing import TypeVar
 from typing import Union
-from weakref import ref
 
 import pluggy
 
@@ -50,9 +49,9 @@
 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 @@ def getargs(self, var: bool = False):
 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._excinfo = excinfo
-        self._repr_style: Optional['Literal["short", "long"]'] = None
+        self._rawentry: "Final" = rawentry
+        self._repr_style: "Final" = repr_style
+
+    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 @@ def getsource(
 
     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 @@ def ishidden(self) -> bool:
             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 @@ def cut(
                 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 @@ def __getitem__(
             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
-        instance, and should return True when the item should be added
-        to the Traceback, False when not.
+        If the filter is an `ExceptionInfo`, removes all the ``TracebackEntry``s
+        which are hidden (see ishidden() above).
 
-        By default this removes all the TracebackEntries which are hidden
-        (see ishidden() above).
+        Otherwise, the filter is a function that gets a single argument, a
+        ``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)
-
-    def getcrashentry(self) -> Optional[TracebackEntry]:
-        """Return last non-hidden traceback entry that lead to the exception of
-        a traceback, or None if all hidden."""
-        for i in range(-1, -len(self) - 1, -1):
-            entry = self[i]
-            if not entry.ishidden():
-                return entry
-        return None
+        if isinstance(_excinfo_or_fn, ExceptionInfo):
+            fn = lambda x: not x.ishidden(_excinfo_or_fn)  # noqa: E731
+        else:
+            fn = _excinfo_or_fn
+        return Traceback(filter(fn, self))
 
     def recursionindex(self) -> Optional[int]:
         """Return the index of the frame/TracebackEntry where recursion originates if
@@ -583,7 +580,7 @@ def typename(self) -> str:
     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 @@ def errisinstance(
         return isinstance(self.value, exc)
 
     def _getreprcrash(self) -> Optional["ReprFileLocation"]:
-        exconly = self.exconly(tryshort=True)
-        entry = self.traceback.getcrashentry()
-        if entry is None:
-            return None
-        path, lineno = entry.frame.code.raw.co_filename, entry.lineno
-        return ReprFileLocation(path, lineno + 1, exconly)
+        # Find last non-hidden traceback entry that led to the exception of the
+        # traceback, or None if all hidden.
+        for i in range(-1, -len(self.traceback) - 1, -1):
+            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 @@ def getrepr(
         :param bool abspath:
             If paths should be changed to absolute or left unchanged.
 
-        :param bool tbfilter:
-            Hide entries that contain a local variable ``__tracebackhide__==True``.
-            Ignored if ``style=="native"``.
+        :param tbfilter:
+            A filter for traceback entries.
+
+            * 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 @@ def _makepath(self, path: Union[Path, str]) -> str:
 
     def repr_traceback(self, excinfo: ExceptionInfo[BaseException]) -> "ReprTraceback":
         traceback = excinfo.traceback
-        if self.tbfilter:
-            traceback = traceback.filter()
+        if callable(self.tbfilter):
+            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 @@
 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 @@ def getparent(self, cls: Type[_NodeType]) -> Optional[_NodeType]:
         assert current is None or isinstance(current, cls)
         return current
 
-    def _prunetraceback(self, excinfo: ExceptionInfo[BaseException]) -> None:
-        pass
+    def _traceback_filter(self, excinfo: ExceptionInfo[BaseException]) -> Traceback:
+        return excinfo.traceback
 
     def _repr_failure_py(
         self,
@@ -449,10 +450,13 @@ def _repr_failure_py(
                 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 @@ def _repr_failure_py(
             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 @@ def repr_failure(  # type: ignore[override]
 
         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 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 @@ def runtest(self) -> None:
     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 @@ def _prunetraceback(self, excinfo: ExceptionInfo[BaseException]) -> None:
                     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:
-                    for entry in excinfo.traceback[1:-1]:
-                        entry.set_repr_style("short")
+                if len(ntraceback) > 2:
+                    ntraceback = Traceback(
+                        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 @@ def runtest(self) -> None:
             finally:
                 delattr(self._testcase, self.name)
 
-    def _prunetraceback(
+    def _traceback_filter(
         self, excinfo: _pytest._code.ExceptionInfo[BaseException]
-    ) -> None:
-        super()._prunetraceback(excinfo)
-        traceback = excinfo.traceback.filter(
-            lambda x: not x.frame.f_globals.get("__unittest")
+    ) -> _pytest._code.Traceback:
+        traceback = super()._traceback_filter(excinfo)
+        ntraceback = traceback.filter(
+            lambda x: not x.frame.f_globals.get("__unittest"),
         )
-        if traceback:
-            excinfo.traceback = traceback
+        if not ntraceback:
+            ntraceback = traceback
+        return ntraceback
 
 
 @hookimpl(tryfirst=True)