diff --git a/changelog/15067.bugfix.rst b/changelog/15067.bugfix.rst new file mode 100644 index 00000000000..4f44cec1da5 --- /dev/null +++ b/changelog/15067.bugfix.rst @@ -0,0 +1 @@ +Fixture finalizers now run when fixture setup is interrupted by a ``BaseException`` such as ``KeyboardInterrupt`` instead of being silently skipped, matching the documented behavior of :meth:`request.addfinalizer`. This also prevents the fixture's finalizer list from leaking. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 30f44d44dfc..8a6610f28da 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1204,9 +1204,11 @@ def addfinalizer(self, finalizer: Callable[[], object]) -> None: self._finalizers.append(finalizer) def finish(self, request: SubRequest) -> None: - if self.cached_result is None: - # Already finished. It is assumed that finalizers cannot be added in - # this state. + if self.cached_result is None and not self._finalizers: + # Already finished, and no finalizers were registered, so there is + # nothing to clean up. Note that a setup interrupted by a + # BaseException (e.g. KeyboardInterrupt) has no cached result but + # may still have pending finalizers; those must run (#15067). return exceptions: list[BaseException] = [] diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py index 27c5739845a..ca9745df413 100644 --- a/src/_pytest/runner.py +++ b/src/_pytest/runner.py @@ -128,6 +128,7 @@ def runtestprotocol( # This only happens if the item is re-run, as is done by # pytest-rerunfailures. item._initrequest() # type: ignore[attr-defined] + reports: list[TestReport] = [] try: rep = call_and_report(item, "setup", log) reports = [rep] @@ -137,17 +138,23 @@ def runtestprotocol( show_test_item(item, add_space=not setup_only) if not setup_only: reports.append(call_and_report(item, "call", log)) + finally: # If the session is about to fail or stop, teardown everything - this is # necessary to correctly report fixture teardown errors (see #11706) if item.session.shouldfail or item.session.shouldstop: nextitem = None - reports.append(call_and_report(item, "teardown", log, nextitem=nextitem)) - finally: - # After all teardown hooks have been called (or an exception was reraised) - # want funcargs and request info to go away. - if hasrequest: - item._request = False # type: ignore[attr-defined] - item.funcargs = None # type: ignore[attr-defined] + # Teardown must run even when setup (or call) re-raised an interruptible + # exception such as KeyboardInterrupt, so that finalizers registered + # before the interruption are still executed (see #15067). The request + # cleanup must run regardless of whether teardown itself re-raises. + try: + reports.append(call_and_report(item, "teardown", log, nextitem=nextitem)) + finally: + # After all teardown hooks have been called (or an exception was reraised) + # want funcargs and request info to go away. + if hasrequest: + item._request = False # type: ignore[attr-defined] + item.funcargs = None # type: ignore[attr-defined] return reports diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index bc7b5a40cc2..f7558bdb6ab 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -3526,6 +3526,37 @@ def test_other(): reprec = pytester.inline_run("-lvs") reprec.assertoutcome(passed=3) + def test_finalizer_runs_when_setup_interrupted_by_keyboard_interrupt( + self, pytester: Pytester + ) -> None: + """Finalizers registered before a KeyboardInterrupt interrupts fixture + setup must still run (#15067).""" + marker = pytester.path / "finalizer-marker.txt" + item = pytester.getitem( + f""" + import pytest + from pathlib import Path + + marker = {str(marker)!r} + + @pytest.fixture + def resource(request): + request.addfinalizer(lambda: Path(marker).write_text("ran", encoding="utf-8")) + raise KeyboardInterrupt + + def test_func(resource): + assert resource + """ + ) + + from _pytest import runner + + try: + runner.runtestprotocol(item, log=False) + except KeyboardInterrupt: + pass + assert Path(marker).read_text(encoding="utf-8") == "ran" + def test_class_scope_parametrization_ordering(self, pytester: Pytester) -> None: """#396""" pytester.makepyfile( diff --git a/testing/test_runner.py b/testing/test_runner.py index b5c3839c79e..3ab134ca5a5 100644 --- a/testing/test_runner.py +++ b/testing/test_runner.py @@ -526,6 +526,37 @@ def test_func(resource): assert not cast(object, item._request) assert not item.funcargs + def test_keyboardinterrupt_during_teardown_clears_request( + self, pytester: Pytester + ) -> None: + """Interrupting teardown must not skip clearing the item's request and + funcargs (#15067).""" + item = pytester.getitem( + """ + import pytest + + @pytest.fixture + def resource(request): + yield + raise KeyboardInterrupt("fake") + + def test_func(resource): + pass + """ + ) + assert isinstance(item, pytest.Function) + assert item._request + + try: + runner.runtestprotocol(item, log=False) + except KeyboardInterrupt: + pass + else: + assert False, "did not raise" + + assert not cast(object, item._request) + assert not item.funcargs + class TestSessionReports: def test_collect_result(self, pytester: Pytester) -> None: