From 4f777ff5c83273deba23832dd7e3768ba122fe39 Mon Sep 17 00:00:00 2001
From: Thomas Grainger <tagrain@gmail.com>
Date: Sun, 24 Nov 2024 00:34:59 +0000
Subject: [PATCH] Prevent Config.add_cleanup callbacks preventing other
 cleanups running (#12982)

Ensure all callbacks registered via Config.add_cleanup will be called, regardless if any of them raises an error.

---------


Co-authored-by: Bruno Oliveira <nicoddemus@gmail.com>
---
 changelog/12981.bugfix.rst     |  1 +
 src/_pytest/config/__init__.py | 28 +++++++++++++++++-----------
 testing/test_config.py         | 31 +++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 11 deletions(-)
 create mode 100644 changelog/12981.bugfix.rst

diff --git a/changelog/12981.bugfix.rst b/changelog/12981.bugfix.rst
new file mode 100644
index 00000000000..5fc8e29656f
--- /dev/null
+++ b/changelog/12981.bugfix.rst
@@ -0,0 +1 @@
+Prevent exceptions in :func:`pytest.Config.add_cleanup` callbacks preventing further cleanups.
diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py
index 35ab622de31..92cf565e85f 100644
--- a/src/_pytest/config/__init__.py
+++ b/src/_pytest/config/__init__.py
@@ -5,6 +5,7 @@
 
 import argparse
 import collections.abc
+import contextlib
 import copy
 import dataclasses
 import enum
@@ -73,7 +74,6 @@
     from _pytest.cacheprovider import Cache
     from _pytest.terminal import TerminalReporter
 
-
 _PluggyPlugin = object
 """A type to represent plugin objects.
 
@@ -1077,7 +1077,7 @@ def __init__(
         self._inicache: dict[str, Any] = {}
         self._override_ini: Sequence[str] = ()
         self._opt2dest: dict[str, str] = {}
-        self._cleanup: list[Callable[[], None]] = []
+        self._cleanup_stack = contextlib.ExitStack()
         self.pluginmanager.register(self, "pytestconfig")
         self._configured = False
         self.hook.pytest_addoption.call_historic(
@@ -1106,8 +1106,9 @@ def inipath(self) -> pathlib.Path | None:
 
     def add_cleanup(self, func: Callable[[], None]) -> None:
         """Add a function to be called when the config object gets out of
-        use (usually coinciding with pytest_unconfigure)."""
-        self._cleanup.append(func)
+        use (usually coinciding with pytest_unconfigure).
+        """
+        self._cleanup_stack.callback(func)
 
     def _do_configure(self) -> None:
         assert not self._configured
@@ -1117,13 +1118,18 @@ def _do_configure(self) -> None:
             self.hook.pytest_configure.call_historic(kwargs=dict(config=self))
 
     def _ensure_unconfigure(self) -> None:
-        if self._configured:
-            self._configured = False
-            self.hook.pytest_unconfigure(config=self)
-            self.hook.pytest_configure._call_history = []
-        while self._cleanup:
-            fin = self._cleanup.pop()
-            fin()
+        try:
+            if self._configured:
+                self._configured = False
+                try:
+                    self.hook.pytest_unconfigure(config=self)
+                finally:
+                    self.hook.pytest_configure._call_history = []
+        finally:
+            try:
+                self._cleanup_stack.close()
+            finally:
+                self._cleanup_stack = contextlib.ExitStack()
 
     def get_terminal_writer(self) -> TerminalWriter:
         terminalreporter: TerminalReporter | None = self.pluginmanager.get_plugin(
diff --git a/testing/test_config.py b/testing/test_config.py
index 13ba66e8f9d..b2825678b46 100644
--- a/testing/test_config.py
+++ b/testing/test_config.py
@@ -983,6 +983,37 @@ def test_confcutdir_check_isdir(self, pytester: Pytester) -> None:
     def test_iter_rewritable_modules(self, names, expected) -> None:
         assert list(_iter_rewritable_modules(names)) == expected
 
+    def test_add_cleanup(self, pytester: Pytester) -> None:
+        config = Config.fromdictargs({}, [])
+        config._do_configure()
+        report = []
+
+        class MyError(BaseException):
+            pass
+
+        @config.add_cleanup
+        def cleanup_last():
+            report.append("cleanup_last")
+
+        @config.add_cleanup
+        def raise_2():
+            report.append("raise_2")
+            raise MyError("raise_2")
+
+        @config.add_cleanup
+        def raise_1():
+            report.append("raise_1")
+            raise MyError("raise_1")
+
+        @config.add_cleanup
+        def cleanup_first():
+            report.append("cleanup_first")
+
+        with pytest.raises(MyError, match=r"raise_2"):
+            config._ensure_unconfigure()
+
+        assert report == ["cleanup_first", "raise_1", "raise_2", "cleanup_last"]
+
 
 class TestConfigFromdictargs:
     def test_basic_behavior(self, _sys_snapshot) -> None: