From 5d674ca2f246f7ff612afb28ae978d07e90ee483 Mon Sep 17 00:00:00 2001 From: Sylvain MARIE Date: Wed, 8 Apr 2026 12:21:16 +0300 Subject: [PATCH 1/6] testing/python/fixtures: add xfail test for static fixture closure with overrides and parametrization Based on existing tests, just adding the parametrization. Refs #14248. --- testing/python/fixtures.py | 42 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 38d219a547b..3f4b745d586 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -5451,6 +5451,48 @@ def test_something(self, request, app): result.assert_outcomes(passed=1) +@pytest.mark.xfail(reason="#14248 still not fixed") +def test_fixture_closure_with_overrides_and_parametrization(pytester: Pytester) -> None: + """Test that an item's static fixture closure properly includes transitive + dependencies through overridden fixtures (#13773) when also including + parametrization (#14248).""" + pytester.makeconftest( + """ + import pytest + + @pytest.fixture + def db(): pass + + @pytest.fixture + def app(db): pass + """ + ) + pytester.makepyfile( + """ + import pytest + + # Overrides conftest-level `app` and requests it. + @pytest.fixture + def app(app): pass + + class TestClass: + # Overrides module-level `app` and requests it. + @pytest.fixture + def app(self, app): pass + + @pytest.mark.parametrize("a", [1]) + def test_something(self, request, app, a): + # Both dynamic and static fixture closures should include 'db'. + assert 'db' in request.fixturenames + assert 'db' in request.node.fixturenames + # No dynamic dependencies, should be equal. + assert set(request.fixturenames) == set(request.node.fixturenames) + """ + ) + result = pytester.runpytest("-v") + result.assert_outcomes(passed=1) + + def test_fixture_closure_with_broken_override_chain(pytester: Pytester) -> None: """Test that an item's static fixture closure properly includes transitive dependencies through overridden fixtures (#13773). From 702b1a81e7ef2c853fc18db80dc3dda6330a235f Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 8 Apr 2026 12:52:56 +0300 Subject: [PATCH 2/6] testing/python/fixtures: modernize test_parametrization_setup_teardown_ordering No functional changes, just making it easier to follow. --- testing/python/fixtures.py | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 3f4b745d586..716dc230b94 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -2424,30 +2424,43 @@ def test_parametrization_setup_teardown_ordering(self, pytester: Pytester) -> No pytester.makepyfile( """ import pytest + values = [] + def pytest_generate_tests(metafunc): if metafunc.cls is None: assert metafunc.function is test_finish if metafunc.cls is not None: metafunc.parametrize("item", [1,2], scope="class") - class TestClass(object): + + class TestClass: @pytest.fixture(scope="class", autouse=True) - def addteardown(self, item, request): + def setup_teardown(self, item): values.append("setup-%d" % item) - request.addfinalizer(lambda: values.append("teardown-%d" % item)) + yield + values.append("teardown-%d" % item) + def test_step1(self, item): values.append("step1-%d" % item) + def test_step2(self, item): values.append("step2-%d" % item) def test_finish(): - print(values) - assert values == ["setup-1", "step1-1", "step2-1", "teardown-1", - "setup-2", "step1-2", "step2-2", "teardown-2",] - """ - ) - reprec = pytester.inline_run("-s") - reprec.assertoutcome(passed=5) + assert values == [ + "setup-1", + "step1-1", + "step2-1", + "teardown-1", + "setup-2", + "step1-2", + "step2-2", + "teardown-2", + ] + """ + ) + result = pytester.inline_run("-vv") + result.assertoutcome(passed=5) def test_ordering_autouse_before_explicit(self, pytester: Pytester) -> None: pytester.makepyfile( From c4e38858817b02b33aec103e049b16630449c46f Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 8 Apr 2026 15:15:13 +0300 Subject: [PATCH 3/6] testing/python/fixtures: rewrite test_func_closure_with_native_fixtures without monkeypatching Seems better to avoid this monkeypatch-pytest-to-smuggle-info-out-of-pytester pattern. --- testing/python/fixtures.py | 66 ++++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 716dc230b94..a14ff4d6a07 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4497,62 +4497,74 @@ def test_func(m1): request = TopRequest(items[0], _ispytest=True) assert request.fixturenames == "m1 f1".split() - def test_func_closure_with_native_fixtures( - self, pytester: Pytester, monkeypatch: MonkeyPatch - ) -> None: - """Sanity check that verifies the order returned by the closures and the actual fixture execution order: - The execution order may differ because of fixture inter-dependencies. - """ - monkeypatch.setattr(pytest, "FIXTURE_ORDER", [], raising=False) + def test_func_closure_with_native_fixtures(self, pytester: Pytester) -> None: + """Sanity check that verifies the order returned by the closures and the + actual fixture execution order: the execution order may differ because + of fixture inter-dependencies.""" pytester.makepyfile( """ import pytest - FIXTURE_ORDER = pytest.FIXTURE_ORDER + fixture_order = [] @pytest.fixture(scope="session") def s1(): - FIXTURE_ORDER.append('s1') + fixture_order.append("s1") @pytest.fixture(scope="package") def p1(): - FIXTURE_ORDER.append('p1') + fixture_order.append("p1") @pytest.fixture(scope="module") def m1(): - FIXTURE_ORDER.append('m1') + fixture_order.append("m1") - @pytest.fixture(scope='session') + @pytest.fixture(scope="session") def my_tmp_path_factory(): - FIXTURE_ORDER.append('my_tmp_path_factory') + fixture_order.append("my_tmp_path_factory") @pytest.fixture def my_tmp_path(my_tmp_path_factory): - FIXTURE_ORDER.append('my_tmp_path') + fixture_order.append("my_tmp_path") @pytest.fixture def f1(my_tmp_path): - FIXTURE_ORDER.append('f1') + fixture_order.append("f1") @pytest.fixture def f2(): - FIXTURE_ORDER.append('f2') - - def test_foo(f1, p1, m1, f2, s1): pass + fixture_order.append("f2") + + def test_foo(f1, p1, m1, f2, s1): + # Actual fixture execution differs from static order: dependent + # fixtures must be created first ("my_tmp_path"). + assert fixture_order == [ + "s1", + "my_tmp_path_factory", + "p1", + "m1", + "my_tmp_path", + "f1", + "f2", + ] """ ) items, _ = pytester.inline_genitems() assert isinstance(items[0], Function) request = TopRequest(items[0], _ispytest=True) - # order of fixtures based on their scope and position in the parameter list - assert ( - request.fixturenames - == "s1 my_tmp_path_factory p1 m1 f1 f2 my_tmp_path".split() - ) - pytester.runpytest() - # actual fixture execution differs: dependent fixtures must be created first ("my_tmp_path") - FIXTURE_ORDER = pytest.FIXTURE_ORDER # type: ignore[attr-defined] - assert FIXTURE_ORDER == "s1 my_tmp_path_factory p1 m1 my_tmp_path f1 f2".split() + # Static order of fixtures based on their scope and position in the + # parameter list. + assert request.fixturenames == [ + "s1", + "my_tmp_path_factory", + "p1", + "m1", + "f1", + "f2", + "my_tmp_path", + ] + result = pytester.runpytest("-vv") + result.assert_outcomes(passed=1) def test_func_closure_module(self, pytester: Pytester) -> None: pytester.makepyfile( From 920b9c44785662e1f073190c9848d910cd3cccf7 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 8 Apr 2026 13:18:48 +0300 Subject: [PATCH 4/6] fixtures: fix parametrization losing (static) transitive dependencies from overridden fixtures As the xfail'ed test `test_fixture_closure_with_overrides_and_parametrization` demonstrates, adding any direct parametrization causes the statically computed fixture closure to omit transitive dependencies from overridden (but still requested) fixtures. (The dynamic algorithm does get it right). The `prune_dependency_tree()` function did not account for overridden fixtures properly (only checked `-1`). Make it share the core recursive DFS traversal used by `getfixtureclosure()`, which already fixed a similar problem (#13789). An alternative solution is to replace `prune_dependency_tree` wholesale with another call to `getfixtureclosure`, as proposed in PR #11243, however it's somewhat harder to get working, so let's go with this solution for now. Fix #14248 --- changelog/14248.bugfix.rst | 1 + src/_pytest/fixtures.py | 92 ++++++++++++++++++++++++-------------- testing/python/fixtures.py | 1 - 3 files changed, 59 insertions(+), 35 deletions(-) create mode 100644 changelog/14248.bugfix.rst diff --git a/changelog/14248.bugfix.rst b/changelog/14248.bugfix.rst new file mode 100644 index 00000000000..1c8563fa954 --- /dev/null +++ b/changelog/14248.bugfix.rst @@ -0,0 +1 @@ +Fixed direct parametrization causing the static fixture closure (as reflected in :data:`request.fixturenames `) to omit fixtures that are requested transitively from overridden fixtures. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index f667c40ea78..c25c590b7c3 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -300,6 +300,49 @@ def reorder_items_atscope( return items_done +def traverse_fixture_closure( + initialnames: Iterable[str], + *, + getfixturedefs: Callable[[str], Sequence[FixtureDef[Any]] | None], +) -> Iterator[str]: + """Statically traverse the fixture dependency closure in DFS order starting + from initialnames, yielding all requested fixture names (argnames). + + argnames may be yielded more than once. + """ + # Track the index for each fixture name in the simulated stack. + # Needed for handling override chains correctly, similar to + # FixtureRequest._get_active_fixturedef. + # Using negative indices: -1 is the most specific (last), -2 is second to + # last, etc. + current_indices: dict[str, int] = {} + + def process_argname(argname: str) -> Iterator[str]: + # Optimization: already processed this argname. + if current_indices.get(argname) == -1: + return + + yield argname + + fixturedefs = getfixturedefs(argname) + if not fixturedefs: + return + + index = current_indices.get(argname, -1) + if -index > len(fixturedefs): + # Exhausted the override chain (will error during runtest). + return + fixturedef = fixturedefs[index] + + current_indices[argname] = index - 1 + for dep in fixturedef.argnames: + yield from process_argname(dep) + current_indices[argname] = index + + for argname in initialnames: + yield from process_argname(argname) + + @dataclasses.dataclass(frozen=True) class FuncFixtureInfo: """Fixture-related information for a fixture-requesting item (e.g. test @@ -343,13 +386,12 @@ def prune_dependency_tree(self) -> None: of argnames may get reduced. """ closure: set[str] = set() - working_set = set(self.initialnames) - while working_set: - argname = working_set.pop() - if argname not in closure and argname in self.names_closure: + for argname in traverse_fixture_closure( + self.initialnames, + getfixturedefs=self.name2fixturedefs.get, + ): + if argname in self.names_closure: closure.add(argname) - if argname in self.name2fixturedefs: - working_set.update(self.name2fixturedefs[argname][-1].argnames) self.names_closure[:] = sorted(closure, key=self.names_closure.index) @@ -1708,43 +1750,25 @@ def getfixtureclosure( arg2fixturedefs: dict[str, Sequence[FixtureDef[Any]]] = {} - # Track the index for each fixture name in the simulated stack. - # Needed for handling override chains correctly, similar to _get_active_fixturedef. - # Using negative indices: -1 is the most specific (last), -2 is second to last, etc. - current_indices: dict[str, int] = {} - - def process_argname(argname: str) -> None: - # Optimization: already processed this argname. - if current_indices.get(argname) == -1: - return - - if argname not in fixturenames_closure: - fixturenames_closure.append(argname) - + def getfixturedefs(argname: str) -> Sequence[FixtureDef[Any]] | None: if argname in ignore_args: - return + return None fixturedefs = arg2fixturedefs.get(argname) if not fixturedefs: fixturedefs = self.getfixturedefs(argname, parentnode) if not fixturedefs: # Fixture not defined or not visible (will error during runtest). - return + return None arg2fixturedefs[argname] = fixturedefs + return fixturedefs - index = current_indices.get(argname, -1) - if -index > len(fixturedefs): - # Exhausted the override chain (will error during runtest). - return - fixturedef = fixturedefs[index] - - current_indices[argname] = index - 1 - for dep in fixturedef.argnames: - process_argname(dep) - current_indices[argname] = index - - for name in initialnames: - process_argname(name) + for argname in traverse_fixture_closure( + initialnames, + getfixturedefs=getfixturedefs, + ): + if argname not in fixturenames_closure: + fixturenames_closure.append(argname) def sort_by_scope(arg_name: str) -> Scope: try: diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index a14ff4d6a07..d76fd528bd8 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -5476,7 +5476,6 @@ def test_something(self, request, app): result.assert_outcomes(passed=1) -@pytest.mark.xfail(reason="#14248 still not fixed") def test_fixture_closure_with_overrides_and_parametrization(pytester: Pytester) -> None: """Test that an item's static fixture closure properly includes transitive dependencies through overridden fixtures (#13773) when also including From 8e53c6bf531c3afe8fe41687ae339cf8727c7e07 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 8 Apr 2026 14:42:32 +0300 Subject: [PATCH 5/6] fixtures: optimize a bit around `traverse_fixture_closure` Didn't include this in the previous commit, in case it causes regressions. --- src/_pytest/fixtures.py | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index c25c590b7c3..f7f0be96649 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -308,7 +308,7 @@ def traverse_fixture_closure( """Statically traverse the fixture dependency closure in DFS order starting from initialnames, yielding all requested fixture names (argnames). - argnames may be yielded more than once. + Each argname is only yielded once. """ # Track the index for each fixture name in the simulated stack. # Needed for handling override chains correctly, similar to @@ -318,11 +318,16 @@ def traverse_fixture_closure( current_indices: dict[str, int] = {} def process_argname(argname: str) -> Iterator[str]: + index = current_indices.get(argname) + # Optimization: already processed this argname. - if current_indices.get(argname) == -1: + if index == -1: return - yield argname + # Only yield each argname once. + if index is None: + yield argname + current_indices[argname] = -1 fixturedefs = getfixturedefs(argname) if not fixturedefs: @@ -385,15 +390,13 @@ def prune_dependency_tree(self) -> None: tree. In this way the dependency tree can get pruned, and the closure of argnames may get reduced. """ - closure: set[str] = set() - for argname in traverse_fixture_closure( - self.initialnames, - getfixturedefs=self.name2fixturedefs.get, - ): - if argname in self.names_closure: - closure.add(argname) - - self.names_closure[:] = sorted(closure, key=self.names_closure.index) + closure = set( + traverse_fixture_closure( + self.initialnames, + getfixturedefs=self.name2fixturedefs.get, + ) + ) + self.names_closure[:] = (name for name in self.names_closure if name in closure) class FixtureRequest(abc.ABC): @@ -1746,8 +1749,6 @@ def getfixtureclosure( # to re-discover fixturedefs again for each fixturename # (discovering matching fixtures for a given name/node is expensive). - fixturenames_closure = list(initialnames) - arg2fixturedefs: dict[str, Sequence[FixtureDef[Any]]] = {} def getfixturedefs(argname: str) -> Sequence[FixtureDef[Any]] | None: @@ -1763,11 +1764,12 @@ def getfixturedefs(argname: str) -> Sequence[FixtureDef[Any]] | None: arg2fixturedefs[argname] = fixturedefs return fixturedefs + fixturenames_closure = list(initialnames) for argname in traverse_fixture_closure( initialnames, getfixturedefs=getfixturedefs, ): - if argname not in fixturenames_closure: + if argname not in initialnames: fixturenames_closure.append(argname) def sort_by_scope(arg_name: str) -> Scope: @@ -1779,6 +1781,7 @@ def sort_by_scope(arg_name: str) -> Scope: return fixturedefs[-1]._scope fixturenames_closure.sort(key=sort_by_scope, reverse=True) + return fixturenames_closure, arg2fixturedefs def pytest_generate_tests(self, metafunc: Metafunc) -> None: From 4722f2472dfb2dad48420cc22e24cd06256ac7bc Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 8 Apr 2026 14:55:18 +0300 Subject: [PATCH 6/6] fixtures: change static fixture closure order fully to DFS (I am talking here about `names_closure`/`fixturenames`.) Before 72ae3dbf9608a70df683d081cf53146a8d79f1ea, the static traversal order was BFS (unlike the dynamic order which is DFS). So it was natural that `initialnames` was always at the beginning of the closure, since it consists the initial working set. When we changed to DFS, we kept this. But it's a bit weird, now the initialnames are in BFS, but all transitive dependencies are in DFS. Remove this special case, making both the static and dynamic order fully DFS. --- src/_pytest/fixtures.py | 17 ++++++++--------- .../fixtures/test_getfixturevalue_dynamic.py | 2 +- testing/python/fixtures.py | 10 +++++----- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index f7f0be96649..7041d971592 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1764,14 +1764,6 @@ def getfixturedefs(argname: str) -> Sequence[FixtureDef[Any]] | None: arg2fixturedefs[argname] = fixturedefs return fixturedefs - fixturenames_closure = list(initialnames) - for argname in traverse_fixture_closure( - initialnames, - getfixturedefs=getfixturedefs, - ): - if argname not in initialnames: - fixturenames_closure.append(argname) - def sort_by_scope(arg_name: str) -> Scope: try: fixturedefs = arg2fixturedefs[arg_name] @@ -1780,7 +1772,14 @@ def sort_by_scope(arg_name: str) -> Scope: else: return fixturedefs[-1]._scope - fixturenames_closure.sort(key=sort_by_scope, reverse=True) + fixturenames_closure = sorted( + traverse_fixture_closure( + initialnames, + getfixturedefs=getfixturedefs, + ), + key=sort_by_scope, + reverse=True, + ) return fixturenames_closure, arg2fixturedefs diff --git a/testing/example_scripts/fixtures/test_getfixturevalue_dynamic.py b/testing/example_scripts/fixtures/test_getfixturevalue_dynamic.py index 0559905cea4..5ea5e6ace79 100644 --- a/testing/example_scripts/fixtures/test_getfixturevalue_dynamic.py +++ b/testing/example_scripts/fixtures/test_getfixturevalue_dynamic.py @@ -20,4 +20,4 @@ def b(a): def test(b, request): - assert request.fixturenames == ["b", "request", "a", "dynamic"] + assert request.fixturenames == ["b", "a", "request", "dynamic"] diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index d76fd528bd8..d02cd28c452 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -1178,7 +1178,7 @@ def test_function(request, farg): def test_request_fixturenames_dynamic_fixture(self, pytester: Pytester) -> None: """Regression test for #3057""" pytester.copy_example("fixtures/test_getfixturevalue_dynamic.py") - result = pytester.runpytest() + result = pytester.runpytest("-vv") result.stdout.fnmatch_lines(["*1 passed*"]) def test_setupdecorator_and_xunit(self, pytester: Pytester) -> None: @@ -4539,8 +4539,8 @@ def test_foo(f1, p1, m1, f2, s1): # Actual fixture execution differs from static order: dependent # fixtures must be created first ("my_tmp_path"). assert fixture_order == [ - "s1", "my_tmp_path_factory", + "s1", "p1", "m1", "my_tmp_path", @@ -4555,13 +4555,13 @@ def test_foo(f1, p1, m1, f2, s1): # Static order of fixtures based on their scope and position in the # parameter list. assert request.fixturenames == [ - "s1", "my_tmp_path_factory", + "s1", "p1", "m1", "f1", - "f2", "my_tmp_path", + "f2", ] result = pytester.runpytest("-vv") result.assert_outcomes(passed=1) @@ -5593,7 +5593,7 @@ def test_circular_deps(fix_a, fix_x): ) items, _hookrec = pytester.inline_genitems() assert isinstance(items[0], Function) - assert items[0].fixturenames == ["fix_a", "fix_x", "fix_b", "fix_y", "fix_z"] + assert items[0].fixturenames == ["fix_a", "fix_b", "fix_x", "fix_y", "fix_z"] def test_fixture_closure_handles_diamond_dependencies(pytester: Pytester) -> None: