Skip to content

Commit

Permalink
Merge pull request #12048 from bluetech/fixture-teardown-excgroup
Browse files Browse the repository at this point in the history
fixtures: use exception group when multiple finalizers raise in fixture teardown
  • Loading branch information
bluetech authored Mar 3, 2024
2 parents 43492f5 + 434282e commit f4e1025
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 24 deletions.
2 changes: 2 additions & 0 deletions changelog/12047.improvement.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
When multiple finalizers of a fixture raise an exception, now all exceptions are reported as an exception group.
Previously, only the first exception was reported.
45 changes: 24 additions & 21 deletions src/_pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import inspect
import os
from pathlib import Path
import sys
from typing import AbstractSet
from typing import Any
from typing import Callable
Expand Down Expand Up @@ -67,6 +68,10 @@
from _pytest.scope import Scope


if sys.version_info[:2] < (3, 11):
from exceptiongroup import BaseExceptionGroup


if TYPE_CHECKING:
from typing import Deque

Expand Down Expand Up @@ -1017,27 +1022,25 @@ def addfinalizer(self, finalizer: Callable[[], object]) -> None:
self._finalizers.append(finalizer)

def finish(self, request: SubRequest) -> None:
exc = None
try:
while self._finalizers:
try:
func = self._finalizers.pop()
func()
except BaseException as e:
# XXX Only first exception will be seen by user,
# ideally all should be reported.
if exc is None:
exc = e
if exc:
raise exc
finally:
ihook = request.node.ihook
ihook.pytest_fixture_post_finalizer(fixturedef=self, request=request)
# Even if finalization fails, we invalidate the cached fixture
# value and remove all finalizers because they may be bound methods
# which will keep instances alive.
self.cached_result = None
self._finalizers.clear()
exceptions: List[BaseException] = []
while self._finalizers:
fin = self._finalizers.pop()
try:
fin()
except BaseException as e:
exceptions.append(e)
node = request.node
node.ihook.pytest_fixture_post_finalizer(fixturedef=self, request=request)
# Even if finalization fails, we invalidate the cached fixture
# value and remove all finalizers because they may be bound methods
# which will keep instances alive.
self.cached_result = None
self._finalizers.clear()
if len(exceptions) == 1:
raise exceptions[0]
elif len(exceptions) > 1:
msg = f'errors while tearing down fixture "{self.argname}" of {node}'
raise BaseExceptionGroup(msg, exceptions[::-1])

def execute(self, request: SubRequest) -> FixtureValue:
# Get required arguments and register our own finish()
Expand Down
15 changes: 12 additions & 3 deletions testing/python/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -932,8 +932,9 @@ def test_request_subrequest_addfinalizer_exceptions(
self, pytester: Pytester
) -> None:
"""
Ensure exceptions raised during teardown by a finalizer are suppressed
until all finalizers are called, re-raising the first exception (#2440)
Ensure exceptions raised during teardown by finalizers are suppressed
until all finalizers are called, then re-reaised together in an
exception group (#2440)
"""
pytester.makepyfile(
"""
Expand All @@ -960,8 +961,16 @@ def test_second():
"""
)
result = pytester.runpytest()
result.assert_outcomes(passed=2, errors=1)
result.stdout.fnmatch_lines(
["*Exception: Error in excepts fixture", "* 2 passed, 1 error in *"]
[
' | *ExceptionGroup: errors while tearing down fixture "subrequest" of <Function test_first> (2 sub-exceptions)', # noqa: E501
" +-+---------------- 1 ----------------",
" | Exception: Error in something fixture",
" +---------------- 2 ----------------",
" | Exception: Error in excepts fixture",
" +------------------------------------",
],
)

def test_request_getmodulepath(self, pytester: Pytester) -> None:
Expand Down

0 comments on commit f4e1025

Please sign in to comment.