From c78734c7eb79906fc2a75a85b0be63dc0f5c07fb Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Sun, 11 Jan 2026 16:58:25 +0300 Subject: [PATCH 01/24] Initial --- changelog/14095.bugfix.rst | 1 + changelog/14103.bugfix.rst | 1 + src/_pytest/fixtures.py | 139 ++++++++++++++++++++++---------- testing/python/fixtures.py | 161 +++++++++++++++++++++++++++++++++++++ 4 files changed, 259 insertions(+), 43 deletions(-) create mode 100644 changelog/14095.bugfix.rst create mode 100644 changelog/14103.bugfix.rst diff --git a/changelog/14095.bugfix.rst b/changelog/14095.bugfix.rst new file mode 100644 index 00000000000..886c3b6894a --- /dev/null +++ b/changelog/14095.bugfix.rst @@ -0,0 +1 @@ +Fixtures are now rebuilt when fixture argnames they depend on resolve to a different fixturedef. diff --git a/changelog/14103.bugfix.rst b/changelog/14103.bugfix.rst new file mode 100644 index 00000000000..13e5c7bdefd --- /dev/null +++ b/changelog/14103.bugfix.rst @@ -0,0 +1 @@ +Fixtures are now rebuilt when param changes for a fixture they depend on, if the dependency is via ``request.getfixturevalue()``. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index d8d19fcac6d..645d75a2844 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -546,6 +546,11 @@ def _iter_chain(self) -> Iterator[SubRequest]: yield current current = current._parent_request + def _account_dependency(self, fixturedef: FixtureDef[object]) -> None: + if isinstance(self, SubRequest): + assert self._fixturedef._requested_fixtures is not None + self._fixturedef._requested_fixtures[fixturedef] = None + def _get_active_fixturedef(self, argname: str) -> FixtureDef[object]: if argname == "request": return RequestFixtureDef(self) @@ -555,8 +560,14 @@ def _get_active_fixturedef(self, argname: str) -> FixtureDef[object]: fixturedef = self._fixture_defs.get(argname) if fixturedef is not None: self._check_scope(fixturedef, fixturedef._scope) + self._account_dependency(fixturedef) return fixturedef + fixturedef = self._find_fixturedef(argname) + self._execute_fixturedef(fixturedef) + return fixturedef + + def _find_fixturedef(self, argname: str) -> FixtureDef[object]: # Find the appropriate fixturedef. fixturedefs = self._arg2fixturedefs.get(argname, None) if fixturedefs is None: @@ -589,8 +600,12 @@ def _get_active_fixturedef(self, argname: str) -> FixtureDef[object]: # If already consumed all of the available levels, fail. if -index > len(fixturedefs): raise FixtureLookupError(argname, self) - fixturedef = fixturedefs[index] + return fixturedefs[index] + def _execute_fixturedef( + self, fixturedef: FixtureDef[object], *, only_maintain_cache: bool = False + ) -> None: + argname = fixturedef.argname # Prepare a SubRequest object for calling the fixture. try: callspec = self._pyfuncitem.callspec @@ -616,11 +631,15 @@ def _get_active_fixturedef(self, argname: str) -> FixtureDef[object]: self, scope, param, param_index, fixturedef, _ispytest=True ) + if only_maintain_cache: + fixturedef._maintain_cache(subrequest) + return + + self._account_dependency(fixturedef) # Make sure the fixture value is cached, running it if it isn't fixturedef.execute(request=subrequest) self._fixture_defs[argname] = fixturedef - return fixturedef def _check_fixturedef_without_param(self, fixturedef: FixtureDef[object]) -> None: """Check that this request is allowed to execute this fixturedef without @@ -636,7 +655,7 @@ def _check_fixturedef_without_param(self, fixturedef: FixtureDef[object]) -> Non ) fail(msg, pytrace=False) if has_params: - frame = inspect.stack()[3] + frame = inspect.stack()[4] frameinfo = inspect.getframeinfo(frame[0]) source_path = absolutepath(frameinfo.filename) source_lineno = frameinfo.lineno @@ -950,6 +969,22 @@ def _eval_scope_callable( return result +def _get_cached_value( + cached_result: _FixtureCachedResult[FixtureValue], +) -> FixtureValue: + if cached_result[2] is not None: + exc, exc_tb = cached_result[2] + raise exc.with_traceback(exc_tb) + else: + return cached_result[0] + + +def _mypy_disable_narrowing( + cached_result: _FixtureCachedResult[FixtureValue] | None, +) -> _FixtureCachedResult[FixtureValue] | None: + return cached_result + + class FixtureDef(Generic[FixtureValue]): """A container for a fixture definition. @@ -1015,6 +1050,9 @@ def __init__( # Can change if the fixture is executed with different parameters. self.cached_result: _FixtureCachedResult[FixtureValue] | None = None self._finalizers: Final[list[Callable[[], object]]] = [] + # Tracks dependencies discovered during the current execution. + # Using dict for a stable order. + self._requested_fixtures: dict[FixtureDef[Any], None] | None = None # only used to emit a deprecationwarning, can be removed in pytest9 self._autouse = _autouse @@ -1042,59 +1080,68 @@ def finish(self, request: SubRequest) -> None: # which will keep instances alive. self.cached_result = None self._finalizers.clear() + self._requested_fixtures = None if len(exceptions) == 1: raise exceptions[0] elif len(exceptions) > 1: msg = f'errors while tearing down fixture "{self.argname}" of {node}' raise BaseExceptionGroup(msg, exceptions[::-1]) - def execute(self, request: SubRequest) -> FixtureValue: - """Return the value of this fixture, executing it if not cached.""" - # Ensure that the dependent fixtures requested by this fixture are loaded. + def _maintain_cache(self, request: SubRequest) -> None: + """Call finish() if param changes.""" + if self.cached_result is None: + return + + assert self._requested_fixtures is not None + # Ensure that the dependent fixtures requested by this fixture are checked. # This needs to be done before checking if we have a cached value, since # if a dependent fixture has their cache invalidated, e.g. due to # parametrization, they finalize themselves and fixtures depending on it - # (which will likely include this fixture) setting `self.cached_result = None`. - # See #4871 - requested_fixtures_that_should_finalize_us = [] - for argname in self.argnames: - fixturedef = request._get_active_fixturedef(argname) - # Saves requested fixtures in a list so we later can add our finalizer - # to them, ensuring that if a requested fixture gets torn down we get torn - # down first. This is generally handled by SetupState, but still currently - # needed when this fixture is not parametrized but depends on a parametrized - # fixture. - requested_fixtures_that_should_finalize_us.append(fixturedef) - - # Check for (and return) cached value/exception. - if self.cached_result is not None: - request_cache_key = self.cache_key(request) - cache_key = self.cached_result[1] - try: - # Attempt to make a normal == check: this might fail for objects - # which do not implement the standard comparison (like numpy arrays -- #6497). - cache_hit = bool(request_cache_key == cache_key) - except (ValueError, RuntimeError): - # If the comparison raises, use 'is' as fallback. - cache_hit = request_cache_key is cache_key - - if cache_hit: - if self.cached_result[2] is not None: - exc, exc_tb = self.cached_result[2] - raise exc.with_traceback(exc_tb) - else: - return self.cached_result[0] + # (which will include this fixture) + # setting `self.cached_result = None`. See #4871 + for fixturedef in self._requested_fixtures: + if request._fixture_defs.get(fixturedef.argname) is fixturedef: + # This dependency has already been computed in this item. + continue + if request._find_fixturedef(fixturedef.argname) is not fixturedef: + # This item should use a different fixturedef than was cached. (#14095) + self.finish(request) + return + request._execute_fixturedef(fixturedef, only_maintain_cache=True) + if _mypy_disable_narrowing(self.cached_result) is None: + # Do not finalize more fixtures than necessary. + return + + request_cache_key = self.cache_key(request) + cache_key = self.cached_result[1] + try: + # Attempt to make a normal == check: this might fail for objects + # which do not implement the standard comparison (like numpy arrays -- #6497). + cache_hit = bool(request_cache_key == cache_key) + except (ValueError, RuntimeError): + # If the comparison raises, use 'is' as fallback. + cache_hit = request_cache_key is cache_key + + if not cache_hit: # We have a previous but differently parametrized fixture instance # so we need to tear it down before creating a new one. self.finish(request) - assert self.cached_result is None - # Add finalizer to requested fixtures we saved previously. - # We make sure to do this after checking for cached value to avoid - # adding our finalizer multiple times. (#12135) - finalizer = functools.partial(self.finish, request=request) - for parent_fixture in requested_fixtures_that_should_finalize_us: - parent_fixture.addfinalizer(finalizer) + def execute(self, request: SubRequest) -> FixtureValue: + """Return the value of this fixture, executing it if not cached.""" + self._maintain_cache(request) + if self.cached_result is not None: + return _get_cached_value(self.cached_result) + + # _get_active_fixturedef calls fill our _requested_fixtures + # so we later can add our finalizer to them, ensuring that + # if a requested fixture gets torn down we get torn down first. + self._requested_fixtures = {} + + # Execute fixtures from argnames here to make sure that analytics + # in pytest_fixture_setup only handle the body of the current fixture. + for argname in self.argnames: + request._get_active_fixturedef(argname) ihook = request.node.ihook try: @@ -1105,7 +1152,13 @@ def execute(self, request: SubRequest) -> FixtureValue: ) finally: # Schedule our finalizer, even if the setup failed. + finalizer = functools.partial(self.finish, request=request) request.node.addfinalizer(finalizer) + # Add finalizer to requested fixtures. + # We make sure to do this after checking for cached value to avoid + # adding our finalizer multiple times. (#12135) + for parent_fixture in self._requested_fixtures: + parent_fixture.addfinalizer(finalizer) return result diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 8b9e3fbb0a5..066b8bb7c43 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -5399,3 +5399,164 @@ def test_it(request, fix1): ) result = pytester.runpytest("-v") result.assert_outcomes(passed=1) + + +def test_getfixturevalue_parametrized_dependency_tracked(pytester: Pytester) -> None: + """ + Test that a fixture depending on a parametrized fixture via getfixturevalue + is properly recomputed when the parametrized fixture changes (#14103). + """ + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="session") + def foo(request): + return request.param + + @pytest.fixture(scope="session") + def bar(request): + return request.getfixturevalue("foo") + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_first(foo, bar): + assert bar == 1 + + @pytest.mark.parametrize("foo", [2], indirect=True) + def test_second(foo, bar): + assert bar == 2 + """ + ) + result = pytester.runpytest("--setup-show", "-vv") + result.assert_outcomes(passed=2) + result.stdout.fnmatch_lines( + [ + "test_fixtures.py::test_first[1] ", + "SETUP S foo[1]", + "SETUP S bar", + " test_fixtures.py::test_first[1] (fixtures used: bar, foo, request)PASSED", + "test_fixtures.py::test_second[2] ", + "TEARDOWN S bar", + "TEARDOWN S foo[1]", + "SETUP S foo[2]", + "SETUP S bar", + " test_fixtures.py::test_second[2] (fixtures used: bar, foo, request)PASSED", + "TEARDOWN S bar", + "TEARDOWN S foo[2]", + ], + consecutive=True, + ) + + +def test_fixture_override_finishes_dependencies(pytester: Pytester) -> None: + """Test that a fixture gets recomputed if its dependency resolves to a different fixturedef (#14095).""" + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="session") + def foo(): + return "outer" + + @pytest.fixture(scope="session") + def bar(foo): + return f"dependent_{foo}" + + def test_before_class(bar): + assert bar == "dependent_outer" + + class TestOverride: + @pytest.fixture(scope="session") + def foo(self): + return "inner" + + def test_in_class(self, bar): + assert bar == "dependent_inner" + + def test_after_class(bar): + assert bar == "dependent_outer" + """ + ) + result = pytester.runpytest("--setup-show", "-vv") + result.assert_outcomes(passed=3) + result.stdout.fnmatch_lines( + [ + "test_fixtures.py::test_before_class ", + "SETUP S foo", + "SETUP S bar (fixtures used: foo)", + " test_fixtures.py::test_before_class (fixtures used: bar, foo)PASSED", + "test_fixtures.py::TestOverride::test_in_class ", + # bar is recomputed because foo resolves to a different fixturedef. + "TEARDOWN S bar", + "SETUP S foo", # The inner foo. + "SETUP S bar (fixtures used: foo)", + " test_fixtures.py::TestOverride::test_in_class (fixtures used: bar, foo)PASSED", + "test_fixtures.py::test_after_class ", + # bar is recomputed because foo resolves to a different fixturedef. + "TEARDOWN S bar", + "SETUP S bar (fixtures used: foo)", + " test_fixtures.py::test_after_class (fixtures used: bar, foo)PASSED", + "TEARDOWN S bar", + "TEARDOWN S foo", + "TEARDOWN S foo", + ], + consecutive=True, + ) + + +def test_override_fixture_with_new_parametrized_fixture(pytester: Pytester) -> None: + """Test what happens when a cached fixture is overridden by a new parametrized fixture, + and another fixture depends on it. + + This test verifies that: + 1. A fixture can be overridden by a parametrized fixture in a nested scope + 2. Dependent fixtures get recomputed because a dependency now resolves to a different fixturedef + 3. The outer fixture is not setup or finalized unnecessarily + """ + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="session") + def foo(request): + assert not hasattr(request, "param") + return "outer" + + @pytest.fixture(scope="session") + def bar(foo): + return f"dependent_{foo}" + + def test_before_class(foo, bar): + assert foo == "outer" + assert bar == "dependent_outer" + + class TestOverride: + @pytest.fixture(scope="session") + def foo(self, request): + return f"inner_{request.param}" + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_in_class(self, foo, bar): + assert foo == "inner_1" + assert bar == "dependent_inner_1" + """ + ) + result = pytester.runpytest("--setup-show", "-vv") + result.assert_outcomes(passed=2) + result.stdout.fnmatch_lines( + [ + "test_fixtures.py::test_before_class ", + "SETUP S foo", + "SETUP S bar (fixtures used: foo)", + " test_fixtures.py::test_before_class (fixtures used: bar, foo, request)PASSED", + "test_fixtures.py::TestOverride::test_in_class[1] ", + "SETUP S foo[1]", + "TEARDOWN S bar", + "SETUP S bar (fixtures used: foo)", + " test_fixtures.py::TestOverride::test_in_class[1] (fixtures used: bar, foo, request)PASSED", + "TEARDOWN S bar", + "TEARDOWN S foo[1]", + "TEARDOWN S foo", + ], + consecutive=True, + ) From ba0ba86abb823b1d8278aa2bd15e1ac719cd79bb Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Mon, 12 Jan 2026 06:01:09 +0300 Subject: [PATCH 02/24] Always fixture finish in teardown phase --- changelog/14095.bugfix.rst | 1 - src/_pytest/fixtures.py | 173 ++++++++++++++++++++++--------------- src/_pytest/main.py | 2 + src/_pytest/pytester.py | 2 + testing/python/fixtures.py | 7 +- 5 files changed, 109 insertions(+), 76 deletions(-) delete mode 100644 changelog/14095.bugfix.rst diff --git a/changelog/14095.bugfix.rst b/changelog/14095.bugfix.rst deleted file mode 100644 index 886c3b6894a..00000000000 --- a/changelog/14095.bugfix.rst +++ /dev/null @@ -1 +0,0 @@ -Fixtures are now rebuilt when fixture argnames they depend on resolve to a different fixturedef. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 645d75a2844..f3772507429 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -16,6 +16,7 @@ import dataclasses import functools import inspect +import itertools import os from pathlib import Path import sys @@ -74,6 +75,8 @@ if TYPE_CHECKING: + from _pytest.nodes import Item + from _pytest.nodes import Node from _pytest.python import CallSpec2 from _pytest.python import Function from _pytest.python import Metafunc @@ -421,7 +424,7 @@ def fixturenames(self) -> list[str]: @property @abc.abstractmethod - def node(self): + def node(self) -> Node: """Underlying collection node (depends on current request scope).""" raise NotImplementedError() @@ -549,7 +552,7 @@ def _iter_chain(self) -> Iterator[SubRequest]: def _account_dependency(self, fixturedef: FixtureDef[object]) -> None: if isinstance(self, SubRequest): assert self._fixturedef._requested_fixtures is not None - self._fixturedef._requested_fixtures[fixturedef] = None + self._fixturedef._requested_fixtures[fixturedef.argname] = fixturedef def _get_active_fixturedef(self, argname: str) -> FixtureDef[object]: if argname == "request": @@ -602,15 +605,10 @@ def _find_fixturedef(self, argname: str) -> FixtureDef[object]: raise FixtureLookupError(argname, self) return fixturedefs[index] - def _execute_fixturedef( - self, fixturedef: FixtureDef[object], *, only_maintain_cache: bool = False - ) -> None: + def _execute_fixturedef(self, fixturedef: FixtureDef[object]) -> None: argname = fixturedef.argname # Prepare a SubRequest object for calling the fixture. - try: - callspec = self._pyfuncitem.callspec - except AttributeError: - callspec = None + callspec: CallSpec2 | None = getattr(self._pyfuncitem, "callspec", None) if callspec is not None and argname in callspec.params: param = callspec.params[argname] param_index = callspec.indices[argname] @@ -631,10 +629,6 @@ def _execute_fixturedef( self, scope, param, param_index, fixturedef, _ispytest=True ) - if only_maintain_cache: - fixturedef._maintain_cache(subrequest) - return - self._account_dependency(fixturedef) # Make sure the fixture value is cached, running it if it isn't fixturedef.execute(request=subrequest) @@ -706,7 +700,7 @@ def _check_scope( pass @property - def node(self): + def node(self) -> Node: return self._pyfuncitem def __repr__(self) -> str: @@ -759,7 +753,7 @@ def _scope(self) -> Scope: return self._scope_field @property - def node(self): + def node(self) -> Node: scope = self._scope if scope is Scope.Function: # This might also be a non-function Item despite its attribute name. @@ -979,12 +973,6 @@ def _get_cached_value( return cached_result[0] -def _mypy_disable_narrowing( - cached_result: _FixtureCachedResult[FixtureValue] | None, -) -> _FixtureCachedResult[FixtureValue] | None: - return cached_result - - class FixtureDef(Generic[FixtureValue]): """A container for a fixture definition. @@ -1050,9 +1038,14 @@ def __init__( # Can change if the fixture is executed with different parameters. self.cached_result: _FixtureCachedResult[FixtureValue] | None = None self._finalizers: Final[list[Callable[[], object]]] = [] + # ID of the latest cache computation. Prevents finalizing a more recent + # evaluation of the fixture than expected. + self._cache_epoch: int = 0 + # SubRequest to use in finish for the current cached result. + self._cached_request: SubRequest | None = None # Tracks dependencies discovered during the current execution. - # Using dict for a stable order. - self._requested_fixtures: dict[FixtureDef[Any], None] | None = None + # Filled by FixtureRequest. + self._requested_fixtures: dict[str, FixtureDef[Any]] | None = None # only used to emit a deprecationwarning, can be removed in pytest9 self._autouse = _autouse @@ -1074,70 +1067,33 @@ def finish(self, request: SubRequest) -> None: except BaseException as e: exceptions.append(e) node = request.node - node.ihook.pytest_fixture_post_finalizer(fixturedef=self, request=request) + try: + node.ihook.pytest_fixture_post_finalizer(fixturedef=self, request=request) + except BaseException as e: + exceptions.append(e) # Even if finalization fails, we invalidate the cached fixture # value and remove all finalizers because they may be bound methods # which will keep instances alive. self.cached_result = None self._finalizers.clear() - self._requested_fixtures = None + self._cache_epoch += 1 + self._cached_request = None if len(exceptions) == 1: raise exceptions[0] elif len(exceptions) > 1: msg = f'errors while tearing down fixture "{self.argname}" of {node}' raise BaseExceptionGroup(msg, exceptions[::-1]) - def _maintain_cache(self, request: SubRequest) -> None: - """Call finish() if param changes.""" - if self.cached_result is None: - return - - assert self._requested_fixtures is not None - # Ensure that the dependent fixtures requested by this fixture are checked. - # This needs to be done before checking if we have a cached value, since - # if a dependent fixture has their cache invalidated, e.g. due to - # parametrization, they finalize themselves and fixtures depending on it - # (which will include this fixture) - # setting `self.cached_result = None`. See #4871 - for fixturedef in self._requested_fixtures: - if request._fixture_defs.get(fixturedef.argname) is fixturedef: - # This dependency has already been computed in this item. - continue - if request._find_fixturedef(fixturedef.argname) is not fixturedef: - # This item should use a different fixturedef than was cached. (#14095) - self.finish(request) - return - request._execute_fixturedef(fixturedef, only_maintain_cache=True) - if _mypy_disable_narrowing(self.cached_result) is None: - # Do not finalize more fixtures than necessary. - return - - request_cache_key = self.cache_key(request) - cache_key = self.cached_result[1] - try: - # Attempt to make a normal == check: this might fail for objects - # which do not implement the standard comparison (like numpy arrays -- #6497). - cache_hit = bool(request_cache_key == cache_key) - except (ValueError, RuntimeError): - # If the comparison raises, use 'is' as fallback. - cache_hit = request_cache_key is cache_key - - if not cache_hit: - # We have a previous but differently parametrized fixture instance - # so we need to tear it down before creating a new one. - self.finish(request) - def execute(self, request: SubRequest) -> FixtureValue: """Return the value of this fixture, executing it if not cached.""" - self._maintain_cache(request) if self.cached_result is not None: + self._check_cache_hit(request, self.cached_result[1]) + if self._should_finalize_in_current_item(request): + finalizer = self._make_finalizer(request) + request._pyfuncitem.addfinalizer(finalizer) return _get_cached_value(self.cached_result) - # _get_active_fixturedef calls fill our _requested_fixtures - # so we later can add our finalizer to them, ensuring that - # if a requested fixture gets torn down we get torn down first. self._requested_fixtures = {} - # Execute fixtures from argnames here to make sure that analytics # in pytest_fixture_setup only handle the body of the current fixture. for argname in self.argnames: @@ -1152,19 +1108,92 @@ def execute(self, request: SubRequest) -> FixtureValue: ) finally: # Schedule our finalizer, even if the setup failed. - finalizer = functools.partial(self.finish, request=request) + finalizer = self._make_finalizer(request) + if self._should_finalize_in_current_item(request): + request._pyfuncitem.addfinalizer(finalizer) request.node.addfinalizer(finalizer) # Add finalizer to requested fixtures. # We make sure to do this after checking for cached value to avoid # adding our finalizer multiple times. (#12135) - for parent_fixture in self._requested_fixtures: + for parent_fixture in self._requested_fixtures.values(): parent_fixture.addfinalizer(finalizer) return result + def _is_cache_hit(self, old_cache_key: object, new_cache_key: object) -> bool: + try: + # Attempt to make a normal == check: this might fail for objects + # which do not implement the standard comparison (like numpy arrays -- #6497). + cache_hit = bool(new_cache_key == old_cache_key) + except (ValueError, RuntimeError): + # If the comparison raises, use 'is' as fallback. + cache_hit = new_cache_key is old_cache_key + return cache_hit + + def _make_finalizer(self, request: SubRequest) -> Callable[[], object]: + self._cached_request = request + cache_epoch = self._cache_epoch + + def finalizer() -> None: + if self._cache_epoch == cache_epoch: + assert self._cached_request is not None + self.finish(self._cached_request) + + return finalizer + + def _should_finalize_in_current_item(self, request: SubRequest) -> bool: + if self.cached_result is None: + return False + + node = request.node + session = request.session + current_item_index = session._current_item_index + assert current_item_index is not None + + for nextitem in itertools.islice(session.items, current_item_index + 1, None): + if node not in nextitem.iter_parents(): + break + fixtureinfo: FuncFixtureInfo = getattr(nextitem, "_fixtureinfo") + if self not in fixtureinfo.name2fixturedefs.get(self.argname, ()): + continue + # Found the next item that uses the fixture. Should finalize in current + # item if the cache should not be kept for the next item. + old_cache_key = self.cached_result[1] + new_cache_key = self._cache_key_internal(nextitem) + return not self._is_cache_hit(old_cache_key, new_cache_key) + + # It is enough to finish the fixture at the level of request.node. + return False + + def _check_cache_hit(self, request: SubRequest, old_cache_key: object) -> None: + new_cache_key = self.cache_key(request) + if self._is_cache_hit(old_cache_key, new_cache_key): + return + + item = request._pyfuncitem + location = getlocation(self.func, item.config.rootpath) + msg = ( + "The requested fixture has no parameter defined for test:\n" + f" {item.nodeid}\n\n" + f"Requested fixture '{self.argname}' defined in:\n" + f"{location}\n\n" + f"A previous test provided param={old_cache_key!r}, but the current test " + "both requested the fixture dynamically via 'getfixturevalue' and did not " + "provide a parameter for the fixture, which is not allowed. " + f"Please make fixture '{self.argname}' statically reachable " + "from the current test, e.g. by adding it as an argument to the test." + ) + fail(msg, pytrace=False) + def cache_key(self, request: SubRequest) -> object: return getattr(request, "param", None) + def _cache_key_internal(self, item: Item) -> object: + callspec: CallSpec2 | None = getattr(item, "callspec", None) + if callspec is not None and self.argname in callspec.params: + return callspec.params[self.argname] + return None + def __repr__(self) -> str: return f"" diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 02c7fb373fd..30de8ee95b3 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -392,6 +392,7 @@ def pytest_runtestloop(session: Session) -> bool: for i, item in enumerate(session.items): nextitem = session.items[i + 1] if i + 1 < len(session.items) else None + session._current_item_index = i item.config.hook.pytest_runtest_protocol(item=item, nextitem=nextitem) if session.shouldfail: raise session.Failed(session.shouldfail) @@ -608,6 +609,7 @@ def __init__(self, config: Config) -> None: self._initial_parts: list[CollectionArgument] = [] self._collection_cache: dict[nodes.Collector, CollectReport] = {} self.items: list[nodes.Item] = [] + self._current_item_index: int | None = None self._bestrelpathcache: dict[Path, str] = _bestrelpath_cache(config.rootpath) diff --git a/src/_pytest/pytester.py b/src/_pytest/pytester.py index 1cd5f05dd7e..def3cce672b 100644 --- a/src/_pytest/pytester.py +++ b/src/_pytest/pytester.py @@ -1280,6 +1280,8 @@ def getitem( items = self.getitems(source) for item in items: if item.name == funcname: + # HACK: keep item.session._setupstate.setup(item) working. + item.session._current_item_index = 0 return item assert 0, f"{funcname!r} item not found in module:\n{source}\nitems: {items}" diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 066b8bb7c43..0c85d8d4c47 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -5435,9 +5435,9 @@ def test_second(foo, bar): "SETUP S foo[1]", "SETUP S bar", " test_fixtures.py::test_first[1] (fixtures used: bar, foo, request)PASSED", - "test_fixtures.py::test_second[2] ", "TEARDOWN S bar", "TEARDOWN S foo[1]", + "test_fixtures.py::test_second[2] ", "SETUP S foo[2]", "SETUP S bar", " test_fixtures.py::test_second[2] (fixtures used: bar, foo, request)PASSED", @@ -5448,6 +5448,7 @@ def test_second(foo, bar): ) +@pytest.mark.xfail(reason="#14095") def test_fixture_override_finishes_dependencies(pytester: Pytester) -> None: """Test that a fixture gets recomputed if its dependency resolves to a different fixturedef (#14095).""" pytester.makepyfile( @@ -5549,14 +5550,14 @@ def test_in_class(self, foo, bar): "SETUP S foo", "SETUP S bar (fixtures used: foo)", " test_fixtures.py::test_before_class (fixtures used: bar, foo, request)PASSED", + "TEARDOWN S bar", + "TEARDOWN S foo", "test_fixtures.py::TestOverride::test_in_class[1] ", "SETUP S foo[1]", - "TEARDOWN S bar", "SETUP S bar (fixtures used: foo)", " test_fixtures.py::TestOverride::test_in_class[1] (fixtures used: bar, foo, request)PASSED", "TEARDOWN S bar", "TEARDOWN S foo[1]", - "TEARDOWN S foo", ], consecutive=True, ) From 643afdf95ed26d9d5e069b602dc04bb81a5b4a52 Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Mon, 12 Jan 2026 12:59:47 +0300 Subject: [PATCH 03/24] Fix current_item_index --- src/_pytest/fixtures.py | 28 +++++++++++++++++++++------- src/_pytest/main.py | 2 -- src/_pytest/pytester.py | 3 +-- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index f3772507429..d120570a99e 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -67,6 +67,7 @@ from _pytest.scope import _ScopeName from _pytest.scope import HIGH_SCOPES from _pytest.scope import Scope +from _pytest.stash import StashKey from _pytest.warning_types import PytestWarning @@ -75,8 +76,6 @@ if TYPE_CHECKING: - from _pytest.nodes import Item - from _pytest.nodes import Node from _pytest.python import CallSpec2 from _pytest.python import Function from _pytest.python import Metafunc @@ -424,7 +423,7 @@ def fixturenames(self) -> list[str]: @property @abc.abstractmethod - def node(self) -> Node: + def node(self): """Underlying collection node (depends on current request scope).""" raise NotImplementedError() @@ -700,7 +699,7 @@ def _check_scope( pass @property - def node(self) -> Node: + def node(self): return self._pyfuncitem def __repr__(self) -> str: @@ -753,7 +752,7 @@ def _scope(self) -> Scope: return self._scope_field @property - def node(self) -> Node: + def node(self): scope = self._scope if scope is Scope.Function: # This might also be a non-function Item despite its attribute name. @@ -973,6 +972,21 @@ def _get_cached_value( return cached_result[0] +_item_index = StashKey[int]() + + +def _get_item_index(item: nodes.Item) -> int: + if (index := item.stash.get(_item_index, None)) is not None: + return index + + for index, session_item in enumerate(item.session.items): + session_item.stash[_item_index] = index + + index = item.stash.get(_item_index, None) + assert index is not None, f"Item {item.nodeid} is not inside its session" + return index + + class FixtureDef(Generic[FixtureValue]): """A container for a fixture definition. @@ -1147,7 +1161,7 @@ def _should_finalize_in_current_item(self, request: SubRequest) -> bool: node = request.node session = request.session - current_item_index = session._current_item_index + current_item_index = _get_item_index(request._pyfuncitem) assert current_item_index is not None for nextitem in itertools.islice(session.items, current_item_index + 1, None): @@ -1188,7 +1202,7 @@ def _check_cache_hit(self, request: SubRequest, old_cache_key: object) -> None: def cache_key(self, request: SubRequest) -> object: return getattr(request, "param", None) - def _cache_key_internal(self, item: Item) -> object: + def _cache_key_internal(self, item: nodes.Item) -> object: callspec: CallSpec2 | None = getattr(item, "callspec", None) if callspec is not None and self.argname in callspec.params: return callspec.params[self.argname] diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 30de8ee95b3..02c7fb373fd 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -392,7 +392,6 @@ def pytest_runtestloop(session: Session) -> bool: for i, item in enumerate(session.items): nextitem = session.items[i + 1] if i + 1 < len(session.items) else None - session._current_item_index = i item.config.hook.pytest_runtest_protocol(item=item, nextitem=nextitem) if session.shouldfail: raise session.Failed(session.shouldfail) @@ -609,7 +608,6 @@ def __init__(self, config: Config) -> None: self._initial_parts: list[CollectionArgument] = [] self._collection_cache: dict[nodes.Collector, CollectReport] = {} self.items: list[nodes.Item] = [] - self._current_item_index: int | None = None self._bestrelpathcache: dict[Path, str] = _bestrelpath_cache(config.rootpath) diff --git a/src/_pytest/pytester.py b/src/_pytest/pytester.py index def3cce672b..ac22d7a1f0c 100644 --- a/src/_pytest/pytester.py +++ b/src/_pytest/pytester.py @@ -1040,6 +1040,7 @@ def genitems(self, colitems: Sequence[Item | Collector]) -> list[Item]: result: list[Item] = [] for colitem in colitems: result.extend(session.genitems(colitem)) + session.items.extend(result) return result def runitem(self, source: str) -> Any: @@ -1280,8 +1281,6 @@ def getitem( items = self.getitems(source) for item in items: if item.name == funcname: - # HACK: keep item.session._setupstate.setup(item) working. - item.session._current_item_index = 0 return item assert 0, f"{funcname!r} item not found in module:\n{source}\nitems: {items}" From 1553d3fd6469d236673ee5914dd13e3fc60ea62e Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Mon, 12 Jan 2026 17:31:57 +0300 Subject: [PATCH 04/24] Use nextitem to teardown stale fixtures --- src/_pytest/fixtures.py | 87 ++++++++++++++++------------------------- src/_pytest/nodes.py | 3 ++ src/_pytest/pytester.py | 1 - src/_pytest/python.py | 3 ++ src/_pytest/runner.py | 5 +++ 5 files changed, 44 insertions(+), 55 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index d120570a99e..df57ca6f0e8 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -16,7 +16,6 @@ import dataclasses import functools import inspect -import itertools import os from pathlib import Path import sys @@ -67,7 +66,6 @@ from _pytest.scope import _ScopeName from _pytest.scope import HIGH_SCOPES from _pytest.scope import Scope -from _pytest.stash import StashKey from _pytest.warning_types import PytestWarning @@ -972,21 +970,6 @@ def _get_cached_value( return cached_result[0] -_item_index = StashKey[int]() - - -def _get_item_index(item: nodes.Item) -> int: - if (index := item.stash.get(_item_index, None)) is not None: - return index - - for index, session_item in enumerate(item.session.items): - session_item.stash[_item_index] = index - - index = item.stash.get(_item_index, None) - assert index is not None, f"Item {item.nodeid} is not inside its session" - return index - - class FixtureDef(Generic[FixtureValue]): """A container for a fixture definition. @@ -1055,7 +1038,7 @@ def __init__( # ID of the latest cache computation. Prevents finalizing a more recent # evaluation of the fixture than expected. self._cache_epoch: int = 0 - # SubRequest to use in finish for the current cached result. + # SubRequest used during setup of the current result. self._cached_request: SubRequest | None = None # Tracks dependencies discovered during the current execution. # Filled by FixtureRequest. @@ -1091,6 +1074,7 @@ def finish(self, request: SubRequest) -> None: self.cached_result = None self._finalizers.clear() self._cache_epoch += 1 + request._fixturemanager._on_parametrized_fixture_finished(self) self._cached_request = None if len(exceptions) == 1: raise exceptions[0] @@ -1102,9 +1086,6 @@ def execute(self, request: SubRequest) -> FixtureValue: """Return the value of this fixture, executing it if not cached.""" if self.cached_result is not None: self._check_cache_hit(request, self.cached_result[1]) - if self._should_finalize_in_current_item(request): - finalizer = self._make_finalizer(request) - request._pyfuncitem.addfinalizer(finalizer) return _get_cached_value(self.cached_result) self._requested_fixtures = {} @@ -1122,9 +1103,9 @@ def execute(self, request: SubRequest) -> FixtureValue: ) finally: # Schedule our finalizer, even if the setup failed. - finalizer = self._make_finalizer(request) - if self._should_finalize_in_current_item(request): - request._pyfuncitem.addfinalizer(finalizer) + self._cached_request = request + finalizer = self._make_finalizer() + request._fixturemanager._on_parametrized_fixture_setup(self) request.node.addfinalizer(finalizer) # Add finalizer to requested fixtures. # We make sure to do this after checking for cached value to avoid @@ -1144,8 +1125,7 @@ def _is_cache_hit(self, old_cache_key: object, new_cache_key: object) -> bool: cache_hit = new_cache_key is old_cache_key return cache_hit - def _make_finalizer(self, request: SubRequest) -> Callable[[], object]: - self._cached_request = request + def _make_finalizer(self) -> Callable[[], object]: cache_epoch = self._cache_epoch def finalizer() -> None: @@ -1155,29 +1135,14 @@ def finalizer() -> None: return finalizer - def _should_finalize_in_current_item(self, request: SubRequest) -> bool: + def _finish_if_param_changed(self, item: nodes.Item) -> None: if self.cached_result is None: - return False - - node = request.node - session = request.session - current_item_index = _get_item_index(request._pyfuncitem) - assert current_item_index is not None - - for nextitem in itertools.islice(session.items, current_item_index + 1, None): - if node not in nextitem.iter_parents(): - break - fixtureinfo: FuncFixtureInfo = getattr(nextitem, "_fixtureinfo") - if self not in fixtureinfo.name2fixturedefs.get(self.argname, ()): - continue - # Found the next item that uses the fixture. Should finalize in current - # item if the cache should not be kept for the next item. - old_cache_key = self.cached_result[1] - new_cache_key = self._cache_key_internal(nextitem) - return not self._is_cache_hit(old_cache_key, new_cache_key) - - # It is enough to finish the fixture at the level of request.node. - return False + return + assert self._cached_request is not None + old_cache_key = self.cached_result[1] + new_cache_key = self._cache_key_internal(item) + if not self._is_cache_hit(old_cache_key, new_cache_key): + self.finish(self._cached_request) def _check_cache_hit(self, request: SubRequest, old_cache_key: object) -> None: new_cache_key = self.cache_key(request) @@ -1187,15 +1152,18 @@ def _check_cache_hit(self, request: SubRequest, old_cache_key: object) -> None: item = request._pyfuncitem location = getlocation(self.func, item.config.rootpath) msg = ( - "The requested fixture has no parameter defined for test:\n" + "Parameter for the requested fixture changed unexpectedly in test:\n" f" {item.nodeid}\n\n" f"Requested fixture '{self.argname}' defined in:\n" f"{location}\n\n" - f"A previous test provided param={old_cache_key!r}, but the current test " - "both requested the fixture dynamically via 'getfixturevalue' and did not " - "provide a parameter for the fixture, which is not allowed. " - f"Please make fixture '{self.argname}' statically reachable " - "from the current test, e.g. by adding it as an argument to the test." + f"Previous parameter value: {old_cache_key!r}\n" + f"New parameter value: {new_cache_key!r}\n\n" + f"This could happen because the current test requested the previously " + "parametrized fixture dynamically via 'getfixturevalue' and did not " + "provide a parameter for the fixture.\n" + "Either provide a parameter for the fixture, or make fixture " + f"'{self.argname}' statically reachable from the current test, " + "e.g. by adding it as an argument to the test function." ) fail(msg, pytrace=False) @@ -1676,6 +1644,7 @@ def __init__(self, session: Session) -> None: self._nodeid_autousenames: Final[dict[str, list[str]]] = { "": self.config.getini("usefixtures"), } + self._active_parametrized_fixturedefs: Final[set[FixtureDef[Any]]] = set() session.config.pluginmanager.register(self, "funcmanage") def getfixtureinfo( @@ -2023,6 +1992,16 @@ def _matchfactories( if fixturedef.baseid in parentnodeids: yield fixturedef + def _teardown_stale_fixtures(self, item: nodes.Item) -> None: + for fixturedef in list(self._active_parametrized_fixturedefs): + fixturedef._finish_if_param_changed(item) + + def _on_parametrized_fixture_setup(self, fixturedef: FixtureDef[Any]) -> None: + self._active_parametrized_fixturedefs.add(fixturedef) + + def _on_parametrized_fixture_finished(self, fixturedef: FixtureDef[Any]) -> None: + self._active_parametrized_fixturedefs.remove(fixturedef) + def show_fixtures_per_test(config: Config) -> int | ExitCode: from _pytest.main import wrap_session diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 6690f6ab1f8..81a744a4038 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -770,3 +770,6 @@ def location(self) -> tuple[str, int | None, str]: relfspath = self.session._node_location_to_relpath(path) assert type(location[2]) is str return (relfspath, location[1], location[2]) + + def _teardown_stale_fixtures(self) -> None: + pass diff --git a/src/_pytest/pytester.py b/src/_pytest/pytester.py index ac22d7a1f0c..1cd5f05dd7e 100644 --- a/src/_pytest/pytester.py +++ b/src/_pytest/pytester.py @@ -1040,7 +1040,6 @@ def genitems(self, colitems: Sequence[Item | Collector]) -> list[Item]: result: list[Item] = [] for colitem in colitems: result.extend(session.genitems(colitem)) - session.items.extend(result) return result def runitem(self, source: str) -> Any: diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 7374fa3cee0..96512450583 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1687,6 +1687,9 @@ def runtest(self) -> None: def setup(self) -> None: self._request._fillfixtures() + def _teardown_stale_fixtures(self) -> None: + self.session._fixturemanager._teardown_stale_fixtures(self) + def _traceback_filter(self, excinfo: ExceptionInfo[BaseException]) -> Traceback: if hasattr(self, "_obj") and not self.config.getoption("fulltrace", False): code = _pytest._code.Code.from_function(get_real_func(self.obj)) diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py index d1090aace89..ee5f118c806 100644 --- a/src/_pytest/runner.py +++ b/src/_pytest/runner.py @@ -558,6 +558,11 @@ def teardown_exact(self, nextitem: Item | None) -> None: fin() except TEST_OUTCOME as e: these_exceptions.append(e) + if isinstance(nextitem, Item): + try: + nextitem._teardown_stale_fixtures() + except BaseException as e: + exceptions.append(e) if len(these_exceptions) == 1: exceptions.extend(these_exceptions) From 917ff33d551c515c2bcbf44b1bcd96a3d2a523e3 Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Mon, 12 Jan 2026 17:42:08 +0300 Subject: [PATCH 05/24] Fix exception handling --- src/_pytest/fixtures.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index df57ca6f0e8..9d69b2d1885 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1993,8 +1993,17 @@ def _matchfactories( yield fixturedef def _teardown_stale_fixtures(self, item: nodes.Item) -> None: + exceptions: list[BaseException] = [] for fixturedef in list(self._active_parametrized_fixturedefs): - fixturedef._finish_if_param_changed(item) + try: + fixturedef._finish_if_param_changed(item) + except BaseException as e: + exceptions.append(e) + if len(exceptions) == 1: + raise exceptions[0] + elif len(exceptions) > 1: + msg = f'errors while tearing down fixtures for "{item.nodeid}"' + raise BaseExceptionGroup(msg, exceptions[::-1]) def _on_parametrized_fixture_setup(self, fixturedef: FixtureDef[Any]) -> None: self._active_parametrized_fixturedefs.add(fixturedef) From 455bdbbc2697492810f601b9413e3b4f52ec2c60 Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Thu, 15 Jan 2026 16:04:46 +0300 Subject: [PATCH 06/24] Rewrite everything, add more tests --- src/_pytest/fixtures.py | 67 +- src/_pytest/setuponly.py | 15 +- testing/python/fixture_dependencies.py | 973 +++++++++++++++++++++++++ testing/python/fixtures.py | 162 ---- 4 files changed, 1014 insertions(+), 203 deletions(-) create mode 100644 testing/python/fixture_dependencies.py diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 9d69b2d1885..babbbbcc8d1 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -389,6 +389,9 @@ def __init__( # - In the future we might consider using a generic for the param type, but # for now just using Any. self.param: Any + # FixtureDefs requested through this specific `request` object. + # Allows tracking dependencies on fixtures. + self._own_fixture_defs: Final[dict[str, FixtureDef[Any]]] = {} @property def _fixturemanager(self) -> FixtureManager: @@ -546,11 +549,6 @@ def _iter_chain(self) -> Iterator[SubRequest]: yield current current = current._parent_request - def _account_dependency(self, fixturedef: FixtureDef[object]) -> None: - if isinstance(self, SubRequest): - assert self._fixturedef._requested_fixtures is not None - self._fixturedef._requested_fixtures[fixturedef.argname] = fixturedef - def _get_active_fixturedef(self, argname: str) -> FixtureDef[object]: if argname == "request": return RequestFixtureDef(self) @@ -560,7 +558,7 @@ def _get_active_fixturedef(self, argname: str) -> FixtureDef[object]: fixturedef = self._fixture_defs.get(argname) if fixturedef is not None: self._check_scope(fixturedef, fixturedef._scope) - self._account_dependency(fixturedef) + self._own_fixture_defs[argname] = fixturedef return fixturedef fixturedef = self._find_fixturedef(argname) @@ -626,7 +624,7 @@ def _execute_fixturedef(self, fixturedef: FixtureDef[object]) -> None: self, scope, param, param_index, fixturedef, _ispytest=True ) - self._account_dependency(fixturedef) + self._own_fixture_defs[argname] = fixturedef # Make sure the fixture value is cached, running it if it isn't fixturedef.execute(request=subrequest) @@ -1038,11 +1036,8 @@ def __init__( # ID of the latest cache computation. Prevents finalizing a more recent # evaluation of the fixture than expected. self._cache_epoch: int = 0 - # SubRequest used during setup of the current result. - self._cached_request: SubRequest | None = None - # Tracks dependencies discovered during the current execution. - # Filled by FixtureRequest. - self._requested_fixtures: dict[str, FixtureDef[Any]] | None = None + # Callback to finalize cached_result. + self._self_finalizer: Callable[[], object] | None = None # only used to emit a deprecationwarning, can be removed in pytest9 self._autouse = _autouse @@ -1075,7 +1070,7 @@ def finish(self, request: SubRequest) -> None: self._finalizers.clear() self._cache_epoch += 1 request._fixturemanager._on_parametrized_fixture_finished(self) - self._cached_request = None + self._self_finalizer = None if len(exceptions) == 1: raise exceptions[0] elif len(exceptions) > 1: @@ -1088,12 +1083,12 @@ def execute(self, request: SubRequest) -> FixtureValue: self._check_cache_hit(request, self.cached_result[1]) return _get_cached_value(self.cached_result) - self._requested_fixtures = {} # Execute fixtures from argnames here to make sure that analytics # in pytest_fixture_setup only handle the body of the current fixture. for argname in self.argnames: request._get_active_fixturedef(argname) + self._self_finalizer = self._make_finalizer(request) ihook = request.node.ihook try: # Setup the fixture, run the code in it, and cache the value @@ -1102,16 +1097,11 @@ def execute(self, request: SubRequest) -> FixtureValue: fixturedef=self, request=request ) finally: - # Schedule our finalizer, even if the setup failed. - self._cached_request = request - finalizer = self._make_finalizer() request._fixturemanager._on_parametrized_fixture_setup(self) - request.node.addfinalizer(finalizer) - # Add finalizer to requested fixtures. - # We make sure to do this after checking for cached value to avoid - # adding our finalizer multiple times. (#12135) - for parent_fixture in self._requested_fixtures.values(): - parent_fixture.addfinalizer(finalizer) + # Schedule our finalizer, even if the setup failed. + request.node.addfinalizer(self._self_finalizer) + for fixturedef in request._own_fixture_defs.values(): + fixturedef.addfinalizer(self._self_finalizer) return result @@ -1125,30 +1115,38 @@ def _is_cache_hit(self, old_cache_key: object, new_cache_key: object) -> bool: cache_hit = new_cache_key is old_cache_key return cache_hit - def _make_finalizer(self) -> Callable[[], object]: + def _make_finalizer(self, request: SubRequest) -> Callable[[], object]: cache_epoch = self._cache_epoch def finalizer() -> None: if self._cache_epoch == cache_epoch: - assert self._cached_request is not None - self.finish(self._cached_request) + self.finish(request) return finalizer def _finish_if_param_changed(self, item: nodes.Item) -> None: if self.cached_result is None: return - assert self._cached_request is not None + assert self._self_finalizer is not None old_cache_key = self.cached_result[1] - new_cache_key = self._cache_key_internal(item) + + callspec: CallSpec2 | None = getattr(item, "callspec", None) + if callspec is None or self.argname not in callspec.params: + # Carry the fixture cache over a test that does not request + # the (previously) parametrized fixture statically. + # This implementation decision has the consequence that requesting + # the fixture dynamically is disallowed, see _check_cache_hit. + return + new_cache_key = callspec.params[self.argname] if not self._is_cache_hit(old_cache_key, new_cache_key): - self.finish(self._cached_request) + self._self_finalizer() def _check_cache_hit(self, request: SubRequest, old_cache_key: object) -> None: new_cache_key = self.cache_key(request) if self._is_cache_hit(old_cache_key, new_cache_key): return + # Finishing the fixture in setup phase in unacceptable (see PR #14104). item = request._pyfuncitem location = getlocation(self.func, item.config.rootpath) msg = ( @@ -1157,7 +1155,7 @@ def _check_cache_hit(self, request: SubRequest, old_cache_key: object) -> None: f"Requested fixture '{self.argname}' defined in:\n" f"{location}\n\n" f"Previous parameter value: {old_cache_key!r}\n" - f"New parameter value: {new_cache_key!r}\n\n" + f"New parameter value: {new_cache_key!r}\n\n" f"This could happen because the current test requested the previously " "parametrized fixture dynamically via 'getfixturevalue' and did not " "provide a parameter for the fixture.\n" @@ -1644,7 +1642,8 @@ def __init__(self, session: Session) -> None: self._nodeid_autousenames: Final[dict[str, list[str]]] = { "": self.config.getini("usefixtures"), } - self._active_parametrized_fixturedefs: Final[set[FixtureDef[Any]]] = set() + # Using dict for keeping insertion order. + self._active_parametrized_fixturedefs: Final[dict[FixtureDef[Any], None]] = {} session.config.pluginmanager.register(self, "funcmanage") def getfixtureinfo( @@ -1994,7 +1993,7 @@ def _matchfactories( def _teardown_stale_fixtures(self, item: nodes.Item) -> None: exceptions: list[BaseException] = [] - for fixturedef in list(self._active_parametrized_fixturedefs): + for fixturedef in reversed(list(self._active_parametrized_fixturedefs.keys())): try: fixturedef._finish_if_param_changed(item) except BaseException as e: @@ -2006,10 +2005,10 @@ def _teardown_stale_fixtures(self, item: nodes.Item) -> None: raise BaseExceptionGroup(msg, exceptions[::-1]) def _on_parametrized_fixture_setup(self, fixturedef: FixtureDef[Any]) -> None: - self._active_parametrized_fixturedefs.add(fixturedef) + self._active_parametrized_fixturedefs[fixturedef] = None def _on_parametrized_fixture_finished(self, fixturedef: FixtureDef[Any]) -> None: - self._active_parametrized_fixturedefs.remove(fixturedef) + del self._active_parametrized_fixturedefs[fixturedef] def show_fixtures_per_test(config: Config) -> int | ExitCode: diff --git a/src/_pytest/setuponly.py b/src/_pytest/setuponly.py index 7e6b46bcdb4..1aa04e14722 100644 --- a/src/_pytest/setuponly.py +++ b/src/_pytest/setuponly.py @@ -47,7 +47,7 @@ def pytest_fixture_setup( else: param = request.param fixturedef.cached_param = param # type: ignore[attr-defined] - _show_fixture_action(fixturedef, request.config, "SETUP") + _show_fixture_action(fixturedef, request, "SETUP") def pytest_fixture_post_finalizer( @@ -56,14 +56,15 @@ def pytest_fixture_post_finalizer( if fixturedef.cached_result is not None: config = request.config if config.option.setupshow: - _show_fixture_action(fixturedef, request.config, "TEARDOWN") + _show_fixture_action(fixturedef, request, "TEARDOWN") if hasattr(fixturedef, "cached_param"): del fixturedef.cached_param def _show_fixture_action( - fixturedef: FixtureDef[object], config: Config, msg: str + fixturedef: FixtureDef[object], request: SubRequest, msg: str ) -> None: + config = request.config capman = config.pluginmanager.getplugin("capturemanager") if capman: capman.suspend_global_capture() @@ -77,14 +78,14 @@ def _show_fixture_action( scopename = fixturedef.scope[0].upper() tw.write(f"{msg:<8} {scopename} {fixturedef.argname}") + if hasattr(fixturedef, "cached_param"): + tw.write(f"[{saferepr(fixturedef.cached_param, maxsize=42)}]") + if msg == "SETUP": - deps = sorted(arg for arg in fixturedef.argnames if arg != "request") + deps = sorted(request._own_fixture_defs.keys()) if deps: tw.write(" (fixtures used: {})".format(", ".join(deps))) - if hasattr(fixturedef, "cached_param"): - tw.write(f"[{saferepr(fixturedef.cached_param, maxsize=42)}]") - tw.flush() if capman: diff --git a/testing/python/fixture_dependencies.py b/testing/python/fixture_dependencies.py new file mode 100644 index 00000000000..4e6e78b6288 --- /dev/null +++ b/testing/python/fixture_dependencies.py @@ -0,0 +1,973 @@ +""" +Tests for fixture functionality covering setup-teardown ordering of fixtures, including +parametrized fixtures and dynamically requested fixtures. +""" + +from __future__ import annotations + +from _pytest.pytester import Pytester +import pytest + + +def test_getfixturevalue_parametrized_dependency_tracked(pytester: Pytester) -> None: + """ + Test that a fixture depending on a parametrized fixture via getfixturevalue + is properly recomputed when the parametrized fixture changes (#14103). + """ + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="session") + def foo(request): + return request.param + + @pytest.fixture(scope="session") + def bar(request): + return request.getfixturevalue("foo") + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_first(foo, bar): + assert bar == 1 + + @pytest.mark.parametrize("foo", [2], indirect=True) + def test_second(foo, bar): + assert bar == 2 + """ + ) + result = pytester.runpytest("-v", "--setup-show") + result.assert_outcomes(passed=2) + result.stdout.fnmatch_lines( + [ + "test_fixtures.py::test_first[1] ", + "SETUP S foo[1]", + "SETUP S bar (fixtures used: foo)", + " *test_first*", + "TEARDOWN S bar", + "TEARDOWN S foo[1]", + "test_fixtures.py::test_second[2] ", + "SETUP S foo[2]", + "SETUP S bar (fixtures used: foo)", + " *test_second*", + "TEARDOWN S bar", + "TEARDOWN S foo[2]", + ], + consecutive=True, + ) + + +@pytest.mark.xfail(reason="#14095") +def test_fixture_override_finishes_dependencies(pytester: Pytester) -> None: + """Test that a fixture gets recomputed if its dependency resolves to a different fixturedef (#14095).""" + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="session") + def foo(): + return "outer" + + @pytest.fixture(scope="session") + def bar(foo): + return f"dependent_{foo}" + + @pytest.fixture(scope="session") + def baz(bar): + return bar + + def test_before_class(baz): + assert baz == "dependent_outer" + + class TestOverride: + @pytest.fixture(scope="session") + def foo(self): + return "inner" + + def test_in_class(self, baz): + assert baz == "dependent_inner" + + def test_after_class(baz): + assert baz == "dependent_outer" + """ + ) + result = pytester.runpytest("-v", "--setup-show") + result.assert_outcomes(passed=3) + result.stdout.fnmatch_lines( + [ + "test_fixtures.py::test_before_class ", + "SETUP S foo", + "SETUP S bar (fixtures used: foo)", + "SETUP S baz (fixtures used: bar)", + " *test_before_class*", + "test_fixtures.py::TestOverride::test_in_class ", + # baz and bar are recomputed because foo resolves to a different fixturedef. + "TEARDOWN S baz", + "TEARDOWN S bar", + "SETUP S foo", # The inner foo. + "SETUP S bar (fixtures used: foo)", + "SETUP S baz (fixtures used: bar)", + " *test_in_class*", + "test_fixtures.py::test_after_class ", + # baz and bar are recomputed because foo resolves to a different fixturedef. + "TEARDOWN S baz", + "TEARDOWN S bar", + "SETUP S bar (fixtures used: foo)", + "SETUP S baz (fixtures used: bar)", + " *test_after_class*", + "TEARDOWN S baz", + "TEARDOWN S bar", + "TEARDOWN S foo", + "TEARDOWN S foo", + ], + consecutive=True, + ) + + +def test_override_fixture_with_new_parametrized_fixture(pytester: Pytester) -> None: + """Test what happens when a cached fixture is overridden by a new parametrized fixture, + and another fixture depends on it. + + This test verifies that: + 1. A fixture can be overridden by a parametrized fixture in a nested scope + 2. Dependent fixtures get recomputed because a dependency now resolves to a different fixturedef + 3. The outer fixture is not setup or finalized unnecessarily + """ + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="session") + def foo(request): + assert not hasattr(request, "param") + return "outer" + + @pytest.fixture(scope="session") + def bar(foo): + return f"dependent_{foo}" + + def test_before_class(foo, bar): + assert foo == "outer" + assert bar == "dependent_outer" + + class TestOverride: + @pytest.fixture(scope="session") + def foo(self, request): + return f"inner_{request.param}" + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_in_class(self, foo, bar): + assert foo == "inner_1" + assert bar == "dependent_inner_1" + """ + ) + result = pytester.runpytest("-v", "--setup-show") + result.assert_outcomes(passed=2) + result.stdout.fnmatch_lines( + [ + "test_fixtures.py::test_before_class ", + "SETUP S foo", + "SETUP S bar (fixtures used: foo)", + " test_fixtures.py::test_before_class (fixtures used: bar, foo, request)PASSED", + "TEARDOWN S bar", + "TEARDOWN S foo", + "test_fixtures.py::TestOverride::test_in_class[1] ", + "SETUP S foo[1]", + "SETUP S bar (fixtures used: foo)", + " test_fixtures.py::TestOverride::test_in_class[1] (fixtures used: bar, foo, request)PASSED", + "TEARDOWN S bar", + "TEARDOWN S foo[1]", + ], + consecutive=True, + ) + + +def test_fixture_post_finalizer_hook_exception(pytester: Pytester) -> None: + """Test that exceptions in pytest_fixture_post_finalizer hook are caught. + + Covers line 1069: node.ihook.pytest_fixture_post_finalizer(fixturedef=self, request=request) + + Also verifies that the fixture cache is properly reset even when the + post_finalizer hook raises an exception, so the fixture can be rebuilt + in subsequent tests. + """ + pytester.makeconftest( + """ + import pytest + + def pytest_fixture_post_finalizer(fixturedef, request): + if "test_first" in request.node.nodeid: + raise RuntimeError("Error in post finalizer hook") + + @pytest.fixture + def my_fixture(): + yield "value" + """ + ) + pytester.makepyfile( + test_fixtures=""" + def test_first(my_fixture): + assert my_fixture == "value" + + def test_second(my_fixture): + assert my_fixture == "value" + """ + ) + result = pytester.runpytest("-v", "--setup-show") + result.assert_outcomes(passed=2, errors=1) + result.stdout.fnmatch_lines( + [ + "*test_first*PASSED", + "*test_first*ERROR", + "*RuntimeError: Error in post finalizer hook*", + ] + ) + # Verify fixture is setup twice (rebuilt for test_second despite error). + result.stdout.fnmatch_lines( + [ + "test_fixtures.py::test_first ", + " SETUP F my_fixture", + " test_fixtures.py::test_first (fixtures used: my_fixture)PASSED", + "test_fixtures.py::test_first ERROR", + "test_fixtures.py::test_second ", + " SETUP F my_fixture", + " test_fixtures.py::test_second (fixtures used: my_fixture)PASSED", + " TEARDOWN F my_fixture", + ], + consecutive=True, + ) + + +def test_fixture_not_rebuilt_when_not_requested(pytester: Pytester) -> None: + """Test that fixtures are NOT rebuilt when not requested in an intermediate test. + + This is a control test showing that when test_b doesn't access foo at all, + the fixture remains cached and is not torn down/rebuilt. + + Scenario: + 1. test_a: fixture 'foo' is parametrized with value 1 + 2. test_b: does NOT request fixture 'foo' + 3. test_c: fixture 'foo' is parametrized with value 1 (same as test_a) + + We disable test reordering to ensure tests run in the defined order. + """ + pytester.makeconftest( + """ + import pytest + + @pytest.hookimpl(wrapper=True, tryfirst=True) + def pytest_collection_modifyitems(items): + original_items = items[:] + yield + items[:] = original_items + """ + ) + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="session") + def foo(request): + return request.param + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_a(foo): + assert foo == 1 + + def test_b(): + pass + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_c(foo): + assert foo == 1 + """ + ) + result = pytester.runpytest("-v", "--setup-show") + result.assert_outcomes(passed=3) + # Verify fixture is setup only once and carries over test_b. + result.stdout.fnmatch_lines( + [ + "test_fixtures.py::test_a[1] ", + "SETUP S foo[1]", + " test_fixtures.py::test_a[1] (fixtures used: foo, request)PASSED", + "test_fixtures.py::test_b ", + " test_fixtures.py::test_bPASSED", + "test_fixtures.py::test_c[1] ", + " test_fixtures.py::test_c[1] (fixtures used: foo, request)PASSED", + "TEARDOWN S foo[1]", + ], + consecutive=True, + ) + + +def test_cache_mismatch_error_on_sudden_getfixturevalue(pytester: Pytester) -> None: + """Test cache key mismatch when accessing parametrized fixture via getfixturevalue. + + This test demonstrates that accessing a previously parametrized fixture via + getfixturevalue without providing a parameter causes a cache key mismatch error. + + Scenario: + 1. test_a: fixture 'foo' is parametrized with value 1 + 2. test_b: fixture 'foo' is accessed via getfixturevalue without parameter + 3. test_c: fixture 'foo' is parametrized again with value 1 + + We disable test reordering to ensure tests run in the defined order. + """ + pytester.makeconftest( + """ + import pytest + + @pytest.hookimpl(wrapper=True, tryfirst=True) + def pytest_collection_modifyitems(items): + original_items = items[:] + yield + items[:] = original_items + """ + ) + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="session") + def foo(request): + return getattr(request, 'param', 'default') + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_a(foo): + assert foo == 1 + + def test_b(request): + value = request.getfixturevalue("foo") + assert value is not None + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_c(foo): + assert foo == 1 + """ + ) + result = pytester.runpytest("-v", "--setup-show") + result.assert_outcomes(passed=2, failed=1) + result.stdout.fnmatch_lines( + [ + "*Parameter for the requested fixture changed unexpectedly*", + "*test_b*", + "*Requested fixture 'foo'*", + "*Previous parameter value: 1", + "*New parameter value: None", + ] + ) + # Verify fixture is NOT torn down during test_b failure. + result.stdout.fnmatch_lines( + [ + "SETUP S foo[1]", + "*test_a*PASSED", + "*test_b*", + "*test_b*FAILED", + "*test_c*", + "*test_c*PASSED", + "TEARDOWN S foo[1]", + ], + consecutive=True, + ) + + +def test_cache_key_mismatch_error_on_unexpected_param_change( + pytester: Pytester, +) -> None: + """Test what happens when param changes unexpectedly, forcing a parametrized + fixture to teardown during setup phase. In this case, the test that changed its + parameter unexpectedly fails. + """ + pytester.makeconftest( + """ + import pytest + + @pytest.hookimpl(wrapper=True, tryfirst=True) + def pytest_collection_modifyitems(items): + # Disable built-in parametrized test reordering. + original_items = items[:] + yield + items[:] = original_items + + @pytest.hookimpl(wrapper=True, tryfirst=True) + def pytest_runtest_protocol(item, nextitem): + # Manipulate callspec for test_b to cause unexpected param change. + if item.name == "test_b[1]": + # Change the parameter value after teardown check but before setup. + item.callspec.params["foo"] = 999 + yield + """ + ) + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="session") + def foo(request): + return request.param + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_a(foo): + assert foo == 1 + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_b(foo): + assert foo == 1 + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_c(foo): + assert foo == 1 + """ + ) + result = pytester.runpytest("-v", "--setup-show") + result.assert_outcomes(passed=2, errors=1) + result.stdout.fnmatch_lines( + [ + "*Parameter for the requested fixture changed unexpectedly*", + "*test_b*", + "*Requested fixture 'foo'*", + "*Previous parameter value: 1", + "*New parameter value: 999", + ] + ) + result.stdout.fnmatch_lines( + [ + "test_fixtures.py::test_a[1] ", + "SETUP S foo[1]", + " *test_a*PASSED", + "test_fixtures.py::test_b[1] ERROR", + "test_fixtures.py::test_c[1] ", + " *test_c*PASSED", + "TEARDOWN S foo[1]", + ], + consecutive=True, + ) + + +def test_teardown_stale_fixtures_single_exception(pytester: Pytester) -> None: + """Test that a single exception during stale fixture teardown is propagated.""" + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="session") + def failing_fixture(request): + yield request.param + raise RuntimeError("Teardown error") + + @pytest.mark.parametrize("failing_fixture", [1], indirect=True) + def test_first(failing_fixture): + assert failing_fixture == 1 + + @pytest.mark.parametrize("failing_fixture", [2], indirect=True) + def test_second(failing_fixture): + assert failing_fixture == 2 + """ + ) + result = pytester.runpytest("-v", "--setup-show") + result.assert_outcomes(passed=2, errors=2) + result.stdout.fnmatch_lines( + [ + "*RuntimeError: Teardown error*", + "*RuntimeError: Teardown error*", + ] + ) + result.stdout.fnmatch_lines( + [ + "test_fixtures.py::test_first[1] ", + "SETUP S failing_fixture[1]", + " *test_first*PASSED", + "TEARDOWN S failing_fixture[1]", + "*test_first*ERROR", + "test_fixtures.py::*test_second* ", + "SETUP S failing_fixture[2]", + " *test_second*PASSED", + "TEARDOWN S failing_fixture[2]", + "*test_second*ERROR", + ], + consecutive=True, + ) + + +def test_teardown_stale_fixtures_multiple_exceptions(pytester: Pytester) -> None: + """Test that multiple exceptions during stale fixture teardown are grouped.""" + pytester.makepyfile( + """ + import pytest + + @pytest.fixture(scope="session") + def fixture_a(request): + yield request.param + raise RuntimeError("Error in fixture_a teardown") + + @pytest.fixture(scope="session") + def fixture_b(request): + yield request.param + raise ValueError("Error in fixture_b teardown") + + @pytest.mark.parametrize("fixture_a,fixture_b", [(1, 1)], indirect=True) + def test_first(fixture_a, fixture_b): + assert fixture_a == 1 + assert fixture_b == 1 + + @pytest.mark.parametrize("fixture_a,fixture_b", [(2, 2)], indirect=True) + def test_second(fixture_a, fixture_b): + assert fixture_a == 2 + assert fixture_b == 2 + """ + ) + result = pytester.runpytest() + result.assert_outcomes(passed=2, errors=2) + # Should have errors with BaseExceptionGroup containing both exceptions. + result.stdout.fnmatch_lines( + [ + "*ExceptionGroup: errors while tearing down fixtures*", + "*RuntimeError: Error in fixture_a teardown*", + "*ValueError: Error in fixture_b teardown*", + ] + ) + + +def test_request_stealing_then_getfixturevalue_on_parametrized( + pytester: Pytester, +) -> None: + """Golden test for the behavior of fixture dependency tracking when a fixture + steals another fixture's request. + + This test demonstrates the behavior when: + 1. A session-scoped fixture returns its request object + 2. Another session-scoped fixture uses that request to call getfixturevalue + 3. The requested fixture is parametrized + + The current behavior is to allow this but skip fixture dependency tracking + for when request object is used after its fixture setup completes. + This could be detected and disallowed in the future. + """ + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="session") + def param_fixture(request): + return request.param + + @pytest.fixture(scope="session") + def request_provider(request): + return request + + @pytest.fixture(scope="session") + def dependent(request_provider): + return request_provider.getfixturevalue("param_fixture") + + @pytest.mark.parametrize("param_fixture", [1], indirect=True) + def test_first(dependent, param_fixture): + assert param_fixture == 1 + assert dependent == 1 + + @pytest.mark.parametrize("param_fixture", [2], indirect=True) + def test_second(dependent, param_fixture): + assert param_fixture == 2 + # Dependency of dependent on param_fixture is not tracked. + assert dependent == 1 + """ + ) + result = pytester.runpytest("-v", "--setup-show") + result.assert_outcomes(passed=2) + result.stdout.fnmatch_lines( + [ + "test_fixtures.py::test_first[1] ", + "SETUP S request_provider", + "SETUP S param_fixture[1]", + "SETUP S dependent (fixtures used: request_provider)", + " *test_first*", + # Dependency of dependent on param_fixture is not tracked. + "TEARDOWN S param_fixture[1]", + "test_fixtures.py::test_second[2] ", + "SETUP S param_fixture[2]", + " *test_second*", + "TEARDOWN S param_fixture[2]", + "TEARDOWN S dependent", + "TEARDOWN S request_provider", + ], + consecutive=True, + ) + + +def test_cache_epoch_prevents_stale_finalizer(pytester: Pytester) -> None: + """Test that _cache_epoch prevents finalization by stale finalizers. + + This test demonstrates the problem that _cache_epoch solves: + + Scenario: + 1. Fixture 'bar' depends on 'foo' via getfixturevalue in first evaluation + 2. Fixture 'bar' gets recomputed and no longer depends on 'foo' + 3. Fixture 'foo' gets finalized + 4. Without _cache_epoch, 'bar' would be finalized by the old finalizer + registered during step 1, even though it has been recomputed + 5. With _cache_epoch, the old finalizer checks the epoch and skips finalization + + The test verifies that 'bar' is NOT finalized when 'foo' is finalized, + because 'bar' was recomputed and the old finalizer should be ignored. + """ + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="session") + def foo(request): + return request.param + + @pytest.fixture(scope="session") + def bar(request): + if request.param == 1: + return request.getfixturevalue("foo") + return "independent" + + @pytest.mark.parametrize("foo,bar", [(1, 1)], indirect=True) + def test_first(foo, bar): + assert foo == 1 + assert bar == 1 + + @pytest.mark.parametrize("foo,bar", [(1, 2)], indirect=True) + def test_second(foo, bar): + assert foo == 1 + assert bar == "independent" + + @pytest.mark.parametrize("foo,bar", [(2, 2)], indirect=True) + def test_third(foo, bar): + assert foo == 2 + assert bar == "independent" + """ + ) + result = pytester.runpytest("-v", "--setup-show") + result.assert_outcomes(passed=3) + result.stdout.fnmatch_lines( + [ + "test_fixtures.py::test_first[1-1] ", + "SETUP S foo[1]", + "SETUP S bar[1] (fixtures used: foo)", + " *test_first*PASSED", + "TEARDOWN S bar[1]", + "test_fixtures.py::test_second[1-2] ", + "SETUP S bar[2]", + " *test_second*PASSED", + "TEARDOWN S foo[1]", + # bar should NOT be torn down here because the old finalizer + # from the first evaluation should be ignored due to _cache_epoch. + "test_fixtures.py::test_third[2-2] ", + "SETUP S foo[2]", + " *test_third*PASSED", + "TEARDOWN S foo[2]", + "TEARDOWN S bar[2]", + ], + consecutive=True, + ) + + +def test_fixture_teardown_when_not_requested_but_param_changes( + pytester: Pytester, +) -> None: + """Test when a parametrized fixture gets torn down when not requested by intermediate test. + + This test demonstrates a surprising but necessary behavior: + When a parametrized fixture is not requested by an intermediate test, and + the next test requests it with a different parameter, the fixture gets torn down + at the end of the intermediate test. + """ + pytester.makeconftest( + """ + import pytest + + @pytest.hookimpl(wrapper=True, tryfirst=True) + def pytest_collection_modifyitems(items): + # Disable built-in parametrized test reordering. + original_items = items[:] + yield + items[:] = original_items + """ + ) + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="session") + def foo(request): + return request.param + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_a(foo): + assert foo == 1 + + def test_b(): + pass + + @pytest.mark.parametrize("foo", [2], indirect=True) + def test_c(foo): + assert foo == 2 + """ + ) + result = pytester.runpytest("-v", "--setup-show") + result.assert_outcomes(passed=3) + result.stdout.fnmatch_lines( + [ + "test_fixtures.py::test_a[1] ", + "SETUP S foo[1]", + " *test_a*PASSED", + "test_fixtures.py::test_b ", + " *test_b*PASSED", + # foo[1] is torn down here, at the end of test_b. + "TEARDOWN S foo[1]", + "test_fixtures.py::test_c[2] ", + "SETUP S foo[2]", + " *test_c*PASSED", + "TEARDOWN S foo[2]", + ], + consecutive=True, + ) + + +def test_fixture_teardown_with_reordered_tests(pytester: Pytester) -> None: + """Test that fixture teardown works correctly when tests are reordered at runtime. + + This test verifies that the fixture teardown mechanism doesn't hard rely on the + ordering of tests from the collection stage. Tests are collected in one order + but executed in a different order via a custom pytest_runtestloop hook. + + Collection order: test_a, test_b, test_c + Execution order: test_a, test_c, test_b + """ + pytester.makeconftest( + """ + import pytest + + @pytest.hookimpl(wrapper=True, tryfirst=True) + def pytest_collection_modifyitems(items): + # Disable built-in parametrized test reordering. + original_items = items[:] + yield + items[:] = original_items + + @pytest.hookimpl(tryfirst=True) + def pytest_runtestloop(session): + # Reorder tests at runtime: test_a, test_c, test_b. + items = list(session.items) + assert len(items) == 3 + # Swap test_b and test_c. + items[1], items[2] = items[2], items[1] + for i, item in enumerate(items): + nextitem = items[i + 1] if i + 1 < len(items) else None + item.config.hook.pytest_runtest_protocol(item=item, nextitem=nextitem) + return True + """ + ) + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="session") + def foo(request): + return request.param + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_a(foo): + assert foo == 1 + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_b(foo): + assert foo == 1 + + @pytest.mark.parametrize("foo", [2], indirect=True) + def test_c(foo): + assert foo == 2 + """ + ) + result = pytester.runpytest("-v", "--setup-show") + result.assert_outcomes(passed=3) + result.stdout.fnmatch_lines( + [ + "test_fixtures.py::test_a[1] ", + "SETUP S foo[1]", + " *test_a*PASSED", + # foo[1] is torn down here because test_c needs foo[2]. + "TEARDOWN S foo[1]", + "test_fixtures.py::test_c[2] ", + "SETUP S foo[2]", + " *test_c*PASSED", + "TEARDOWN S foo[2]", + "test_fixtures.py::test_b[1] ", + "SETUP S foo[1]", + " *test_b*PASSED", + "TEARDOWN S foo[1]", + ], + consecutive=True, + ) + + +def test_fixture_post_finalizer_called_once(pytester: Pytester) -> None: + """Test that pytest_fixture_post_finalizer is called only once per fixture teardown. + + When a fixture depends on multiple parametrized fixtures and all their parameters + change at the same time, the dependent fixture should be torn down only once, + and pytest_fixture_post_finalizer should be called only once for it. + """ + pytester.makeconftest( + """ + import pytest + + finalizer_calls = [] + + def pytest_fixture_post_finalizer(fixturedef, request): + finalizer_calls.append(fixturedef.argname) + + @pytest.fixture(autouse=True) + def check_finalizer_calls(request): + yield + # After each test, verify no duplicate finalizer calls. + if finalizer_calls: + assert len(finalizer_calls) == len(set(finalizer_calls)), ( + f"Duplicate finalizer calls detected: {finalizer_calls}" + ) + finalizer_calls.clear() + """ + ) + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="session") + def foo(request): + return request.param + + @pytest.fixture(scope="session") + def bar(request): + return request.param + + @pytest.fixture(scope="session") + def baz(foo, bar): + return f"{foo}-{bar}" + + @pytest.mark.parametrize("foo,bar", [(1, 1)], indirect=True) + def test_first(foo, bar, baz): + assert foo == 1 + assert bar == 1 + assert baz == "1-1" + + @pytest.mark.parametrize("foo,bar", [(2, 2)], indirect=True) + def test_second(foo, bar, baz): + assert foo == 2 + assert bar == 2 + assert baz == "2-2" + """ + ) + result = pytester.runpytest("-v") + # The test passes, which means no duplicate finalizer calls were detected + # by the check_finalizer_calls autouse fixture. + result.assert_outcomes(passed=2) + + +def test_parametrized_fixtures_teardown_in_reverse_setup_order( + pytester: Pytester, +) -> None: + """Test that when multiple parametrized fixtures change at the same time, they are torn down in reverse setup order. + + When two parametrized fixtures both change their parameters between tests, + they should be torn down in the reverse order of their setup. + """ + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="session") + def foo(request): + return request.param + + @pytest.fixture(scope="session") + def bar(request): + return request.param + + @pytest.mark.parametrize("foo,bar", [(1, 1)], indirect=True) + def test_first(foo, bar): + assert foo == 1 + assert bar == 1 + + @pytest.mark.parametrize("foo,bar", [(2, 2)], indirect=True) + def test_second(foo, bar): + assert foo == 2 + assert bar == 2 + """ + ) + result = pytester.runpytest("-v", "--setup-show") + result.assert_outcomes(passed=2) + result.stdout.fnmatch_lines( + [ + "test_fixtures.py::test_first[1-1] ", + "SETUP S foo[1]", + "SETUP S bar[1]", + " *test_first*PASSED", + # Teardown in reverse setup order: bar first, then foo. + "TEARDOWN S bar[1]", + "TEARDOWN S foo[1]", + "test_fixtures.py::test_second[2-2] ", + "SETUP S foo[2]", + "SETUP S bar[2]", + " *test_second*PASSED", + "TEARDOWN S bar[2]", + "TEARDOWN S foo[2]", + ], + consecutive=True, + ) + + +def test_parametrized_fixture_teardown_order_with_mixed_scopes( + pytester: Pytester, +) -> None: + """Test teardown order when parametrized fixtures are mixed with regular fixtures. + + When a function-scoped, parametrized session-scoped, and class-scoped fixtures + are all torn down at the same time, parametrized fixtures are considered to be + between function-scoped and class-scoped in teardown order. + """ + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="function") + def func_fixture(): + yield "func" + + @pytest.fixture(scope="session") + def session_param_fixture(request): + yield request.param + + @pytest.fixture(scope="class") + def class_fixture(): + yield "class" + + class TestClass: + @pytest.mark.parametrize("session_param_fixture", [1], indirect=True) + def test_first(self, func_fixture, session_param_fixture, class_fixture): + pass + + @pytest.mark.parametrize("session_param_fixture", [2], indirect=True) + def test_second(session_param_fixture): + pass + """ + ) + result = pytester.runpytest("-v", "--setup-plan") + result.stdout.fnmatch_lines( + [ + "test_fixtures.py::TestClass::test_first[1] ", + "SETUP S session_param_fixture[1]", + " SETUP C class_fixture", + " SETUP F func_fixture", + " *test_first*", + " TEARDOWN F func_fixture", + # Parametrized fixture torn down between function and class fixtures. + "TEARDOWN S session_param_fixture[1]", + " TEARDOWN C class_fixture", + "test_fixtures.py::test_second[2] ", + "SETUP S session_param_fixture[2]", + " *test_second*", + "TEARDOWN S session_param_fixture[2]", + ], + consecutive=True, + ) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 0c85d8d4c47..8b9e3fbb0a5 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -5399,165 +5399,3 @@ def test_it(request, fix1): ) result = pytester.runpytest("-v") result.assert_outcomes(passed=1) - - -def test_getfixturevalue_parametrized_dependency_tracked(pytester: Pytester) -> None: - """ - Test that a fixture depending on a parametrized fixture via getfixturevalue - is properly recomputed when the parametrized fixture changes (#14103). - """ - pytester.makepyfile( - test_fixtures=""" - import pytest - - @pytest.fixture(scope="session") - def foo(request): - return request.param - - @pytest.fixture(scope="session") - def bar(request): - return request.getfixturevalue("foo") - - @pytest.mark.parametrize("foo", [1], indirect=True) - def test_first(foo, bar): - assert bar == 1 - - @pytest.mark.parametrize("foo", [2], indirect=True) - def test_second(foo, bar): - assert bar == 2 - """ - ) - result = pytester.runpytest("--setup-show", "-vv") - result.assert_outcomes(passed=2) - result.stdout.fnmatch_lines( - [ - "test_fixtures.py::test_first[1] ", - "SETUP S foo[1]", - "SETUP S bar", - " test_fixtures.py::test_first[1] (fixtures used: bar, foo, request)PASSED", - "TEARDOWN S bar", - "TEARDOWN S foo[1]", - "test_fixtures.py::test_second[2] ", - "SETUP S foo[2]", - "SETUP S bar", - " test_fixtures.py::test_second[2] (fixtures used: bar, foo, request)PASSED", - "TEARDOWN S bar", - "TEARDOWN S foo[2]", - ], - consecutive=True, - ) - - -@pytest.mark.xfail(reason="#14095") -def test_fixture_override_finishes_dependencies(pytester: Pytester) -> None: - """Test that a fixture gets recomputed if its dependency resolves to a different fixturedef (#14095).""" - pytester.makepyfile( - test_fixtures=""" - import pytest - - @pytest.fixture(scope="session") - def foo(): - return "outer" - - @pytest.fixture(scope="session") - def bar(foo): - return f"dependent_{foo}" - - def test_before_class(bar): - assert bar == "dependent_outer" - - class TestOverride: - @pytest.fixture(scope="session") - def foo(self): - return "inner" - - def test_in_class(self, bar): - assert bar == "dependent_inner" - - def test_after_class(bar): - assert bar == "dependent_outer" - """ - ) - result = pytester.runpytest("--setup-show", "-vv") - result.assert_outcomes(passed=3) - result.stdout.fnmatch_lines( - [ - "test_fixtures.py::test_before_class ", - "SETUP S foo", - "SETUP S bar (fixtures used: foo)", - " test_fixtures.py::test_before_class (fixtures used: bar, foo)PASSED", - "test_fixtures.py::TestOverride::test_in_class ", - # bar is recomputed because foo resolves to a different fixturedef. - "TEARDOWN S bar", - "SETUP S foo", # The inner foo. - "SETUP S bar (fixtures used: foo)", - " test_fixtures.py::TestOverride::test_in_class (fixtures used: bar, foo)PASSED", - "test_fixtures.py::test_after_class ", - # bar is recomputed because foo resolves to a different fixturedef. - "TEARDOWN S bar", - "SETUP S bar (fixtures used: foo)", - " test_fixtures.py::test_after_class (fixtures used: bar, foo)PASSED", - "TEARDOWN S bar", - "TEARDOWN S foo", - "TEARDOWN S foo", - ], - consecutive=True, - ) - - -def test_override_fixture_with_new_parametrized_fixture(pytester: Pytester) -> None: - """Test what happens when a cached fixture is overridden by a new parametrized fixture, - and another fixture depends on it. - - This test verifies that: - 1. A fixture can be overridden by a parametrized fixture in a nested scope - 2. Dependent fixtures get recomputed because a dependency now resolves to a different fixturedef - 3. The outer fixture is not setup or finalized unnecessarily - """ - pytester.makepyfile( - test_fixtures=""" - import pytest - - @pytest.fixture(scope="session") - def foo(request): - assert not hasattr(request, "param") - return "outer" - - @pytest.fixture(scope="session") - def bar(foo): - return f"dependent_{foo}" - - def test_before_class(foo, bar): - assert foo == "outer" - assert bar == "dependent_outer" - - class TestOverride: - @pytest.fixture(scope="session") - def foo(self, request): - return f"inner_{request.param}" - - @pytest.mark.parametrize("foo", [1], indirect=True) - def test_in_class(self, foo, bar): - assert foo == "inner_1" - assert bar == "dependent_inner_1" - """ - ) - result = pytester.runpytest("--setup-show", "-vv") - result.assert_outcomes(passed=2) - result.stdout.fnmatch_lines( - [ - "test_fixtures.py::test_before_class ", - "SETUP S foo", - "SETUP S bar (fixtures used: foo)", - " test_fixtures.py::test_before_class (fixtures used: bar, foo, request)PASSED", - "TEARDOWN S bar", - "TEARDOWN S foo", - "test_fixtures.py::TestOverride::test_in_class[1] ", - "SETUP S foo[1]", - "SETUP S bar (fixtures used: foo)", - " test_fixtures.py::TestOverride::test_in_class[1] (fixtures used: bar, foo, request)PASSED", - "TEARDOWN S bar", - "TEARDOWN S foo[1]", - ], - consecutive=True, - ) From e7c31fa0b40f0dada2ce432d1bf9b832c262098b Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Thu, 15 Jan 2026 16:20:04 +0300 Subject: [PATCH 07/24] Add changelogs --- changelog/2043.bugfix.rst | 1 + changelog/4871.improvement.rst | 1 + 2 files changed, 2 insertions(+) create mode 100644 changelog/2043.bugfix.rst create mode 100644 changelog/4871.improvement.rst diff --git a/changelog/2043.bugfix.rst b/changelog/2043.bugfix.rst new file mode 100644 index 00000000000..416325b4eed --- /dev/null +++ b/changelog/2043.bugfix.rst @@ -0,0 +1 @@ +If a fixture depends on a fixture that becomes hidden in a test (compared to the previous test), the hidden fixture definition is no longer executed. diff --git a/changelog/4871.improvement.rst b/changelog/4871.improvement.rst new file mode 100644 index 00000000000..320cfe45377 --- /dev/null +++ b/changelog/4871.improvement.rst @@ -0,0 +1 @@ +``pytest_fixture_post_finalizer`` is no longer called extra times for the same fixture teardown. From e9df3c1d7439daeead0a721733cf78b10fd92c8d Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Thu, 15 Jan 2026 16:22:34 +0300 Subject: [PATCH 08/24] Fix coverage --- src/_pytest/fixtures.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index babbbbcc8d1..404b9a5a286 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1125,8 +1125,7 @@ def finalizer() -> None: return finalizer def _finish_if_param_changed(self, item: nodes.Item) -> None: - if self.cached_result is None: - return + assert self.cached_result is not None assert self._self_finalizer is not None old_cache_key = self.cached_result[1] @@ -1168,12 +1167,6 @@ def _check_cache_hit(self, request: SubRequest, old_cache_key: object) -> None: def cache_key(self, request: SubRequest) -> object: return getattr(request, "param", None) - def _cache_key_internal(self, item: nodes.Item) -> object: - callspec: CallSpec2 | None = getattr(item, "callspec", None) - if callspec is not None and self.argname in callspec.params: - return callspec.params[self.argname] - return None - def __repr__(self) -> str: return f"" From 5aef1d5f719e236cd3cd390e60d6850d76429488 Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Thu, 15 Jan 2026 16:58:14 +0300 Subject: [PATCH 09/24] Fix changelogs --- changelog/14114.bugfix.rst | 1 + .../{4871.improvement.rst => 5848.improvement.rst} | 0 testing/python/fixture_dependencies.py | 12 ++++++------ 3 files changed, 7 insertions(+), 6 deletions(-) create mode 100644 changelog/14114.bugfix.rst rename changelog/{4871.improvement.rst => 5848.improvement.rst} (100%) diff --git a/changelog/14114.bugfix.rst b/changelog/14114.bugfix.rst new file mode 100644 index 00000000000..27ed1cfd9c5 --- /dev/null +++ b/changelog/14114.bugfix.rst @@ -0,0 +1 @@ +An exception from ``pytest_fixture_post_finalizer`` no longer prevents fixtures from being torn down, causing additional errors in the following tests. diff --git a/changelog/4871.improvement.rst b/changelog/5848.improvement.rst similarity index 100% rename from changelog/4871.improvement.rst rename to changelog/5848.improvement.rst diff --git a/testing/python/fixture_dependencies.py b/testing/python/fixture_dependencies.py index 4e6e78b6288..e0fd376ac6e 100644 --- a/testing/python/fixture_dependencies.py +++ b/testing/python/fixture_dependencies.py @@ -199,17 +199,17 @@ def pytest_fixture_post_finalizer(fixturedef, request): raise RuntimeError("Error in post finalizer hook") @pytest.fixture - def my_fixture(): - yield "value" + def my_fixture(request): + yield request.node.nodeid """ ) pytester.makepyfile( test_fixtures=""" def test_first(my_fixture): - assert my_fixture == "value" + assert "test_first" in my_fixture def test_second(my_fixture): - assert my_fixture == "value" + assert "test_second" in my_fixture """ ) result = pytester.runpytest("-v", "--setup-show") @@ -226,11 +226,11 @@ def test_second(my_fixture): [ "test_fixtures.py::test_first ", " SETUP F my_fixture", - " test_fixtures.py::test_first (fixtures used: my_fixture)PASSED", + " test_fixtures.py::test_first (fixtures used: my_fixture, request)PASSED", "test_fixtures.py::test_first ERROR", "test_fixtures.py::test_second ", " SETUP F my_fixture", - " test_fixtures.py::test_second (fixtures used: my_fixture)PASSED", + " test_fixtures.py::test_second (fixtures used: my_fixture, request)PASSED", " TEARDOWN F my_fixture", ], consecutive=True, From 54a3a358d21163a941daa6ae2853a522b26bacbd Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Thu, 15 Jan 2026 21:01:41 +0300 Subject: [PATCH 10/24] Remove fixture finalizers as they become stale --- changelog/4871.improvement.rst | 3 +++ changelog/9287.bugfix.rst | 3 +++ src/_pytest/fixtures.py | 37 ++++++++++++-------------- src/_pytest/nodes.py | 25 +++++++++++++++-- src/_pytest/runner.py | 18 +++++++++---- testing/python/fixture_dependencies.py | 17 +++++------- 6 files changed, 66 insertions(+), 37 deletions(-) create mode 100644 changelog/4871.improvement.rst create mode 100644 changelog/9287.bugfix.rst diff --git a/changelog/4871.improvement.rst b/changelog/4871.improvement.rst new file mode 100644 index 00000000000..9d90ebbbd83 --- /dev/null +++ b/changelog/4871.improvement.rst @@ -0,0 +1,3 @@ +Garbage finalizers for fixture teardown are no longer accumulated in nodes and fixtures. + +As a side-effect, :func:`Node.addfinalizer <_pytest.nodes.Node.addfinalizer>` now returns a handle that allows to remove the finalizer. diff --git a/changelog/9287.bugfix.rst b/changelog/9287.bugfix.rst new file mode 100644 index 00000000000..8e00a104683 --- /dev/null +++ b/changelog/9287.bugfix.rst @@ -0,0 +1,3 @@ +Teardown of parametrized fixtures now happens in the teardown stage of the test before the parameter changes. + +Previously teardown would happen in the setup stage of the test where the parameter changes. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 404b9a5a286..adb2f9dbe80 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1032,12 +1032,11 @@ def __init__( # If the fixture was executed, the current value of the fixture. # Can change if the fixture is executed with different parameters. self.cached_result: _FixtureCachedResult[FixtureValue] | None = None - self._finalizers: Final[list[Callable[[], object]]] = [] - # ID of the latest cache computation. Prevents finalizing a more recent - # evaluation of the fixture than expected. - self._cache_epoch: int = 0 + self._finalizers: Final[nodes.FinalizerStorage] = {} # Callback to finalize cached_result. self._self_finalizer: Callable[[], object] | None = None + # Handles to remove _self_finalizer from various scopes. + self._self_finalizer_handles: Final[list[Callable[[], None]]] = [] # only used to emit a deprecationwarning, can be removed in pytest9 self._autouse = _autouse @@ -1047,13 +1046,13 @@ def scope(self) -> _ScopeName: """Scope string, one of "function", "class", "module", "package", "session".""" return self._scope.value - def addfinalizer(self, finalizer: Callable[[], object]) -> None: - self._finalizers.append(finalizer) + def addfinalizer(self, finalizer: Callable[[], object]) -> Callable[[], None]: + return nodes.append_finalizer(self._finalizers, finalizer) def finish(self, request: SubRequest) -> None: exceptions: list[BaseException] = [] while self._finalizers: - fin = self._finalizers.pop() + _, fin = self._finalizers.popitem() try: fin() except BaseException as e: @@ -1068,9 +1067,12 @@ def finish(self, request: SubRequest) -> None: # which will keep instances alive. self.cached_result = None self._finalizers.clear() - self._cache_epoch += 1 request._fixturemanager._on_parametrized_fixture_finished(self) self._self_finalizer = None + # Avoid accumulating garbage finalizers in nodes and fixturedefs (#4871). + for handle in self._self_finalizer_handles: + handle() + self._self_finalizer_handles.clear() if len(exceptions) == 1: raise exceptions[0] elif len(exceptions) > 1: @@ -1088,7 +1090,7 @@ def execute(self, request: SubRequest) -> FixtureValue: for argname in self.argnames: request._get_active_fixturedef(argname) - self._self_finalizer = self._make_finalizer(request) + self._self_finalizer = functools.partial(self.finish, request) ihook = request.node.ihook try: # Setup the fixture, run the code in it, and cache the value @@ -1099,9 +1101,13 @@ def execute(self, request: SubRequest) -> FixtureValue: finally: request._fixturemanager._on_parametrized_fixture_setup(self) # Schedule our finalizer, even if the setup failed. - request.node.addfinalizer(self._self_finalizer) + self._self_finalizer_handles.append( + request.node.addfinalizer(self._self_finalizer) + ) for fixturedef in request._own_fixture_defs.values(): - fixturedef.addfinalizer(self._self_finalizer) + self._self_finalizer_handles.append( + fixturedef.addfinalizer(self._self_finalizer) + ) return result @@ -1115,15 +1121,6 @@ def _is_cache_hit(self, old_cache_key: object, new_cache_key: object) -> bool: cache_hit = new_cache_key is old_cache_key return cache_hit - def _make_finalizer(self, request: SubRequest) -> Callable[[], object]: - cache_epoch = self._cache_epoch - - def finalizer() -> None: - if self._cache_epoch == cache_epoch: - self.finish(request) - - return finalizer - def _finish_if_param_changed(self, item: nodes.Item) -> None: assert self.cached_result is not None assert self._self_finalizer is not None diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 81a744a4038..ab5555c418b 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -78,6 +78,25 @@ def _imply_path( return Path(fspath) +class _FinalizerId: + __slots__ = () + + +FinalizerStorage = dict[_FinalizerId, Callable[[], object]] + + +def append_finalizer( + finalizer_storage: FinalizerStorage, finalizer: Callable[[], object] +) -> Callable[[], None]: + finalizer_id = _FinalizerId() + finalizer_storage[finalizer_id] = finalizer + + def remove_finalizer() -> None: + finalizer_storage.pop(finalizer_id, None) + + return remove_finalizer + + _NodeType = TypeVar("_NodeType", bound="Node") @@ -381,14 +400,16 @@ def listextrakeywords(self) -> set[str]: def listnames(self) -> list[str]: return [x.name for x in self.listchain()] - def addfinalizer(self, fin: Callable[[], object]) -> None: + def addfinalizer(self, fin: Callable[[], object]) -> Callable[[], None]: """Register a function to be called without arguments when this node is finalized. This method can only be called when this node is active in a setup chain, for example during self.setup(). + + :returns: A handle that can be used to remove the finalizer. """ - self.session._setupstate.addfinalizer(fin, self) + return self.session._setupstate.addfinalizer(fin, self) def getparent(self, cls: type[_NodeType]) -> _NodeType | None: """Get the closest parent node (including self) which is an instance of diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py index ee5f118c806..dd16a7f07b6 100644 --- a/src/_pytest/runner.py +++ b/src/_pytest/runner.py @@ -27,8 +27,10 @@ from _pytest._code.code import TerminalRepr from _pytest.config.argparsing import Parser from _pytest.deprecated import check_ispytest +from _pytest.nodes import append_finalizer from _pytest.nodes import Collector from _pytest.nodes import Directory +from _pytest.nodes import FinalizerStorage from _pytest.nodes import Item from _pytest.nodes import Node from _pytest.outcomes import Exit @@ -501,7 +503,7 @@ def __init__(self) -> None: Node, tuple[ # Node's finalizers. - list[Callable[[], object]], + FinalizerStorage, # Node's exception and original traceback, if its setup raised. tuple[OutcomeException | Exception, types.TracebackType | None] | None, ], @@ -521,22 +523,28 @@ def setup(self, item: Item) -> None: for col in needed_collectors[len(self.stack) :]: assert col not in self.stack # Push onto the stack. - self.stack[col] = ([col.teardown], None) + finalizers = FinalizerStorage() + append_finalizer(finalizers, col.teardown) + self.stack[col] = (finalizers, None) try: col.setup() except TEST_OUTCOME as exc: self.stack[col] = (self.stack[col][0], (exc, exc.__traceback__)) raise - def addfinalizer(self, finalizer: Callable[[], object], node: Node) -> None: + def addfinalizer( + self, finalizer: Callable[[], object], node: Node + ) -> Callable[[], None]: """Attach a finalizer to the given node. The node must be currently active in the stack. + + :returns: A handle that can be used to remove the finalizer. """ assert node and not isinstance(node, tuple) assert callable(finalizer) assert node in self.stack, (node, self.stack) - self.stack[node][0].append(finalizer) + return append_finalizer(self.stack[node][0], finalizer) def teardown_exact(self, nextitem: Item | None) -> None: """Teardown the current stack up until reaching nodes that nextitem @@ -553,7 +561,7 @@ def teardown_exact(self, nextitem: Item | None) -> None: node, (finalizers, _) = self.stack.popitem() these_exceptions = [] while finalizers: - fin = finalizers.pop() + _, fin = finalizers.popitem() try: fin() except TEST_OUTCOME as e: diff --git a/testing/python/fixture_dependencies.py b/testing/python/fixture_dependencies.py index e0fd376ac6e..ba3409419bf 100644 --- a/testing/python/fixture_dependencies.py +++ b/testing/python/fixture_dependencies.py @@ -592,18 +592,16 @@ def test_second(dependent, param_fixture): ) -def test_cache_epoch_prevents_stale_finalizer(pytester: Pytester) -> None: - """Test that _cache_epoch prevents finalization by stale finalizers. - - This test demonstrates the problem that _cache_epoch solves: +def test_stale_finalizer_not_invoked(pytester: Pytester) -> None: + """Test that stale fixture finalizers are not invoked. Scenario: - 1. Fixture 'bar' depends on 'foo' via getfixturevalue in first evaluation + 1. Fixture 'bar' depends on 'foo' via getfixturevalue in first evaluation; + in a possible implementation, a finalizer is added to 'foo' to first destroy 'bar' 2. Fixture 'bar' gets recomputed and no longer depends on 'foo' 3. Fixture 'foo' gets finalized - 4. Without _cache_epoch, 'bar' would be finalized by the old finalizer - registered during step 1, even though it has been recomputed - 5. With _cache_epoch, the old finalizer checks the epoch and skips finalization + 4. Without any measures to remove the stale finalizer, 'bar' would be finalized + by the finalizer registered during step 1, even though 'bar' has been recomputed The test verifies that 'bar' is NOT finalized when 'foo' is finalized, because 'bar' was recomputed and the old finalizer should be ignored. @@ -651,8 +649,7 @@ def test_third(foo, bar): "SETUP S bar[2]", " *test_second*PASSED", "TEARDOWN S foo[1]", - # bar should NOT be torn down here because the old finalizer - # from the first evaluation should be ignored due to _cache_epoch. + # bar should NOT be torn down here. "test_fixtures.py::test_third[2-2] ", "SETUP S foo[2]", " *test_third*PASSED", From 35eb82bfc2b375383fb79b2876f4da36e9f32fe6 Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Thu, 15 Jan 2026 21:14:27 +0300 Subject: [PATCH 11/24] Fix typing --- changelog/4871.improvement.rst | 2 +- src/_pytest/fixtures.py | 21 ++++++++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/changelog/4871.improvement.rst b/changelog/4871.improvement.rst index 9d90ebbbd83..af08b2677fe 100644 --- a/changelog/4871.improvement.rst +++ b/changelog/4871.improvement.rst @@ -1,3 +1,3 @@ Garbage finalizers for fixture teardown are no longer accumulated in nodes and fixtures. -As a side-effect, :func:`Node.addfinalizer <_pytest.nodes.Node.addfinalizer>` now returns a handle that allows to remove the finalizer. +As a side-effect, :func:`Node.addfinalizer <_pytest.nodes.Node.addfinalizer>` and :func:`FixtureRequest.addfinalizer <_pytest.fixtures.FixtureRequest.addfinalizer>` now return a handle that allows to remove the finalizer. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index adb2f9dbe80..a0de4efea3f 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -486,9 +486,12 @@ def session(self) -> Session: return self._pyfuncitem.session @abc.abstractmethod - def addfinalizer(self, finalizer: Callable[[], object]) -> None: + def addfinalizer(self, finalizer: Callable[[], object]) -> Callable[[], None]: """Add finalizer/teardown function to be called without arguments after - the last test within the requesting test context finished execution.""" + the last test within the requesting test context finished execution. + + :returns: A handle that can be used to remove the finalizer. + """ raise NotImplementedError() def applymarker(self, marker: str | MarkDecorator) -> None: @@ -695,7 +698,7 @@ def _check_scope( pass @property - def node(self): + def node(self) -> nodes.Node: return self._pyfuncitem def __repr__(self) -> str: @@ -707,8 +710,8 @@ def _fillfixtures(self) -> None: if argname not in item.funcargs: item.funcargs[argname] = self.getfixturevalue(argname) - def addfinalizer(self, finalizer: Callable[[], object]) -> None: - self.node.addfinalizer(finalizer) + def addfinalizer(self, finalizer: Callable[[], object]) -> Callable[[], None]: + return self.node.addfinalizer(finalizer) @final @@ -794,8 +797,8 @@ def _format_fixturedef_line(self, fixturedef: FixtureDef[object]) -> str: sig = signature(factory) return f"{path}:{lineno + 1}: def {factory.__name__}{sig}" - def addfinalizer(self, finalizer: Callable[[], object]) -> None: - self._fixturedef.addfinalizer(finalizer) + def addfinalizer(self, finalizer: Callable[[], object]) -> Callable[[], None]: + return self._fixturedef.addfinalizer(finalizer) @final @@ -1186,8 +1189,8 @@ def __init__(self, request: FixtureRequest) -> None: ) self.cached_result = (request, [0], None) - def addfinalizer(self, finalizer: Callable[[], object]) -> None: - pass + def addfinalizer(self, finalizer: Callable[[], object]) -> Callable[[], None]: + return lambda: None def resolve_fixture_function( From 2151943f53838a2cd640afaf2bd993e7e5ff3aab Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Thu, 15 Jan 2026 21:19:47 +0300 Subject: [PATCH 12/24] Fix docs --- changelog/4871.improvement.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/4871.improvement.rst b/changelog/4871.improvement.rst index af08b2677fe..50c6d965803 100644 --- a/changelog/4871.improvement.rst +++ b/changelog/4871.improvement.rst @@ -1,3 +1,3 @@ Garbage finalizers for fixture teardown are no longer accumulated in nodes and fixtures. -As a side-effect, :func:`Node.addfinalizer <_pytest.nodes.Node.addfinalizer>` and :func:`FixtureRequest.addfinalizer <_pytest.fixtures.FixtureRequest.addfinalizer>` now return a handle that allows to remove the finalizer. +As a side-effect, :func:`Node.addfinalizer <_pytest.nodes.Node.addfinalizer>` and ``request.addfinalizer()`` now return a handle that allows to remove the finalizer. From 3e156b357822d5136a291f1cf094fd284a801bb1 Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Thu, 15 Jan 2026 21:43:56 +0300 Subject: [PATCH 13/24] Expand changelog entries --- changelog/4871.improvement.rst | 2 +- changelog/9287.bugfix.rst | 44 ++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/changelog/4871.improvement.rst b/changelog/4871.improvement.rst index 50c6d965803..29daca2f5ab 100644 --- a/changelog/4871.improvement.rst +++ b/changelog/4871.improvement.rst @@ -1,3 +1,3 @@ Garbage finalizers for fixture teardown are no longer accumulated in nodes and fixtures. -As a side-effect, :func:`Node.addfinalizer <_pytest.nodes.Node.addfinalizer>` and ``request.addfinalizer()`` now return a handle that allows to remove the finalizer. +:func:`Node.addfinalizer <_pytest.nodes.Node.addfinalizer>` and ``request.addfinalizer()`` now return a handle that allows to remove the finalizer. diff --git a/changelog/9287.bugfix.rst b/changelog/9287.bugfix.rst index 8e00a104683..4dc3406b587 100644 --- a/changelog/9287.bugfix.rst +++ b/changelog/9287.bugfix.rst @@ -1,3 +1,47 @@ Teardown of parametrized fixtures now happens in the teardown stage of the test before the parameter changes. Previously teardown would happen in the setup stage of the test where the parameter changes. + +If a test forces teardown of a parametrized fixture, e.g. using ``request.getfixturevalue()``, it instead fails. An example of such test: + +.. code-block:: pytest + + # conftest.py + import pytest + + @pytest.hookimpl(wrapper=True, tryfirst=True) + def pytest_collection_modifyitems(items): + # Disable built-in test reordering. + original_items = items[:] + yield + items[:] = original_items + + # test_invalid.py + import pytest + + @pytest.fixture(scope="session") + def foo(request): + return getattr(request, 'param', 'default') + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_a(foo): + assert foo == 1 + + def test_b(request): + request.getfixturevalue("foo") + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_c(foo): + assert foo == 1 + +This produces the following error: + +.. code-block:: console + + Parameter for the requested fixture changed unexpectedly in test: + test_invalid.py::test_b + Requested fixture 'foo' defined in: + test_invalid.py:4 + + Previous parameter value: 1 + New parameter value: None From 7f11150b8bc90a0f78fe6138b67d17e0b452baae Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Fri, 16 Jan 2026 00:59:54 +0300 Subject: [PATCH 14/24] Fix coverage --- src/_pytest/fixtures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index a0de4efea3f..20d0d999f02 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1190,7 +1190,7 @@ def __init__(self, request: FixtureRequest) -> None: self.cached_result = (request, [0], None) def addfinalizer(self, finalizer: Callable[[], object]) -> Callable[[], None]: - return lambda: None + assert False def resolve_fixture_function( From 3b69c2bd079341715c134ec6fd9ccf6b0acc5b6d Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Sun, 18 Jan 2026 20:03:09 +0300 Subject: [PATCH 15/24] Catch TEST_OUTCOME, fix _finish_if_param_changed --- src/_pytest/fixtures.py | 84 +++++++++++++++----------- src/_pytest/nodes.py | 8 +-- src/_pytest/python.py | 3 - src/_pytest/runner.py | 10 ++- testing/python/fixture_dependencies.py | 45 +++++++++++++- 5 files changed, 102 insertions(+), 48 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 20d0d999f02..1f5453d6e2f 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1036,9 +1036,9 @@ def __init__( # Can change if the fixture is executed with different parameters. self.cached_result: _FixtureCachedResult[FixtureValue] | None = None self._finalizers: Final[nodes.FinalizerStorage] = {} - # Callback to finalize cached_result. - self._self_finalizer: Callable[[], object] | None = None - # Handles to remove _self_finalizer from various scopes. + # The request object with which the fixture was set up. + self._cached_request: SubRequest | None = None + # Handles to remove our finalizer from various scopes. self._self_finalizer_handles: Final[list[Callable[[], None]]] = [] # only used to emit a deprecationwarning, can be removed in pytest9 @@ -1058,24 +1058,26 @@ def finish(self, request: SubRequest) -> None: _, fin = self._finalizers.popitem() try: fin() - except BaseException as e: + except TEST_OUTCOME as e: exceptions.append(e) node = request.node try: node.ihook.pytest_fixture_post_finalizer(fixturedef=self, request=request) - except BaseException as e: + except TEST_OUTCOME as e: exceptions.append(e) + # Even if finalization fails, we invalidate the cached fixture # value and remove all finalizers because they may be bound methods # which will keep instances alive. self.cached_result = None self._finalizers.clear() - request._fixturemanager._on_parametrized_fixture_finished(self) - self._self_finalizer = None + request._fixturemanager._on_fixture_finished(self) + self._cached_request = None # Avoid accumulating garbage finalizers in nodes and fixturedefs (#4871). for handle in self._self_finalizer_handles: handle() self._self_finalizer_handles.clear() + if len(exceptions) == 1: raise exceptions[0] elif len(exceptions) > 1: @@ -1093,7 +1095,6 @@ def execute(self, request: SubRequest) -> FixtureValue: for argname in self.argnames: request._get_active_fixturedef(argname) - self._self_finalizer = functools.partial(self.finish, request) ihook = request.node.ihook try: # Setup the fixture, run the code in it, and cache the value @@ -1102,15 +1103,13 @@ def execute(self, request: SubRequest) -> FixtureValue: fixturedef=self, request=request ) finally: - request._fixturemanager._on_parametrized_fixture_setup(self) + self._cached_request = request + request._fixturemanager._on_fixture_setup(self) # Schedule our finalizer, even if the setup failed. - self._self_finalizer_handles.append( - request.node.addfinalizer(self._self_finalizer) - ) + fin = functools.partial(self.finish, request) + self._self_finalizer_handles.append(request.node.addfinalizer(fin)) for fixturedef in request._own_fixture_defs.values(): - self._self_finalizer_handles.append( - fixturedef.addfinalizer(self._self_finalizer) - ) + self._self_finalizer_handles.append(fixturedef.addfinalizer(fin)) return result @@ -1124,21 +1123,36 @@ def _is_cache_hit(self, old_cache_key: object, new_cache_key: object) -> bool: cache_hit = new_cache_key is old_cache_key return cache_hit - def _finish_if_param_changed(self, item: nodes.Item) -> None: + def _finish_if_param_changed(self, nextitem: nodes.Item) -> None: + assert self._cached_request is not None assert self.cached_result is not None - assert self._self_finalizer is not None old_cache_key = self.cached_result[1] - callspec: CallSpec2 | None = getattr(item, "callspec", None) - if callspec is None or self.argname not in callspec.params: - # Carry the fixture cache over a test that does not request - # the (previously) parametrized fixture statically. - # This implementation decision has the consequence that requesting - # the fixture dynamically is disallowed, see _check_cache_hit. + callspec: CallSpec2 | None = getattr(nextitem, "callspec", None) + if callspec is not None: + new_cache_key = callspec.params.get(self.argname, None) + else: + new_cache_key = None + + if old_cache_key is None and new_cache_key is None: + # Shortcut for the most common case. return - new_cache_key = callspec.params[self.argname] + + if new_cache_key is None: + fixtureinfo: FuncFixtureInfo | None + fixtureinfo = getattr(nextitem, "_fixtureinfo", None) + if fixtureinfo is None: + return + fixturedefs = fixtureinfo.name2fixturedefs.get(self.argname, ()) + if self not in fixturedefs: + # Carry the fixture cache over a test that does not request + # the (previously) parametrized fixture statically. + # This implementation decision has the consequence that requesting + # the fixture dynamically is disallowed, see _check_cache_hit. + return + if not self._is_cache_hit(old_cache_key, new_cache_key): - self._self_finalizer() + self.finish(self._cached_request) def _check_cache_hit(self, request: SubRequest, old_cache_key: object) -> None: new_cache_key = self.cache_key(request) @@ -1636,7 +1650,7 @@ def __init__(self, session: Session) -> None: "": self.config.getini("usefixtures"), } # Using dict for keeping insertion order. - self._active_parametrized_fixturedefs: Final[dict[FixtureDef[Any], None]] = {} + self._active_fixturedefs: Final[dict[FixtureDef[Any], None]] = {} session.config.pluginmanager.register(self, "funcmanage") def getfixtureinfo( @@ -1984,24 +1998,24 @@ def _matchfactories( if fixturedef.baseid in parentnodeids: yield fixturedef - def _teardown_stale_fixtures(self, item: nodes.Item) -> None: + def _teardown_stale_fixtures(self, nextitem: nodes.Item) -> None: exceptions: list[BaseException] = [] - for fixturedef in reversed(list(self._active_parametrized_fixturedefs.keys())): + for fixturedef in reversed(list(self._active_fixturedefs.keys())): try: - fixturedef._finish_if_param_changed(item) - except BaseException as e: + fixturedef._finish_if_param_changed(nextitem) + except TEST_OUTCOME as e: exceptions.append(e) if len(exceptions) == 1: raise exceptions[0] elif len(exceptions) > 1: - msg = f'errors while tearing down fixtures for "{item.nodeid}"' + msg = f'errors while tearing down fixtures for "{nextitem.nodeid}"' raise BaseExceptionGroup(msg, exceptions[::-1]) - def _on_parametrized_fixture_setup(self, fixturedef: FixtureDef[Any]) -> None: - self._active_parametrized_fixturedefs[fixturedef] = None + def _on_fixture_setup(self, fixturedef: FixtureDef[Any]) -> None: + self._active_fixturedefs[fixturedef] = None - def _on_parametrized_fixture_finished(self, fixturedef: FixtureDef[Any]) -> None: - del self._active_parametrized_fixturedefs[fixturedef] + def _on_fixture_finished(self, fixturedef: FixtureDef[Any]) -> None: + del self._active_fixturedefs[fixturedef] def show_fixtures_per_test(config: Config) -> int | ExitCode: diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index ab5555c418b..e99f2d03baa 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -82,11 +82,12 @@ class _FinalizerId: __slots__ = () -FinalizerStorage = dict[_FinalizerId, Callable[[], object]] +Finalizer = Callable[[], object] +FinalizerStorage = dict[_FinalizerId, Finalizer] def append_finalizer( - finalizer_storage: FinalizerStorage, finalizer: Callable[[], object] + finalizer_storage: FinalizerStorage, finalizer: Finalizer ) -> Callable[[], None]: finalizer_id = _FinalizerId() finalizer_storage[finalizer_id] = finalizer @@ -791,6 +792,3 @@ def location(self) -> tuple[str, int | None, str]: relfspath = self.session._node_location_to_relpath(path) assert type(location[2]) is str return (relfspath, location[1], location[2]) - - def _teardown_stale_fixtures(self) -> None: - pass diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 96512450583..7374fa3cee0 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1687,9 +1687,6 @@ def runtest(self) -> None: def setup(self) -> None: self._request._fillfixtures() - def _teardown_stale_fixtures(self) -> None: - self.session._fixturemanager._teardown_stale_fixtures(self) - def _traceback_filter(self, excinfo: ExceptionInfo[BaseException]) -> Traceback: if hasattr(self, "_obj") and not self.config.getoption("fulltrace", False): code = _pytest._code.Code.from_function(get_real_func(self.obj)) diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py index dd16a7f07b6..093858e9933 100644 --- a/src/_pytest/runner.py +++ b/src/_pytest/runner.py @@ -566,10 +566,11 @@ def teardown_exact(self, nextitem: Item | None) -> None: fin() except TEST_OUTCOME as e: these_exceptions.append(e) - if isinstance(nextitem, Item): + + if isinstance(node, Item) and nextitem is not None: try: - nextitem._teardown_stale_fixtures() - except BaseException as e: + self._teardown_item_towards_nextitem(nextitem) + except TEST_OUTCOME as e: exceptions.append(e) if len(these_exceptions) == 1: @@ -585,6 +586,9 @@ def teardown_exact(self, nextitem: Item | None) -> None: if nextitem is None: assert not self.stack + def _teardown_item_towards_nextitem(self, nextitem: Item) -> None: + nextitem.session._fixturemanager._teardown_stale_fixtures(nextitem) + def collect_one_node(collector: Collector) -> CollectReport: ihook = collector.ihook diff --git a/testing/python/fixture_dependencies.py b/testing/python/fixture_dependencies.py index ba3409419bf..66740590be8 100644 --- a/testing/python/fixture_dependencies.py +++ b/testing/python/fixture_dependencies.py @@ -184,8 +184,6 @@ def test_in_class(self, foo, bar): def test_fixture_post_finalizer_hook_exception(pytester: Pytester) -> None: """Test that exceptions in pytest_fixture_post_finalizer hook are caught. - Covers line 1069: node.ihook.pytest_fixture_post_finalizer(fixturedef=self, request=request) - Also verifies that the fixture cache is properly reset even when the post_finalizer hook raises an exception, so the fixture can be rebuilt in subsequent tests. @@ -237,6 +235,49 @@ def test_second(my_fixture): ) +def test_fixture_rebuilt_when_param_appears(pytester: Pytester) -> None: + """Test that fixtures are rebuilt when their parameter appears or disappears. + + We disable test reordering to ensure tests run in the defined order. + """ + pytester.makeconftest( + """ + import pytest + + @pytest.hookimpl(wrapper=True, tryfirst=True) + def pytest_collection_modifyitems(items): + original_items = items[:] + yield + items[:] = original_items + """ + ) + pytester.makepyfile( + test_fixtures=""" + import pytest + + @pytest.fixture(scope="session") + def foo(request): + return getattr(request, "param", None) + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_a(foo): + assert foo == 1 + + def test_b(foo): + assert foo is None + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_c(foo): + assert foo == 1 + + def test_d(foo): + assert foo is None + """ + ) + result = pytester.runpytest() + result.assert_outcomes(passed=4) + + def test_fixture_not_rebuilt_when_not_requested(pytester: Pytester) -> None: """Test that fixtures are NOT rebuilt when not requested in an intermediate test. From 07a0a0c0e5c0fe3b64638fad137ff862e62ab148 Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Sun, 18 Jan 2026 20:44:34 +0300 Subject: [PATCH 16/24] Fix coverage --- testing/python/fixture_dependencies.py | 63 ++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/testing/python/fixture_dependencies.py b/testing/python/fixture_dependencies.py index 66740590be8..e3e23fb4da8 100644 --- a/testing/python/fixture_dependencies.py +++ b/testing/python/fixture_dependencies.py @@ -235,6 +235,69 @@ def test_second(my_fixture): ) +def test_parametrized_fixture_carries_over_unaware_item(pytester: Pytester) -> None: + """Test that cached parametrized fixtures carry over non-fixture-aware test items. + + We disable test reordering to ensure tests run in the defined order. + """ + pytester.makeconftest( + """ + import pytest + + class MeaningOfLifeTest(pytest.Item): + def runtest(self): + return self.path.read_text(encoding="utf-8").strip() == "42" + + def pytest_collect_file(parent, file_path): + if "meaning_of_life" in file_path.name: + return MeaningOfLifeTest.from_parent(parent, path=file_path, name=file_path.stem) + + @pytest.hookimpl(wrapper=True, tryfirst=True) + def pytest_collection_modifyitems(items): + original_items = items[:] + yield + items[:] = original_items + + @pytest.fixture(scope="session") + def foo(request): + return getattr(request, "param", None) + """ + ) + pytester.makepyfile( + test_1=""" + import pytest + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_first(foo): + pass + """ + ) + pytester.maketxtfile(test_2_meaning_of_life="42\n") + pytester.makepyfile( + test_3=""" + import pytest + + @pytest.mark.parametrize("foo", [1], indirect=True) + def test_third(foo): + pass + """ + ) + result = pytester.runpytest("-v", "--setup-only") + result.stdout.fnmatch_lines( + [ + "test_1.py::test_first[1] ", + "SETUP S foo[1]", + " test_1.py::test_first[1] (fixtures used: foo, request)", + ".::test_2_meaning_of_life ", + " .::test_2_meaning_of_life", + "test_3.py::test_third[1] ", + " test_3.py::test_third[1] (fixtures used: foo, request)", + "TEARDOWN S foo[1]", + ], + consecutive=True, + ) + + def test_fixture_rebuilt_when_param_appears(pytester: Pytester) -> None: """Test that fixtures are rebuilt when their parameter appears or disappears. From 60f428645188a8715db3eb426c567e4008952952 Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Sun, 18 Jan 2026 20:57:25 +0300 Subject: [PATCH 17/24] Use TypeAlias --- src/_pytest/nodes.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 16d6ceb5c34..97ae01efdc1 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -16,6 +16,7 @@ from typing import NoReturn from typing import overload from typing import TYPE_CHECKING +from typing import TypeAlias from typing import TypeVar import warnings @@ -96,8 +97,8 @@ class _FinalizerId: __slots__ = () -Finalizer = Callable[[], object] -FinalizerStorage = dict[_FinalizerId, Finalizer] +Finalizer: TypeAlias = Callable[[], object] +FinalizerStorage: TypeAlias = dict[_FinalizerId, Finalizer] def append_finalizer( From d88ccd9ef76514032be0dc9c1136a0bdfd8205ea Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Mon, 19 Jan 2026 21:02:09 +0300 Subject: [PATCH 18/24] Move finalizers management to SetupState --- src/_pytest/fixtures.py | 80 ++++++++++++----------------------------- src/_pytest/nodes.py | 21 ----------- src/_pytest/runner.py | 80 ++++++++++++++++++++++++++++++++++++----- 3 files changed, 93 insertions(+), 88 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index a6e506a826a..a4c997e2889 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -69,10 +69,6 @@ from _pytest.warning_types import PytestWarning -if sys.version_info < (3, 11): - from exceptiongroup import BaseExceptionGroup - - if TYPE_CHECKING: from _pytest.python import CallSpec2 from _pytest.python import Function @@ -1035,7 +1031,6 @@ def __init__( # If the fixture was executed, the current value of the fixture. # Can change if the fixture is executed with different parameters. self.cached_result: _FixtureCachedResult[FixtureValue] | None = None - self._finalizers: Final[nodes.FinalizerStorage] = {} # The request object with which the fixture was set up. self._cached_request: SubRequest | None = None # Handles to remove our finalizer from various scopes. @@ -1050,39 +1045,20 @@ def scope(self) -> _ScopeName: return self._scope.value def addfinalizer(self, finalizer: Callable[[], object]) -> Callable[[], None]: - return nodes.append_finalizer(self._finalizers, finalizer) + assert self._cached_request is not None + setupstate = self._cached_request.session._setupstate + return setupstate.fixture_addfinalizer(finalizer, self) def finish(self, request: SubRequest) -> None: - exceptions: list[BaseException] = [] - while self._finalizers: - _, fin = self._finalizers.popitem() - try: - fin() - except TEST_OUTCOME as e: - exceptions.append(e) - node = request.node try: - node.ihook.pytest_fixture_post_finalizer(fixturedef=self, request=request) - except TEST_OUTCOME as e: - exceptions.append(e) - - # Even if finalization fails, we invalidate the cached fixture - # value and remove all finalizers because they may be bound methods - # which will keep instances alive. - self.cached_result = None - self._finalizers.clear() - request._fixturemanager._on_fixture_finished(self) - self._cached_request = None - # Avoid accumulating garbage finalizers in nodes and fixturedefs (#4871). - for handle in self._self_finalizer_handles: - handle() - self._self_finalizer_handles.clear() - - if len(exceptions) == 1: - raise exceptions[0] - elif len(exceptions) > 1: - msg = f'errors while tearing down fixture "{self.argname}" of {node}' - raise BaseExceptionGroup(msg, exceptions[::-1]) + request.session._setupstate.fixture_teardown(self, request.node) + finally: + self.cached_result = None + self._cached_request = None + # Avoid accumulating garbage finalizers in nodes and fixturedefs (#4871). + for handle in self._self_finalizer_handles: + handle() + self._self_finalizer_handles.clear() def execute(self, request: SubRequest) -> FixtureValue: """Return the value of this fixture, executing it if not cached.""" @@ -1095,6 +1071,11 @@ def execute(self, request: SubRequest) -> FixtureValue: for argname in self.argnames: request._get_active_fixturedef(argname) + self._cached_request = request + setupstate = request.session._setupstate + setupstate.fixture_setup(self) + setupstate.fixture_addfinalizer(self._run_post_finalizer, self) + ihook = request.node.ihook try: # Setup the fixture, run the code in it, and cache the value @@ -1103,8 +1084,6 @@ def execute(self, request: SubRequest) -> FixtureValue: fixturedef=self, request=request ) finally: - self._cached_request = request - request._fixturemanager._on_fixture_setup(self) # Schedule our finalizer, even if the setup failed. fin = functools.partial(self.finish, request) self._self_finalizer_handles.append(request.node.addfinalizer(fin)) @@ -1113,6 +1092,12 @@ def execute(self, request: SubRequest) -> FixtureValue: return result + def _run_post_finalizer(self) -> None: + request = self._cached_request + assert request is not None + ihook = request.node.ihook + ihook.pytest_fixture_post_finalizer(fixturedef=self, request=request) + def _is_cache_hit(self, old_cache_key: object, new_cache_key: object) -> bool: try: # Attempt to make a normal == check: this might fail for objects @@ -1649,8 +1634,6 @@ def __init__(self, session: Session) -> None: self._nodeid_autousenames: Final[dict[str, list[str]]] = { "": self.config.getini("usefixtures"), } - # Using dict for keeping insertion order. - self._active_fixturedefs: Final[dict[FixtureDef[Any], None]] = {} session.config.pluginmanager.register(self, "funcmanage") def getfixtureinfo( @@ -1998,25 +1981,6 @@ def _matchfactories( if fixturedef.baseid in parentnodeids: yield fixturedef - def _teardown_stale_fixtures(self, nextitem: nodes.Item) -> None: - exceptions: list[BaseException] = [] - for fixturedef in reversed(list(self._active_fixturedefs.keys())): - try: - fixturedef._finish_if_param_changed(nextitem) - except TEST_OUTCOME as e: - exceptions.append(e) - if len(exceptions) == 1: - raise exceptions[0] - elif len(exceptions) > 1: - msg = f'errors while tearing down fixtures for "{nextitem.nodeid}"' - raise BaseExceptionGroup(msg, exceptions[::-1]) - - def _on_fixture_setup(self, fixturedef: FixtureDef[Any]) -> None: - self._active_fixturedefs[fixturedef] = None - - def _on_fixture_finished(self, fixturedef: FixtureDef[Any]) -> None: - del self._active_fixturedefs[fixturedef] - def show_fixtures_per_test(config: Config) -> int | ExitCode: from _pytest.main import wrap_session diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 97ae01efdc1..39b7bb89ae9 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -16,7 +16,6 @@ from typing import NoReturn from typing import overload from typing import TYPE_CHECKING -from typing import TypeAlias from typing import TypeVar import warnings @@ -93,26 +92,6 @@ def _imply_path( return Path(fspath) -class _FinalizerId: - __slots__ = () - - -Finalizer: TypeAlias = Callable[[], object] -FinalizerStorage: TypeAlias = dict[_FinalizerId, Finalizer] - - -def append_finalizer( - finalizer_storage: FinalizerStorage, finalizer: Finalizer -) -> Callable[[], None]: - finalizer_id = _FinalizerId() - finalizer_storage[finalizer_id] = finalizer - - def remove_finalizer() -> None: - finalizer_storage.pop(finalizer_id, None) - - return remove_finalizer - - _NodeType = TypeVar("_NodeType", bound="Node") diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py index 093858e9933..ec501d2c761 100644 --- a/src/_pytest/runner.py +++ b/src/_pytest/runner.py @@ -9,11 +9,13 @@ import os import sys import types +from typing import Any from typing import cast from typing import final from typing import Generic from typing import Literal from typing import TYPE_CHECKING +from typing import TypeAlias from typing import TypeVar from .config import Config @@ -27,10 +29,8 @@ from _pytest._code.code import TerminalRepr from _pytest.config.argparsing import Parser from _pytest.deprecated import check_ispytest -from _pytest.nodes import append_finalizer from _pytest.nodes import Collector from _pytest.nodes import Directory -from _pytest.nodes import FinalizerStorage from _pytest.nodes import Item from _pytest.nodes import Node from _pytest.outcomes import Exit @@ -43,6 +43,7 @@ from exceptiongroup import BaseExceptionGroup if TYPE_CHECKING: + from _pytest.fixtures import FixtureDef from _pytest.main import Session from _pytest.terminal import TerminalReporter @@ -433,6 +434,26 @@ def collect() -> list[Item | Collector]: return rep +class _FinalizerId: + __slots__ = () + + +_Finalizer: TypeAlias = Callable[[], object] +_FinalizerStorage: TypeAlias = dict[_FinalizerId, _Finalizer] + + +def _append_finalizer( + finalizer_storage: _FinalizerStorage, finalizer: _Finalizer +) -> Callable[[], None]: + finalizer_id = _FinalizerId() + finalizer_storage[finalizer_id] = finalizer + + def remove_finalizer() -> None: + finalizer_storage.pop(finalizer_id, None) + + return remove_finalizer + + class SetupState: """Shared state for setting up/tearing down test items or collectors in a session. @@ -503,11 +524,12 @@ def __init__(self) -> None: Node, tuple[ # Node's finalizers. - FinalizerStorage, + _FinalizerStorage, # Node's exception and original traceback, if its setup raised. tuple[OutcomeException | Exception, types.TracebackType | None] | None, ], ] = {} + self._fixture_finalizers: dict[FixtureDef[Any], _FinalizerStorage] = {} def setup(self, item: Item) -> None: """Setup objects along the collector chain to the item.""" @@ -523,8 +545,8 @@ def setup(self, item: Item) -> None: for col in needed_collectors[len(self.stack) :]: assert col not in self.stack # Push onto the stack. - finalizers = FinalizerStorage() - append_finalizer(finalizers, col.teardown) + finalizers = _FinalizerStorage() + _append_finalizer(finalizers, col.teardown) self.stack[col] = (finalizers, None) try: col.setup() @@ -544,7 +566,7 @@ def addfinalizer( assert node and not isinstance(node, tuple) assert callable(finalizer) assert node in self.stack, (node, self.stack) - return append_finalizer(self.stack[node][0], finalizer) + return _append_finalizer(self.stack[node][0], finalizer) def teardown_exact(self, nextitem: Item | None) -> None: """Teardown the current stack up until reaching nodes that nextitem @@ -569,7 +591,7 @@ def teardown_exact(self, nextitem: Item | None) -> None: if isinstance(node, Item) and nextitem is not None: try: - self._teardown_item_towards_nextitem(nextitem) + self._finish_stale_fixtures(nextitem) except TEST_OUTCOME as e: exceptions.append(e) @@ -586,8 +608,48 @@ def teardown_exact(self, nextitem: Item | None) -> None: if nextitem is None: assert not self.stack - def _teardown_item_towards_nextitem(self, nextitem: Item) -> None: - nextitem.session._fixturemanager._teardown_stale_fixtures(nextitem) + def fixture_setup(self, fixturedef: FixtureDef[Any]) -> None: + assert fixturedef not in self._fixture_finalizers + self._fixture_finalizers[fixturedef] = _FinalizerStorage() + + def fixture_addfinalizer( + self, finalizer: Callable[[], object], fixturedef: FixtureDef[Any] + ) -> Callable[[], None]: + assert fixturedef in self._fixture_finalizers + return _append_finalizer(self._fixture_finalizers[fixturedef], finalizer) + + def fixture_teardown(self, fixturedef: FixtureDef[Any], node: Node) -> None: + assert fixturedef in self._fixture_finalizers + # Do not remove for now to allow adding finalizers last second. + finalizers = self._fixture_finalizers[fixturedef] + + exceptions: list[BaseException] = [] + while finalizers: + _, fin = finalizers.popitem() + try: + fin() + except TEST_OUTCOME as e: + exceptions.append(e) + + self._fixture_finalizers.pop(fixturedef) + if len(exceptions) == 1: + raise exceptions[0] + elif len(exceptions) > 1: + msg = f'errors while tearing down fixture "{fixturedef.argname}" of {node}' + raise BaseExceptionGroup(msg, exceptions[::-1]) + + def _finish_stale_fixtures(self, nextitem: Item) -> None: + exceptions: list[BaseException] = [] + for fixturedef in reversed(list(self._fixture_finalizers.keys())): + try: + fixturedef._finish_if_param_changed(nextitem) + except TEST_OUTCOME as e: + exceptions.append(e) + if len(exceptions) == 1: + raise exceptions[0] + elif len(exceptions) > 1: + msg = f'errors while tearing down fixtures for "{nextitem.nodeid}"' + raise BaseExceptionGroup(msg, exceptions[::-1]) def collect_one_node(collector: Collector) -> CollectReport: From 4301de74b56ae9196358803e9b24d57cffa81399 Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Mon, 19 Jan 2026 21:22:06 +0300 Subject: [PATCH 19/24] Minor diff cleanups --- src/_pytest/fixtures.py | 14 ++++---------- src/_pytest/runner.py | 9 ++++----- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index a4c997e2889..682de44fa30 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -387,7 +387,7 @@ def __init__( self.param: Any # FixtureDefs requested through this specific `request` object. # Allows tracking dependencies on fixtures. - self._own_fixture_defs: Final[dict[str, FixtureDef[Any]]] = {} + self._own_fixture_defs: Final[dict[str, FixtureDef[object]]] = {} @property def _fixturemanager(self) -> FixtureManager: @@ -560,11 +560,6 @@ def _get_active_fixturedef(self, argname: str) -> FixtureDef[object]: self._own_fixture_defs[argname] = fixturedef return fixturedef - fixturedef = self._find_fixturedef(argname) - self._execute_fixturedef(fixturedef) - return fixturedef - - def _find_fixturedef(self, argname: str) -> FixtureDef[object]: # Find the appropriate fixturedef. fixturedefs = self._arg2fixturedefs.get(argname, None) if fixturedefs is None: @@ -597,10 +592,8 @@ def _find_fixturedef(self, argname: str) -> FixtureDef[object]: # If already consumed all of the available levels, fail. if -index > len(fixturedefs): raise FixtureLookupError(argname, self) - return fixturedefs[index] + fixturedef = fixturedefs[index] - def _execute_fixturedef(self, fixturedef: FixtureDef[object]) -> None: - argname = fixturedef.argname # Prepare a SubRequest object for calling the fixture. callspec: CallSpec2 | None = getattr(self._pyfuncitem, "callspec", None) if callspec is not None and argname in callspec.params: @@ -628,6 +621,7 @@ def _execute_fixturedef(self, fixturedef: FixtureDef[object]) -> None: fixturedef.execute(request=subrequest) self._fixture_defs[argname] = fixturedef + return fixturedef def _check_fixturedef_without_param(self, fixturedef: FixtureDef[object]) -> None: """Check that this request is allowed to execute this fixturedef without @@ -643,7 +637,7 @@ def _check_fixturedef_without_param(self, fixturedef: FixtureDef[object]) -> Non ) fail(msg, pytrace=False) if has_params: - frame = inspect.stack()[4] + frame = inspect.stack()[3] frameinfo = inspect.getframeinfo(frame[0]) source_path = absolutepath(frameinfo.filename) source_lineno = frameinfo.lineno diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py index ec501d2c761..2981bed44ec 100644 --- a/src/_pytest/runner.py +++ b/src/_pytest/runner.py @@ -9,7 +9,6 @@ import os import sys import types -from typing import Any from typing import cast from typing import final from typing import Generic @@ -529,7 +528,7 @@ def __init__(self) -> None: tuple[OutcomeException | Exception, types.TracebackType | None] | None, ], ] = {} - self._fixture_finalizers: dict[FixtureDef[Any], _FinalizerStorage] = {} + self._fixture_finalizers: dict[FixtureDef[object], _FinalizerStorage] = {} def setup(self, item: Item) -> None: """Setup objects along the collector chain to the item.""" @@ -608,17 +607,17 @@ def teardown_exact(self, nextitem: Item | None) -> None: if nextitem is None: assert not self.stack - def fixture_setup(self, fixturedef: FixtureDef[Any]) -> None: + def fixture_setup(self, fixturedef: FixtureDef[object]) -> None: assert fixturedef not in self._fixture_finalizers self._fixture_finalizers[fixturedef] = _FinalizerStorage() def fixture_addfinalizer( - self, finalizer: Callable[[], object], fixturedef: FixtureDef[Any] + self, finalizer: Callable[[], object], fixturedef: FixtureDef[object] ) -> Callable[[], None]: assert fixturedef in self._fixture_finalizers return _append_finalizer(self._fixture_finalizers[fixturedef], finalizer) - def fixture_teardown(self, fixturedef: FixtureDef[Any], node: Node) -> None: + def fixture_teardown(self, fixturedef: FixtureDef[object], node: Node) -> None: assert fixturedef in self._fixture_finalizers # Do not remove for now to allow adding finalizers last second. finalizers = self._fixture_finalizers[fixturedef] From dfcff7901d1e8804e0a95ea4fc132f0b675cb416 Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Mon, 19 Jan 2026 21:40:41 +0300 Subject: [PATCH 20/24] Use double quotes in changelog's snippet --- changelog/9287.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/9287.bugfix.rst b/changelog/9287.bugfix.rst index 4dc3406b587..ae0dff3f5de 100644 --- a/changelog/9287.bugfix.rst +++ b/changelog/9287.bugfix.rst @@ -21,7 +21,7 @@ If a test forces teardown of a parametrized fixture, e.g. using ``request.getfix @pytest.fixture(scope="session") def foo(request): - return getattr(request, 'param', 'default') + return getattr(request, "param", "default") @pytest.mark.parametrize("foo", [1], indirect=True) def test_a(foo): From a4b64893be3b6096d379046851ec560db00ee61e Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Mon, 19 Jan 2026 23:47:42 +0300 Subject: [PATCH 21/24] Restart flaky test_timeout_and_exit From e29ac16b60ee81909e37b31a974e2b734bb70e47 Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Sun, 25 Jan 2026 02:01:10 +0300 Subject: [PATCH 22/24] Add FinalizerHandle class --- doc/en/reference/reference.rst | 7 +++++++ src/_pytest/fixtures.py | 22 +++++++++++++------- src/_pytest/nodes.py | 6 +++++- src/_pytest/runner.py | 37 ++++++++++++++++++++++++++-------- src/pytest/__init__.py | 2 ++ 5 files changed, 58 insertions(+), 16 deletions(-) diff --git a/doc/en/reference/reference.rst b/doc/en/reference/reference.rst index 62ae3564e18..6126b4787f7 100644 --- a/doc/en/reference/reference.rst +++ b/doc/en/reference/reference.rst @@ -993,6 +993,13 @@ ExitCode :members: +FinalizerHandle +~~~~~~~~~~~~~~~ + +.. autoclass:: pytest.FinalizerHandle() + :members: + + FixtureDef ~~~~~~~~~~ diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 682de44fa30..384d6f238b8 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -73,6 +73,7 @@ from _pytest.python import CallSpec2 from _pytest.python import Function from _pytest.python import Metafunc + from _pytest.runner import FinalizerHandle # The value of the fixture -- return/yield of the fixture function (type variable). @@ -482,11 +483,14 @@ def session(self) -> Session: return self._pyfuncitem.session @abc.abstractmethod - def addfinalizer(self, finalizer: Callable[[], object]) -> Callable[[], None]: + def addfinalizer(self, finalizer: Callable[[], object]) -> FinalizerHandle: """Add finalizer/teardown function to be called without arguments after the last test within the requesting test context finished execution. :returns: A handle that can be used to remove the finalizer. + + .. versionadded:: 9.1 + The :class:`FinalizerHandle ` result. """ raise NotImplementedError() @@ -700,7 +704,7 @@ def _fillfixtures(self) -> None: if argname not in item.funcargs: item.funcargs[argname] = self.getfixturevalue(argname) - def addfinalizer(self, finalizer: Callable[[], object]) -> Callable[[], None]: + def addfinalizer(self, finalizer: Callable[[], object]) -> FinalizerHandle: return self.node.addfinalizer(finalizer) @@ -787,7 +791,7 @@ def _format_fixturedef_line(self, fixturedef: FixtureDef[object]) -> str: sig = signature(factory) return f"{path}:{lineno + 1}: def {factory.__name__}{sig}" - def addfinalizer(self, finalizer: Callable[[], object]) -> Callable[[], None]: + def addfinalizer(self, finalizer: Callable[[], object]) -> FinalizerHandle: return self._fixturedef.addfinalizer(finalizer) @@ -1028,7 +1032,7 @@ def __init__( # The request object with which the fixture was set up. self._cached_request: SubRequest | None = None # Handles to remove our finalizer from various scopes. - self._self_finalizer_handles: Final[list[Callable[[], None]]] = [] + self._self_finalizer_handles: Final[list[FinalizerHandle]] = [] # only used to emit a deprecationwarning, can be removed in pytest9 self._autouse = _autouse @@ -1038,7 +1042,7 @@ def scope(self) -> _ScopeName: """Scope string, one of "function", "class", "module", "package", "session".""" return self._scope.value - def addfinalizer(self, finalizer: Callable[[], object]) -> Callable[[], None]: + def addfinalizer(self, finalizer: Callable[[], object]) -> FinalizerHandle: assert self._cached_request is not None setupstate = self._cached_request.session._setupstate return setupstate.fixture_addfinalizer(finalizer, self) @@ -1051,7 +1055,7 @@ def finish(self, request: SubRequest) -> None: self._cached_request = None # Avoid accumulating garbage finalizers in nodes and fixturedefs (#4871). for handle in self._self_finalizer_handles: - handle() + handle.remove_finalizer() self._self_finalizer_handles.clear() def execute(self, request: SubRequest) -> FixtureValue: @@ -1182,7 +1186,11 @@ def __init__(self, request: FixtureRequest) -> None: ) self.cached_result = (request, [0], None) - def addfinalizer(self, finalizer: Callable[[], object]) -> Callable[[], None]: + def addfinalizer(self, finalizer: Callable[[], object]) -> FinalizerHandle: + # RequestFixtureDef is not exposed to the user, e.g. it does not call + # pytest_fixture_setup and pytest_fixture_post_teardown. + # Also RequestFixtureDef is not finalized properly, so if addfinalizer is + # somehow called, then the finalizer would never be called. assert False diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 39b7bb89ae9..57b09ab6ba7 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -47,6 +47,7 @@ # Imported here due to circular import. from _pytest.main import Session + from _pytest.runner import FinalizerHandle SEP = "/" @@ -395,7 +396,7 @@ def listextrakeywords(self) -> set[str]: def listnames(self) -> list[str]: return [x.name for x in self.listchain()] - def addfinalizer(self, fin: Callable[[], object]) -> Callable[[], None]: + def addfinalizer(self, fin: Callable[[], object]) -> FinalizerHandle: """Register a function to be called without arguments when this node is finalized. @@ -403,6 +404,9 @@ def addfinalizer(self, fin: Callable[[], object]) -> Callable[[], None]: in a setup chain, for example during self.setup(). :returns: A handle that can be used to remove the finalizer. + + .. versionadded:: 9.1 + The :class:`FinalizerHandle ` result. """ return self.session._setupstate.addfinalizer(fin, self) diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py index 2981bed44ec..d2fa7b19bdc 100644 --- a/src/_pytest/runner.py +++ b/src/_pytest/runner.py @@ -441,16 +441,37 @@ class _FinalizerId: _FinalizerStorage: TypeAlias = dict[_FinalizerId, _Finalizer] +class FinalizerHandle: + """ + Allows to remove a finalizer after an ``addfinalizer`` call. + + The handle does not own the finalizer, dropping the handle does nothing. + + .. versionadded:: 9.1 + """ + + def __init__( + self, + finalizer_storage: _FinalizerStorage, + id: _FinalizerId, + *, + _ispytest: bool = False, + ) -> None: + check_ispytest(_ispytest) + self._finalizer_storage = finalizer_storage + self._id = id + + def remove_finalizer(self) -> None: + """Remove the finalizer.""" + self._finalizer_storage.pop(self._id, None) + + def _append_finalizer( finalizer_storage: _FinalizerStorage, finalizer: _Finalizer -) -> Callable[[], None]: +) -> FinalizerHandle: finalizer_id = _FinalizerId() finalizer_storage[finalizer_id] = finalizer - - def remove_finalizer() -> None: - finalizer_storage.pop(finalizer_id, None) - - return remove_finalizer + return FinalizerHandle(finalizer_storage, finalizer_id, _ispytest=True) class SetupState: @@ -555,7 +576,7 @@ def setup(self, item: Item) -> None: def addfinalizer( self, finalizer: Callable[[], object], node: Node - ) -> Callable[[], None]: + ) -> FinalizerHandle: """Attach a finalizer to the given node. The node must be currently active in the stack. @@ -613,7 +634,7 @@ def fixture_setup(self, fixturedef: FixtureDef[object]) -> None: def fixture_addfinalizer( self, finalizer: Callable[[], object], fixturedef: FixtureDef[object] - ) -> Callable[[], None]: + ) -> FinalizerHandle: assert fixturedef in self._fixture_finalizers return _append_finalizer(self._fixture_finalizers[fixturedef], finalizer) diff --git a/src/pytest/__init__.py b/src/pytest/__init__.py index 3e6281ac388..63f6196e271 100644 --- a/src/pytest/__init__.py +++ b/src/pytest/__init__.py @@ -69,6 +69,7 @@ from _pytest.reports import CollectReport from _pytest.reports import TestReport from _pytest.runner import CallInfo +from _pytest.runner import FinalizerHandle from _pytest.stash import Stash from _pytest.stash import StashKey from _pytest.subtests import SubtestReport @@ -110,6 +111,7 @@ "ExceptionInfo", "ExitCode", "File", + "FinalizerHandle", "FixtureDef", "FixtureLookupError", "FixtureRequest", From e4e6d539bfb0bba5d3ae3e02068b51ac4fe8a9a1 Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Sun, 25 Jan 2026 02:06:08 +0300 Subject: [PATCH 23/24] Spelling --- src/_pytest/fixtures.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 384d6f238b8..72f34abb26b 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1187,10 +1187,10 @@ def __init__(self, request: FixtureRequest) -> None: self.cached_result = (request, [0], None) def addfinalizer(self, finalizer: Callable[[], object]) -> FinalizerHandle: - # RequestFixtureDef is not exposed to the user, e.g. it does not call - # pytest_fixture_setup and pytest_fixture_post_teardown. + # RequestFixtureDef is not exposed to the user, e.g. + # pytest_fixture_setup and pytest_fixture_post_teardown are not called. # Also RequestFixtureDef is not finalized properly, so if addfinalizer is - # somehow called, then the finalizer would never be called. + # somehow called, then the finalizer will never be called. assert False From 95a8e336105ee16b4e544502c928744922f0af92 Mon Sep 17 00:00:00 2001 From: Anton Zhilin Date: Sun, 25 Jan 2026 02:21:08 +0300 Subject: [PATCH 24/24] Add __slots__ to FinalizerHandle --- src/_pytest/runner.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py index d2fa7b19bdc..83f1239db72 100644 --- a/src/_pytest/runner.py +++ b/src/_pytest/runner.py @@ -450,6 +450,8 @@ class FinalizerHandle: .. versionadded:: 9.1 """ + __slots__ = ("_finalizer_storage", "_id") + def __init__( self, finalizer_storage: _FinalizerStorage,