Skip to content
2 changes: 2 additions & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,11 @@ Kevin Hierro Carrasco
Kevin J. Foley
Kian Eliasi
Kian-Meng Ang
Kirill Zhdanov
Kim Soo
Kodi B. Arfer
Kojo Idrissa
Konstantin Shkel
Kostis Anagnostopoulos
Kristoffer Nordström
Kyle Altendorf
Expand Down
1 change: 1 addition & 0 deletions changelog/12163.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Teardown fixtures now can access the information about current teardown exceptions in `node.teardown_exceptions`.
Copy link
Copy Markdown
Member

@nicoddemus nicoddemus Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit confusing as there are no "teardown fixtures", perhaps:

Suggested change
Teardown fixtures now can access the information about current teardown exceptions in `node.teardown_exceptions`.
Fixtures now can access the information about current teardown exceptions in `node.teardown_exceptions` during their own teardowns.

4 changes: 4 additions & 0 deletions src/_pytest/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ def __init__(
# Deprecated alias. Was never public. Can be removed in a few releases.
self._store = self.stash

#: A list of exceptions that happened during teardown. Intended for
#: post-teardown inspection, not required internally.
self.teardown_exceptions: list[BaseException] = []
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be a list and not an exception group?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to use a list then pass the list to the exception group, you can't append things to a group

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I didn't realize they were immutable..

Copy link
Copy Markdown

@storenth storenth Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guys, where to put tests? Can u suggest appropriate place? Looks like: testing/test_collection.py and teardown?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged, thank you. Now we have a test to check if the fixture fails and exception is added to the list


@classmethod
def from_parent(cls, parent: Node, **kw) -> Self:
"""Public constructor for Nodes.
Expand Down
13 changes: 7 additions & 6 deletions src/_pytest/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -561,19 +561,20 @@ def teardown_exact(self, nextitem: Item | None) -> None:
if list(self.stack.keys()) == needed_collectors[: len(self.stack)]:
break
node, (finalizers, _) = self.stack.popitem()
these_exceptions = []
while finalizers:
fin = finalizers.pop()
try:
fin()
except TEST_OUTCOME as e:
these_exceptions.append(e)
node.teardown_exceptions.append(e)

if len(these_exceptions) == 1:
exceptions.extend(these_exceptions)
elif these_exceptions:
if len(node.teardown_exceptions) == 1:
exceptions.extend(node.teardown_exceptions)
elif node.teardown_exceptions:
msg = f"errors while tearing down {node!r}"
exceptions.append(BaseExceptionGroup(msg, these_exceptions[::-1]))
exceptions.append(
BaseExceptionGroup(msg, node.teardown_exceptions[::-1])
)
Comment on lines +571 to +577
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is perhaps something for another PR (or too late/not worth changing), but from trio's experience with strict_exception_groups it's generally a bad design philosophy to sometimes return an exception group. This leads users to write code where they assume there's only ever one or no exceptions, and everything breaks when >1 exceptions happen to occur.
Maybe it's less of an issue here, given that we're not raising the exceptions, but I could see this complicating parsing logic nevertheless.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're raising them a few lines later, after all teardowns. However, this is a design change, and probably should be done in another PR


if len(exceptions) == 1:
raise exceptions[0]
Expand Down
27 changes: 27 additions & 0 deletions testing/acceptance_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1589,6 +1589,33 @@ def test_no_terminal_plugin(pytester: Pytester) -> None:
assert result.ret == ExitCode.TESTS_FAILED


def test_get_exception_on_teardown_failure(pytester: Pytester) -> None:
"""Smoke test to be sure teardown exceptions handled properly via node property"""
pytester.makepyfile(
conftest="""
import sys
import pytest
def pytest_exception_interact(node, call, report):
sys.stderr.write("teardown_exceptions: `{}`".format(node.teardown_exceptions))

@pytest.fixture
def mylist():
yield
raise AssertionError(111)
""",
test_file="""
def test_func(mylist):
assert True
""",
)
result = pytester.runpytest()
assert result.ret == ExitCode.TESTS_FAILED
assert "teardown_exceptions: `[AssertionError(111)]`" in result.stderr.str()
# Related to the #9909 - first the test passes, then the teardown fails, what
# results in a double-reporting.
result.assert_outcomes(passed=1, errors=1)


def test_stop_iteration_from_collect(pytester: Pytester) -> None:
pytester.makepyfile(test_it="raise StopIteration('hello')")
result = pytester.runpytest()
Expand Down
Loading