From 33234efc0044becf8735e6868573b1d11ccb4d9d Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Thu, 21 Nov 2024 09:48:56 +0000 Subject: [PATCH] prevent Config.add_cleanup callbacks preventing other cleanups running --- src/_pytest/config/__init__.py | 30 ++++++++++++++++++++---------- testing/test_config.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 35ab622de31..4141926c7f2 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 @@ -33,6 +34,7 @@ from typing import TextIO from typing import Type from typing import TYPE_CHECKING +from typing import TypeVar import warnings import pluggy @@ -73,6 +75,8 @@ from _pytest.cacheprovider import Cache from _pytest.terminal import TerminalReporter +_T_callback = TypeVar("_T_callback", bound=Callable[[], None]) + _PluggyPlugin = object """A type to represent plugin objects. @@ -1077,7 +1081,7 @@ def __init__( self._inicache: dict[str, Any] = {} self._override_ini: Sequence[str] = () self._opt2dest: dict[str, str] = {} - self._cleanup: list[Callable[[], None]] = [] + self._exit_stack = contextlib.ExitStack() self.pluginmanager.register(self, "pytestconfig") self._configured = False self.hook.pytest_addoption.call_historic( @@ -1104,10 +1108,11 @@ def inipath(self) -> pathlib.Path | None: """ return self._inipath - def add_cleanup(self, func: Callable[[], None]) -> None: + def add_cleanup(self, func: _T_callback) -> _T_callback: """Add a function to be called when the config object gets out of use (usually coinciding with pytest_unconfigure).""" - self._cleanup.append(func) + self._exit_stack.callback(func) + return func def _do_configure(self) -> None: assert not self._configured @@ -1117,13 +1122,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._exit_stack.close() + finally: + self._exit_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: