diff --git a/changelog/12047.improvement.rst b/changelog/12047.improvement.rst new file mode 100644 index 00000000000..e9ad5eddcab --- /dev/null +++ b/changelog/12047.improvement.rst @@ -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. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 0a505d65ad0..b619dc358e1 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -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 @@ -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 @@ -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() diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 299e411a695..6edff6ecd43 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -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( """ @@ -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 (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: