diff --git a/changelog/1904.bugfix.rst b/changelog/1904.bugfix.rst new file mode 100644 index 000000000..3e1a29215 --- /dev/null +++ b/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). diff --git a/src/_pytest/_code/code.py b/src/_pytest/_code/code.py index 5bc78f478..9b051332b 100644 --- a/src/_pytest/_code/code.py +++ b/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._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 @@ 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 - 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 @@ 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) - 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 @@ class ExceptionInfo(Generic[E]): :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 @@ class FormattedExcinfo: 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) diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index ea016786e..dbd6b0a42 100644 --- a/src/_pytest/nodes.py +++ b/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: - pass + def _traceback_filter(self, excinfo: ExceptionInfo[BaseException]) -> Traceback: + 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]: diff --git a/src/_pytest/python.py b/src/_pytest/python.py index b2f757da4..94f000939 100644 --- a/src/_pytest/python.py +++ b/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: - 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] diff --git a/src/_pytest/unittest.py b/src/_pytest/unittest.py index c660aa75d..d42a12a3a 100644 --- a/src/_pytest/unittest.py +++ b/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: - 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) diff --git a/testing/code/test_excinfo.py b/testing/code/test_excinfo.py index 6a720e64c..88aa5f0f1 100644 --- a/testing/code/test_excinfo.py +++ b/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 - entry = tb.getcrashentry() - assert entry is not None + reprcrash = excinfo._getreprcrash() + assert reprcrash is not None co = _pytest._code.Code.from_function(h) - assert entry.frame.code.path == co.path - assert entry.lineno == co.firstlineno + 1 - assert entry.frame.code.name == "h" + assert reprcrash.path == str(co.path) + assert reprcrash.lineno == co.firstlineno + 1 + 1 - 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[1].set_repr_style("short") - excinfo.traceback[2].set_repr_style("short") + excinfo.traceback = excinfo.traceback.filter(excinfo) + excinfo.traceback = _pytest._code.Traceback( + 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, + ) diff --git a/testing/python/collect.py b/testing/python/collect.py index ac3edd395..41845517b 100644 --- a/testing/python/collect.py +++ b/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(