From 33234efc0044becf8735e6868573b1d11ccb4d9d Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Thu, 21 Nov 2024 09:48:56 +0000 Subject: [PATCH 1/8] 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: From 4391b12054bd7e8c4c52f98b785a85ca7dea799d Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Thu, 21 Nov 2024 10:01:34 +0000 Subject: [PATCH 2/8] add changelog --- changelog/12981.bugfix.rst | 1 + 1 file changed, 1 insertion(+) 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..ae447ee578a --- /dev/null +++ b/changelog/12981.bugfix.rst @@ -0,0 +1 @@ +Prevent exceptions in pytest.Config.add_cleanup preventing further cleanups From a76eb1f447fa44b9eb564a6cc3e11c3d438bcd56 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Thu, 21 Nov 2024 14:21:40 +0000 Subject: [PATCH 3/8] Apply suggestions from code review Co-authored-by: Bruno Oliveira --- changelog/12981.bugfix.rst | 2 +- src/_pytest/config/__init__.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/changelog/12981.bugfix.rst b/changelog/12981.bugfix.rst index ae447ee578a..f995480970d 100644 --- a/changelog/12981.bugfix.rst +++ b/changelog/12981.bugfix.rst @@ -1 +1 @@ -Prevent exceptions in pytest.Config.add_cleanup preventing further cleanups +Prevent exceptions in :func:`pytest.Config.add_cleanup` preventing further cleanups. diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 4141926c7f2..c898a924125 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -1110,7 +1110,10 @@ def inipath(self) -> pathlib.Path | 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).""" + use (usually coinciding with pytest_unconfigure). + + Returns the passed function. + """ self._exit_stack.callback(func) return func From 3e44b998385e6eb05a7998b5b5b8705074384f5b Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 21 Nov 2024 14:23:00 +0000 Subject: [PATCH 4/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/_pytest/config/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index c898a924125..d00ecb876fa 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -1111,7 +1111,7 @@ def inipath(self) -> pathlib.Path | 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). - + Returns the passed function. """ self._exit_stack.callback(func) From cd11075287d52f75f91e38ee3503659865788692 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Thu, 21 Nov 2024 14:34:22 +0000 Subject: [PATCH 5/8] rename self._exit_stack to self._cleanup_stack --- src/_pytest/config/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index d00ecb876fa..fda16b9f766 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -1081,7 +1081,7 @@ def __init__( self._inicache: dict[str, Any] = {} self._override_ini: Sequence[str] = () self._opt2dest: dict[str, str] = {} - self._exit_stack = contextlib.ExitStack() + self._cleanup_stack = contextlib.ExitStack() self.pluginmanager.register(self, "pytestconfig") self._configured = False self.hook.pytest_addoption.call_historic( @@ -1114,7 +1114,7 @@ def add_cleanup(self, func: _T_callback) -> _T_callback: Returns the passed function. """ - self._exit_stack.callback(func) + self._cleanup_stack.callback(func) return func def _do_configure(self) -> None: @@ -1134,9 +1134,9 @@ def _ensure_unconfigure(self) -> None: self.hook.pytest_configure._call_history = [] finally: try: - self._exit_stack.close() + self._cleanup_stack.close() finally: - self._exit_stack = contextlib.ExitStack() + self._cleanup_stack = contextlib.ExitStack() def get_terminal_writer(self) -> TerminalWriter: terminalreporter: TerminalReporter | None = self.pluginmanager.get_plugin( From 6c427964aeab745cbef17c3b2d314ff69e407474 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Thu, 21 Nov 2024 14:36:30 +0000 Subject: [PATCH 6/8] Update src/_pytest/config/__init__.py --- src/_pytest/config/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index fda16b9f766..2413de6a218 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -1112,7 +1112,7 @@ 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). - Returns the passed function. + Returns the passed function, so you can use @config.add_cleanup as a decorator """ self._cleanup_stack.callback(func) return func From dcc6ce059b0243202f8e23d537a64a46ca15ed23 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Fri, 22 Nov 2024 13:37:16 +0000 Subject: [PATCH 7/8] remove return value from add_cleanup --- src/_pytest/config/__init__.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 2413de6a218..92cf565e85f 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -34,7 +34,6 @@ from typing import TextIO from typing import Type from typing import TYPE_CHECKING -from typing import TypeVar import warnings import pluggy @@ -75,9 +74,6 @@ 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. @@ -1108,14 +1104,11 @@ def inipath(self) -> pathlib.Path | None: """ return self._inipath - def add_cleanup(self, func: _T_callback) -> _T_callback: + 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). - - Returns the passed function, so you can use @config.add_cleanup as a decorator """ self._cleanup_stack.callback(func) - return func def _do_configure(self) -> None: assert not self._configured From 22c2a04cc77c0c48ada19c03b0711e339842be7d Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Fri, 22 Nov 2024 13:38:02 +0000 Subject: [PATCH 8/8] Update changelog/12981.bugfix.rst --- changelog/12981.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/12981.bugfix.rst b/changelog/12981.bugfix.rst index f995480970d..5fc8e29656f 100644 --- a/changelog/12981.bugfix.rst +++ b/changelog/12981.bugfix.rst @@ -1 +1 @@ -Prevent exceptions in :func:`pytest.Config.add_cleanup` preventing further cleanups. +Prevent exceptions in :func:`pytest.Config.add_cleanup` callbacks preventing further cleanups.