Skip to content
Open
Show file tree
Hide file tree
Changes from 7 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
2 changes: 2 additions & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,10 @@ Kevin Hierro Carrasco
Kevin J. Foley
Kian Eliasi
Kian-Meng Ang
Kirill Zhdanov
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
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.

3 changes: 3 additions & 0 deletions src/_pytest/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,9 @@ 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
self.teardown_exceptions: list[BaseException] = []
Copy link
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
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
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

@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?

Choose a reason for hiding this comment

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

Copy link
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 @@ -539,19 +539,20 @@
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)

Check warning on line 547 in src/_pytest/runner.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/runner.py#L547

Added line #L547 was not covered by tests

if len(these_exceptions) == 1:
exceptions.extend(these_exceptions)
elif these_exceptions:
if len(node.teardown_exceptions) == 1:
exceptions.extend(node.teardown_exceptions)

Check warning on line 550 in src/_pytest/runner.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/runner.py#L550

Added line #L550 was not covered by tests
elif node.teardown_exceptions:
msg = f"errors while tearing down {node!r}"
exceptions.append(BaseExceptionGroup(msg, these_exceptions[::-1]))
exceptions.append(

Check warning on line 553 in src/_pytest/runner.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/runner.py#L553

Added line #L553 was not covered by tests
BaseExceptionGroup(msg, node.teardown_exceptions[::-1])
)
Comment on lines +549 to +555
Copy link
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
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
25 changes: 25 additions & 0 deletions testing/acceptance_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1584,3 +1584,28 @@ def test_no_terminal_plugin(pytester: Pytester) -> None:
pytester.makepyfile("def test(): assert 1 == 2")
result = pytester.runpytest("-pno:terminal", "-s")
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()
result.assert_outcomes(passed=1, errors=1)
Loading