Merge pull request #12048 from bluetech/fixture-teardown-excgroup

fixtures: use exception group when multiple finalizers raise in fixture teardown

Author: bluetech

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.

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()

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 <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: