Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.3.x] Revert "Correctly handle tracebackhide for chained exceptions (#10772)" #10906

Merged
merged 1 commit into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
[7.3.x] Revert "Correctly handle tracebackhide for chained exceptions (
  • Loading branch information
bluetech authored and pytestbot committed Apr 13, 2023
commit a4121aa0b6d7f8d67c6934dbb81671a859b9bdf3
1 change: 0 additions & 1 deletion AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ Erik M. Bray
Evan Kepner
Fabien Zarifian
Fabio Zadrozny
Felix Hofstätter
Felix Nieuwenhuizen
Feng Ma
Florian Bruhin
Expand Down
2 changes: 2 additions & 0 deletions changelog/10903.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix crash ``INTERNALERROR IndexError: list index out of range`` which happens when displaying an exception where all entries are hidden.
This reverts the change "Correctly handle ``__tracebackhide__`` for chained exceptions." introduced in version 7.3.0.
1 change: 1 addition & 0 deletions doc/en/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ Bug Fixes


- `#1904 <https://github.com/pytest-dev/pytest/issues/1904>`_: Correctly handle ``__tracebackhide__`` for chained exceptions.
NOTE: This change was reverted in version 7.3.1.



Expand Down
27 changes: 10 additions & 17 deletions src/_pytest/_code/code.py
Original file line number Diff line number Diff line change
Expand Up @@ -411,13 +411,13 @@ def filter(
"""
return Traceback(filter(fn, self), self._excinfo)

def getcrashentry(self) -> Optional[TracebackEntry]:
def getcrashentry(self) -> TracebackEntry:
"""Return last non-hidden traceback entry that lead to the exception of a traceback."""
for i in range(-1, -len(self) - 1, -1):
entry = self[i]
if not entry.ishidden():
return entry
return None
return self[-1]

def recursionindex(self) -> Optional[int]:
"""Return the index of the frame/TracebackEntry where recursion originates if
Expand Down Expand Up @@ -602,13 +602,11 @@ def errisinstance(
"""
return isinstance(self.value, exc)

def _getreprcrash(self) -> Optional["ReprFileLocation"]:
def _getreprcrash(self) -> "ReprFileLocation":
exconly = self.exconly(tryshort=True)
entry = self.traceback.getcrashentry()
if entry:
path, lineno = entry.frame.code.raw.co_filename, entry.lineno
return ReprFileLocation(path, lineno + 1, exconly)
return None
path, lineno = entry.frame.code.raw.co_filename, entry.lineno
return ReprFileLocation(path, lineno + 1, exconly)

def getrepr(
self,
Expand Down Expand Up @@ -946,23 +944,18 @@ def repr_excinfo(
)
else:
reprtraceback = self.repr_traceback(excinfo_)

# will be None if all traceback entries are hidden
reprcrash: Optional[ReprFileLocation] = excinfo_._getreprcrash()
if reprcrash:
if self.style == "value":
repr_chain += [(reprtraceback, None, descr)]
else:
repr_chain += [(reprtraceback, reprcrash, descr)]
reprcrash: Optional[ReprFileLocation] = (
excinfo_._getreprcrash() if self.style != "value" else None
)
else:
# Fallback to native repr if the exception doesn't have a traceback:
# ExceptionInfo objects require a full traceback to work.
reprtraceback = ReprTracebackNative(
traceback.format_exception(type(e), e, None)
)
reprcrash = None
repr_chain += [(reprtraceback, reprcrash, descr)]

repr_chain += [(reprtraceback, reprcrash, descr)]
if e.__cause__ is not None and self.chain:
e = e.__cause__
excinfo_ = (
Expand Down Expand Up @@ -1053,7 +1046,7 @@ def toterminal(self, tw: TerminalWriter) -> None:
@dataclasses.dataclass(eq=False)
class ReprExceptionInfo(ExceptionRepr):
reprtraceback: "ReprTraceback"
reprcrash: Optional["ReprFileLocation"]
reprcrash: "ReprFileLocation"

def toterminal(self, tw: TerminalWriter) -> None:
self.reprtraceback.toterminal(tw)
Expand Down
4 changes: 0 additions & 4 deletions src/_pytest/reports.py
Original file line number Diff line number Diff line change
Expand Up @@ -347,10 +347,6 @@ def from_item_and_call(cls, item: Item, call: "CallInfo[None]") -> "TestReport":
elif isinstance(excinfo.value, skip.Exception):
outcome = "skipped"
r = excinfo._getreprcrash()
if r is None:
raise ValueError(
"There should always be a traceback entry for skipping a test."
)
if excinfo.value._use_item_location:
path, line = item.reportinfo()[:2]
assert line is not None
Expand Down
23 changes: 21 additions & 2 deletions testing/code/test_excinfo.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,6 @@ def f():
excinfo = pytest.raises(ValueError, f)
tb = excinfo.traceback
entry = tb.getcrashentry()
assert entry is not None
co = _pytest._code.Code.from_function(h)
assert entry.frame.code.path == co.path
assert entry.lineno == co.firstlineno + 1
Expand All @@ -312,7 +311,10 @@ def f():
excinfo = pytest.raises(ValueError, f)
tb = excinfo.traceback
entry = tb.getcrashentry()
assert entry is None
co = _pytest._code.Code.from_function(g)
assert entry.frame.code.path == co.path
assert entry.lineno == co.firstlineno + 2
assert entry.frame.code.name == "g"


def test_excinfo_exconly():
Expand Down Expand Up @@ -1573,3 +1575,20 @@ def test_exceptiongroup(pytester: Pytester, outer_chain, inner_chain) -> None:
# with py>=3.11 does not depend on exceptiongroup, though there is a toxenv for it
pytest.importorskip("exceptiongroup")
_exceptiongroup_common(pytester, outer_chain, inner_chain, native=False)


def test_all_entries_hidden_doesnt_crash(pytester: Pytester) -> None:
"""Regression test for #10903.

We're not really sure what should be *displayed* here, so this test
just verified that at least it doesn't crash.
"""
pytester.makepyfile(
"""
def test():
__tracebackhide__ = True
1 / 0
"""
)
result = pytester.runpytest()
assert result.ret == 1
25 changes: 0 additions & 25 deletions testing/test_tracebackhide.py

This file was deleted.