From 3a68c08426326e6c8129e94f98d263131da9a032 Mon Sep 17 00:00:00 2001 From: Zac Hatfield-Dodds Date: Sun, 23 Oct 2022 15:06:29 -0700 Subject: [PATCH] Use exceptiongroup for teardown errors --- changelog/10226.improvement.rst | 1 + src/_pytest/runner.py | 24 ++++++++++++++++------- testing/test_runner.py | 34 +++++++++++++++++++++++++++++---- 3 files changed, 48 insertions(+), 11 deletions(-) create mode 100644 changelog/10226.improvement.rst diff --git a/changelog/10226.improvement.rst b/changelog/10226.improvement.rst new file mode 100644 index 000000000..f1506eaf7 --- /dev/null +++ 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 index 584c3229d..cc17cf2f4 100644 --- a/src/_pytest/runner.py +++ b/src/_pytest/runner.py @@ -35,6 +35,9 @@ from _pytest.outcomes import OutcomeException 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 @@ class SetupState: 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 index 2e2c462d9..49adc04fe 100644 --- a/testing/test_runner.py +++ b/testing/test_runner.py @@ -2,6 +2,7 @@ import inspect 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.monkeypatch import MonkeyPatch 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 @@ class TestSetupState: 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 @@ class TestSetupState: 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 @@ class TestSetupState: 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: