Merge pull request #10226 from Zac-HD/use-exceptiongroup-for-teardown

Zac-HD · web-flow · commit 10f55f79afcb · 2022-10-24T08:44:02.000-07:00
Use exceptiongroup for multiple errors during teardown
diff --git a/changelog/10226.improvement.rst b/changelog/10226.improvement.rst
@@ -0,0 +1 @@
+If multiple errors are raised in teardown, we now re-raise an ``ExceptionGroup`` of them instead of discarding all but the last.
diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
@@ -35,6 +35,9 @@
 from _pytest.outcomes import Skipped
 from _pytest.outcomes import TEST_OUTCOME
 
+if sys.version_info[:2] < (3, 11):
+    from exceptiongroup import BaseExceptionGroup
+
 if TYPE_CHECKING:
     from typing_extensions import Literal
 
@@ -512,22 +515,29 @@ def teardown_exact(self, nextitem: Optional[Item]) -> None:
         stack is torn down.
         """
         needed_collectors = nextitem and nextitem.listchain() or []
-        exc = None
+        exceptions: List[BaseException] = []
         while self.stack:
             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:
-                    # XXX Only first exception will be seen by user,
-                    #     ideally all should be reported.
-                    if exc is None:
-                        exc = e
-        if exc:
-            raise exc
+                    these_exceptions.append(e)
+
+            if len(these_exceptions) == 1:
+                exceptions.extend(these_exceptions)
+            elif these_exceptions:
+                msg = f"errors while tearing down {node!r}"
+                exceptions.append(BaseExceptionGroup(msg, these_exceptions[::-1]))
+
+        if len(exceptions) == 1:
+            raise exceptions[0]
+        elif exceptions:
+            raise BaseExceptionGroup("errors during test teardown", exceptions[::-1])
         if nextitem is None:
             assert not self.stack
 
diff --git a/testing/test_runner.py b/testing/test_runner.py
@@ -2,6 +2,7 @@
 import os
 import sys
 import types
+from functools import partial
 from pathlib import Path
 from typing import Dict
 from typing import List
@@ -19,6 +20,9 @@
 from _pytest.outcomes import OutcomeException
 from _pytest.pytester import Pytester
 
+if sys.version_info[:2] < (3, 11):
+    from exceptiongroup import ExceptionGroup
+
 
 class TestSetupState:
     def test_setup(self, pytester: Pytester) -> None:
@@ -77,8 +81,6 @@ def fin3():
         assert r == ["fin3", "fin1"]
 
     def test_teardown_multiple_fail(self, pytester: Pytester) -> None:
-        # Ensure the first exception is the one which is re-raised.
-        # Ideally both would be reported however.
         def fin1():
             raise Exception("oops1")
 
@@ -90,9 +92,14 @@ def fin2():
         ss.setup(item)
         ss.addfinalizer(fin1, item)
         ss.addfinalizer(fin2, item)
-        with pytest.raises(Exception) as err:
+        with pytest.raises(ExceptionGroup) as err:
             ss.teardown_exact(None)
-        assert err.value.args == ("oops2",)
+
+        # Note that finalizers are run LIFO, but because FIFO is more intuitive for
+        # users we reverse the order of messages, and see the error from fin1 first.
+        err1, err2 = err.value.exceptions
+        assert err1.args == ("oops1",)
+        assert err2.args == ("oops2",)
 
     def test_teardown_multiple_scopes_one_fails(self, pytester: Pytester) -> None:
         module_teardown = []
@@ -113,6 +120,25 @@ def fin_module():
             ss.teardown_exact(None)
         assert module_teardown == ["fin_module"]
 
+    def test_teardown_multiple_scopes_several_fail(self, pytester) -> None:
+        def raiser(exc):
+            raise exc
+
+        item = pytester.getitem("def test_func(): pass")
+        mod = item.listchain()[-2]
+        ss = item.session._setupstate
+        ss.setup(item)
+        ss.addfinalizer(partial(raiser, KeyError("from module scope")), mod)
+        ss.addfinalizer(partial(raiser, TypeError("from function scope 1")), item)
+        ss.addfinalizer(partial(raiser, ValueError("from function scope 2")), item)
+
+        with pytest.raises(ExceptionGroup, match="errors during test teardown") as e:
+            ss.teardown_exact(None)
+        mod, func = e.value.exceptions
+        assert isinstance(mod, KeyError)
+        assert isinstance(func.exceptions[0], TypeError)  # type: ignore
+        assert isinstance(func.exceptions[1], ValueError)  # type: ignore
+
 
 class BaseFunctionalTests:
     def test_passfunction(self, pytester: Pytester) -> None:

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+If multiple errors are raised in teardown, we now re-raise an ``ExceptionGroup`` of them instead of discarding all but the last.