Merge pull request #12264 from bluetech/reraise-with-original-tb

fixtures,runner: fix tracebacks getting longer and longer
This commit is contained in:
Ran Benita 2024-04-29 20:02:44 +03:00 committed by GitHub
commit feaae2fb35
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 82 additions and 9 deletions

View File

@ -0,0 +1,7 @@
Fix a regression in pytest 8.0 where tracebacks get longer and longer when multiple tests fail due to a shared higher-scope fixture which raised.
Also fix a similar regression in pytest 5.4 for collectors which raise during setup.
The fix necessitated internal changes which may affect some plugins:
- ``FixtureDef.cached_result[2]`` is now a tuple ``(exc, tb)`` instead of ``exc``.
- ``SetupState.stack`` failures are now a tuple ``(exc, tb)`` instead of ``exc``.

View File

@ -8,6 +8,7 @@ import inspect
import os import os
from pathlib import Path from pathlib import Path
import sys import sys
import types
from typing import AbstractSet from typing import AbstractSet
from typing import Any from typing import Any
from typing import Callable from typing import Callable
@ -104,8 +105,8 @@ _FixtureCachedResult = Union[
None, None,
# Cache key. # Cache key.
object, object,
# Exception if raised. # The exception and the original traceback.
BaseException, Tuple[BaseException, Optional[types.TracebackType]],
], ],
] ]
@ -1049,8 +1050,8 @@ class FixtureDef(Generic[FixtureValue]):
# numpy arrays (#6497). # numpy arrays (#6497).
if my_cache_key is cache_key: if my_cache_key is cache_key:
if self.cached_result[2] is not None: if self.cached_result[2] is not None:
exc = self.cached_result[2] exc, exc_tb = self.cached_result[2]
raise exc raise exc.with_traceback(exc_tb)
else: else:
result = self.cached_result[0] result = self.cached_result[0]
return result return result
@ -1126,7 +1127,7 @@ def pytest_fixture_setup(
# Don't show the fixture as the skip location, as then the user # Don't show the fixture as the skip location, as then the user
# wouldn't know which test skipped. # wouldn't know which test skipped.
e._use_item_location = True e._use_item_location = True
fixturedef.cached_result = (None, my_cache_key, e) fixturedef.cached_result = (None, my_cache_key, (e, e.__traceback__))
raise raise
fixturedef.cached_result = (result, my_cache_key, None) fixturedef.cached_result = (result, my_cache_key, None)
return result return result

View File

@ -5,6 +5,7 @@ import bdb
import dataclasses import dataclasses
import os import os
import sys import sys
import types
from typing import Callable from typing import Callable
from typing import cast from typing import cast
from typing import Dict from typing import Dict
@ -488,8 +489,13 @@ class SetupState:
Tuple[ Tuple[
# Node's finalizers. # Node's finalizers.
List[Callable[[], object]], List[Callable[[], object]],
# Node's exception, if its setup raised. # Node's exception and original traceback, if its setup raised.
Optional[Union[OutcomeException, Exception]], Optional[
Tuple[
Union[OutcomeException, Exception],
Optional[types.TracebackType],
]
],
], ],
] = {} ] = {}
@ -502,7 +508,7 @@ class SetupState:
for col, (finalizers, exc) in self.stack.items(): for col, (finalizers, exc) in self.stack.items():
assert col in needed_collectors, "previous item was not torn down properly" assert col in needed_collectors, "previous item was not torn down properly"
if exc: if exc:
raise exc raise exc[0].with_traceback(exc[1])
for col in needed_collectors[len(self.stack) :]: for col in needed_collectors[len(self.stack) :]:
assert col not in self.stack assert col not in self.stack
@ -511,7 +517,7 @@ class SetupState:
try: try:
col.setup() col.setup()
except TEST_OUTCOME as exc: except TEST_OUTCOME as exc:
self.stack[col] = (self.stack[col][0], exc) self.stack[col] = (self.stack[col][0], (exc, exc.__traceback__))
raise raise
def addfinalizer(self, finalizer: Callable[[], object], node: Node) -> None: def addfinalizer(self, finalizer: Callable[[], object], node: Node) -> None:

View File

@ -3397,6 +3397,28 @@ class TestErrors:
["*def gen(qwe123):*", "*fixture*qwe123*not found*", "*1 error*"] ["*def gen(qwe123):*", "*fixture*qwe123*not found*", "*1 error*"]
) )
def test_cached_exception_doesnt_get_longer(self, pytester: Pytester) -> None:
"""Regression test for #12204."""
pytester.makepyfile(
"""
import pytest
@pytest.fixture(scope="session")
def bad(): 1 / 0
def test_1(bad): pass
def test_2(bad): pass
def test_3(bad): pass
"""
)
result = pytester.runpytest_inprocess("--tb=native")
assert result.ret == ExitCode.TESTS_FAILED
failures = result.reprec.getfailures() # type: ignore[attr-defined]
assert len(failures) == 3
lines1 = failures[1].longrepr.reprtraceback.reprentries[0].lines
lines2 = failures[2].longrepr.reprtraceback.reprentries[0].lines
assert len(lines1) == len(lines2)
class TestShowFixtures: class TestShowFixtures:
def test_funcarg_compat(self, pytester: Pytester) -> None: def test_funcarg_compat(self, pytester: Pytester) -> None:

View File

@ -142,6 +142,43 @@ class TestSetupState:
assert isinstance(func.exceptions[0], TypeError) # type: ignore assert isinstance(func.exceptions[0], TypeError) # type: ignore
assert isinstance(func.exceptions[1], ValueError) # type: ignore assert isinstance(func.exceptions[1], ValueError) # type: ignore
def test_cached_exception_doesnt_get_longer(self, pytester: Pytester) -> None:
"""Regression test for #12204 (the "BTW" case)."""
pytester.makepyfile(test="")
# If the collector.setup() raises, all collected items error with this
# exception.
pytester.makeconftest(
"""
import pytest
class MyItem(pytest.Item):
def runtest(self) -> None: pass
class MyBadCollector(pytest.Collector):
def collect(self):
return [
MyItem.from_parent(self, name="one"),
MyItem.from_parent(self, name="two"),
MyItem.from_parent(self, name="three"),
]
def setup(self):
1 / 0
def pytest_collect_file(file_path, parent):
if file_path.name == "test.py":
return MyBadCollector.from_parent(parent, name='bad')
"""
)
result = pytester.runpytest_inprocess("--tb=native")
assert result.ret == ExitCode.TESTS_FAILED
failures = result.reprec.getfailures() # type: ignore[attr-defined]
assert len(failures) == 3
lines1 = failures[1].longrepr.reprtraceback.reprentries[0].lines
lines2 = failures[2].longrepr.reprtraceback.reprentries[0].lines
assert len(lines1) == len(lines2)
class BaseFunctionalTests: class BaseFunctionalTests:
def test_passfunction(self, pytester: Pytester) -> None: def test_passfunction(self, pytester: Pytester) -> None: