Skip to content

Commit

Permalink
Prevent Config.add_cleanup callbacks preventing other cleanups running (
Browse files Browse the repository at this point in the history
#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 <[email protected]>
  • Loading branch information
graingert and nicoddemus authored Nov 24, 2024
1 parent 72f17d1 commit 4f777ff
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 11 deletions.
1 change: 1 addition & 0 deletions changelog/12981.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Prevent exceptions in :func:`pytest.Config.add_cleanup` callbacks preventing further cleanups.
28 changes: 17 additions & 11 deletions src/_pytest/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import argparse
import collections.abc
import contextlib
import copy
import dataclasses
import enum
Expand Down Expand Up @@ -73,7 +74,6 @@
from _pytest.cacheprovider import Cache
from _pytest.terminal import TerminalReporter


_PluggyPlugin = object
"""A type to represent plugin objects.
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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(
Expand Down
31 changes: 31 additions & 0 deletions testing/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit 4f777ff

Please sign in to comment.