From cc23ec91d042ee15145b890aea04e96f6e831101 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 12 Apr 2023 23:17:54 +0300 Subject: [PATCH 1/5] code: stop storing weakref to ExceptionInfo on Traceback and TracebackEntry TracebackEntry needs the excinfo for the `__tracebackhide__ = callback` functionality, where `callback` accepts the excinfo. Currently it achieves this by storing a weakref to the excinfo which created it. I think this is not great, mixing layers and bloating the objects. Instead, have `ishidden` (and transitively, `Traceback.filter()`) take the excinfo as a parameter. --- src/_pytest/_code/code.py | 53 ++++++++++++++++++++---------------- src/_pytest/nodes.py | 2 +- src/_pytest/python.py | 2 +- src/_pytest/unittest.py | 2 +- testing/code/test_excinfo.py | 24 ++++++++-------- testing/python/collect.py | 4 +-- 6 files changed, 46 insertions(+), 41 deletions(-) diff --git a/src/_pytest/_code/code.py b/src/_pytest/_code/code.py index 5bc78f478..26db750e6 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 @@ -52,7 +51,6 @@ from _pytest.pathlib import bestrelpath if TYPE_CHECKING: from typing_extensions import Literal from typing_extensions import SupportsIndex - from weakref import ReferenceType _TracebackStyle = Literal["long", "short", "line", "no", "native", "value", "auto"] @@ -194,15 +192,13 @@ 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, ) -> None: self._rawentry = rawentry - self._excinfo = excinfo self._repr_style: Optional['Literal["short", "long"]'] = None @property @@ -272,7 +268,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 +292,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 +325,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 +372,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,25 +392,36 @@ 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) + 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 getcrashentry(self) -> Optional[TracebackEntry]: + def getcrashentry( + self, excinfo: Optional["ExceptionInfo[BaseException]"] + ) -> 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(): + if not entry.ishidden(excinfo): return entry return None @@ -583,7 +588,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 @@ -624,7 +629,7 @@ class ExceptionInfo(Generic[E]): def _getreprcrash(self) -> Optional["ReprFileLocation"]: exconly = self.exconly(tryshort=True) - entry = self.traceback.getcrashentry() + entry = self.traceback.getcrashentry(self) if entry is None: return None path, lineno = entry.frame.code.raw.co_filename, entry.lineno @@ -882,7 +887,7 @@ class FormattedExcinfo: def repr_traceback(self, excinfo: ExceptionInfo[BaseException]) -> "ReprTraceback": traceback = excinfo.traceback if self.tbfilter: - traceback = traceback.filter() + 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..738ab97e9 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -560,7 +560,7 @@ class Collector(Node): ntraceback = traceback.cut(path=self.path) if ntraceback == traceback: ntraceback = ntraceback.cut(excludepath=tracebackcutdir) - excinfo.traceback = ntraceback.filter() + excinfo.traceback = ntraceback.filter(excinfo) def _check_initialpaths_for_relpath(session: "Session", path: Path) -> Optional[str]: diff --git a/src/_pytest/python.py b/src/_pytest/python.py index d04b6fa4d..c65c41b97 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1814,7 +1814,7 @@ class Function(PyobjMixin, nodes.Item): if not ntraceback: ntraceback = traceback - excinfo.traceback = ntraceback.filter() + excinfo.traceback = ntraceback.filter(excinfo) # 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": diff --git a/src/_pytest/unittest.py b/src/_pytest/unittest.py index c660aa75d..7a5e73661 100644 --- a/src/_pytest/unittest.py +++ b/src/_pytest/unittest.py @@ -339,7 +339,7 @@ class TestCaseFunction(Function): ) -> None: super()._prunetraceback(excinfo) traceback = excinfo.traceback.filter( - lambda x: not x.frame.f_globals.get("__unittest") + lambda x: not x.frame.f_globals.get("__unittest"), ) if traceback: excinfo.traceback = traceback diff --git a/testing/code/test_excinfo.py b/testing/code/test_excinfo.py index 6a720e64c..d0eb5e646 100644 --- a/testing/code/test_excinfo.py +++ b/testing/code/test_excinfo.py @@ -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}") @@ -307,7 +307,7 @@ class TestTraceback_f_g_h: excinfo = pytest.raises(ValueError, f) tb = excinfo.traceback - entry = tb.getcrashentry() + entry = tb.getcrashentry(excinfo) assert entry is not None co = _pytest._code.Code.from_function(h) assert entry.frame.code.path == co.path @@ -324,7 +324,7 @@ class TestTraceback_f_g_h: g() excinfo = pytest.raises(ValueError, f) - assert excinfo.traceback.getcrashentry() is None + assert excinfo.traceback.getcrashentry(excinfo) is None def test_excinfo_exconly(): @@ -626,7 +626,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 +659,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 +686,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 +960,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 +994,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 +1026,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,7 +1123,7 @@ 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[2].set_repr_style("short") r = excinfo.getrepr(style="long") @@ -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"]) 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( From 0a20452f78a2f5401cf0fc05dad04c8aeee170d7 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 12 Apr 2023 23:43:00 +0300 Subject: [PATCH 2/5] code: inline `Traceback.getcrashentry` into `ExceptionInfo._getreprcrash` Since `Traceback.getcrashentry` takes the `ExceptionInfo`, it is not really independent of it and is in the wrong layer. Prevent nonsensical mistakes by inlining it. --- src/_pytest/_code/code.py | 26 +++++++++----------------- testing/code/test_excinfo.py | 18 ++++++++---------- 2 files changed, 17 insertions(+), 27 deletions(-) diff --git a/src/_pytest/_code/code.py b/src/_pytest/_code/code.py index 26db750e6..581399949 100644 --- a/src/_pytest/_code/code.py +++ b/src/_pytest/_code/code.py @@ -414,17 +414,6 @@ class Traceback(List[TracebackEntry]): fn = _excinfo_or_fn return Traceback(filter(fn, self)) - def getcrashentry( - self, excinfo: Optional["ExceptionInfo[BaseException]"] - ) -> 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(excinfo): - return entry - return None - def recursionindex(self) -> Optional[int]: """Return the index of the frame/TracebackEntry where recursion originates if appropriate, None if no recursion occurred.""" @@ -628,12 +617,15 @@ class ExceptionInfo(Generic[E]): return isinstance(self.value, exc) def _getreprcrash(self) -> Optional["ReprFileLocation"]: - exconly = self.exconly(tryshort=True) - entry = self.traceback.getcrashentry(self) - 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, diff --git a/testing/code/test_excinfo.py b/testing/code/test_excinfo.py index d0eb5e646..3b05390be 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 @@ -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(excinfo) - 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(excinfo) is None + assert excinfo._getreprcrash() is None def test_excinfo_exconly(): From 6f7f89f3c42861448a370fe7bfc343083850f600 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 12 Apr 2023 22:58:57 +0300 Subject: [PATCH 3/5] code: make TracebackEntry immutable TracebackEntry being mutable caught me by surprise and makes reasoning about the exception formatting code harder. Make it a proper value. --- src/_pytest/_code/code.py | 15 +++++++++------ src/_pytest/python.py | 9 +++++++-- testing/code/test_excinfo.py | 6 ++++-- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/_pytest/_code/code.py b/src/_pytest/_code/code.py index 581399949..5148dcdbe 100644 --- a/src/_pytest/_code/code.py +++ b/src/_pytest/_code/code.py @@ -49,6 +49,7 @@ 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 @@ -197,18 +198,20 @@ class TracebackEntry: def __init__( self, rawentry: TracebackType, + repr_style: Optional['Literal["short", "long"]'] = None, ) -> None: - self._rawentry = rawentry - 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) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index c65c41b97..8ed6b46df 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 @@ -1819,8 +1820,12 @@ class Function(PyobjMixin, nodes.Item): # 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") + excinfo.traceback = Traceback( + entry + if i == 0 or i == len(excinfo.traceback) - 1 + else entry.with_repr_style("short") + for i, entry in enumerate(excinfo.traceback) + ) # TODO: Type ignored -- breaks Liskov Substitution. def repr_failure( # type: ignore[override] diff --git a/testing/code/test_excinfo.py b/testing/code/test_excinfo.py index 3b05390be..bda6cd4cd 100644 --- a/testing/code/test_excinfo.py +++ b/testing/code/test_excinfo.py @@ -1122,8 +1122,10 @@ raise ValueError() ) excinfo = pytest.raises(ValueError, mod.f) excinfo.traceback = excinfo.traceback.filter(excinfo) - excinfo.traceback[1].set_repr_style("short") - excinfo.traceback[2].set_repr_style("short") + 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: From fcada1ea4763c0e3471cd58ac4b89e7c874e6264 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Thu, 13 Apr 2023 13:48:44 +0300 Subject: [PATCH 4/5] nodes: change _prunetraceback to return the new traceback instead of modifying excinfo This makes it usable as a general function, and just more understandable in general. --- src/_pytest/nodes.py | 12 +++++++----- src/_pytest/python.py | 15 +++++++++------ src/_pytest/unittest.py | 13 +++++++------ 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 738ab97e9..1a1a47a28 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, @@ -452,7 +453,7 @@ class Node(metaclass=NodeMeta): if self.config.getoption("fulltrace", False): style = "long" else: - self._prunetraceback(excinfo) + excinfo.traceback = self._traceback_filter(excinfo) if style == "auto": style = "long" # XXX should excinfo.getrepr record all data and toterminal() process it? @@ -554,13 +555,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(excinfo) + 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 8ed6b46df..ae09da48e 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1802,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 @@ -1814,19 +1814,22 @@ class Function(PyobjMixin, nodes.Item): ntraceback = ntraceback.filter(filter_traceback) if not ntraceback: ntraceback = traceback + ntraceback = ntraceback.filter(excinfo) - excinfo.traceback = ntraceback.filter(excinfo) # 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: - excinfo.traceback = Traceback( + if len(ntraceback) > 2: + ntraceback = Traceback( entry - if i == 0 or i == len(excinfo.traceback) - 1 + if i == 0 or i == len(ntraceback) - 1 else entry.with_repr_style("short") - for i, entry in enumerate(excinfo.traceback) + for i, entry in enumerate(ntraceback) ) + return ntraceback + return excinfo.traceback + # TODO: Type ignored -- breaks Liskov Substitution. def repr_failure( # type: ignore[override] self, diff --git a/src/_pytest/unittest.py b/src/_pytest/unittest.py index 7a5e73661..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( + ) -> _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) From dd667336ce0b55b430452e78cd57ba2c0a769066 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sun, 16 Apr 2023 18:51:49 +0300 Subject: [PATCH 5/5] nodes: apply same traceback filtering for chained exceptions as main exception Fix #1904. --- changelog/1904.bugfix.rst | 1 + src/_pytest/_code/code.py | 22 +++++++++++++----- src/_pytest/nodes.py | 7 ++++-- testing/code/test_excinfo.py | 45 ++++++++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 changelog/1904.bugfix.rst 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 5148dcdbe..9b051332b 100644 --- a/src/_pytest/_code/code.py +++ b/src/_pytest/_code/code.py @@ -635,7 +635,9 @@ class ExceptionInfo(Generic[E]): 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,7 +889,9 @@ class FormattedExcinfo: def repr_traceback(self, excinfo: ExceptionInfo[BaseException]) -> "ReprTraceback": traceback = excinfo.traceback - if self.tbfilter: + if callable(self.tbfilter): + traceback = self.tbfilter(excinfo) + elif self.tbfilter: traceback = traceback.filter(excinfo) if isinstance(excinfo.value, RecursionError): diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 1a1a47a28..dbd6b0a42 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -450,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: - excinfo.traceback = self._traceback_filter(excinfo) + tbfilter = self._traceback_filter if style == "auto": style = "long" # XXX should excinfo.getrepr record all data and toterminal() process it? @@ -484,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, ) diff --git a/testing/code/test_excinfo.py b/testing/code/test_excinfo.py index bda6cd4cd..88aa5f0f1 100644 --- a/testing/code/test_excinfo.py +++ b/testing/code/test_excinfo.py @@ -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, + )