diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index b0ce620c13a..1f583a610f0 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1634,29 +1634,44 @@ def getfixtureclosure( fixturenames_closure = list(initialnames) arg2fixturedefs: dict[str, Sequence[FixtureDef[Any]]] = {} - lastlen = -1 - while lastlen != len(fixturenames_closure): - lastlen = len(fixturenames_closure) - for argname in fixturenames_closure: - if argname in ignore_args: - continue - if argname in arg2fixturedefs: - continue + + # 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) + + if argname in ignore_args: + return + + fixturedefs = arg2fixturedefs.get(argname) + if not fixturedefs: fixturedefs = self.getfixturedefs(argname, parentnode) - if fixturedefs: - arg2fixturedefs[argname] = fixturedefs - - # Add dependencies from this fixture. - # If it overrides a fixture with the same name and requests - # it, also add dependencies from the overridden fixtures in - # the chain. See also similar dealing in _get_active_fixturedef(). - for fixturedef in reversed(fixturedefs): # pragma: no cover - for arg in fixturedef.argnames: - if arg not in fixturenames_closure: - fixturenames_closure.append(arg) - if argname not in fixturedef.argnames: - # Overrides, but doesn't request super. - break + if not fixturedefs: + # Fixture not defined or not visible (will error during runtest). + return + arg2fixturedefs[argname] = 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) def sort_by_scope(arg_name: str) -> Scope: try: diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index d673eac7742..ce0e5f87050 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -5110,7 +5110,6 @@ def test_something(self, request, app): result.assert_outcomes(passed=1) -@pytest.mark.xfail(reason="not currently handled correctly") def test_fixture_closure_with_overrides_and_intermediary(pytester: Pytester) -> None: """Test that an item's static fixture closure properly includes transitive dependencies through overridden fixtures (#13773). @@ -5198,3 +5197,135 @@ def test_something(self, request, app): ) result = pytester.runpytest("-v") result.assert_outcomes(passed=1) + + +def test_fixture_closure_handles_circular_dependencies(pytester: Pytester) -> None: + """Test that getfixtureclosure properly handles circular dependencies. + + The test will error in the runtest phase due to the fixture loop, + but the closure computation still completes. + """ + pytester.makepyfile( + """ + import pytest + + # Direct circular dependency. + @pytest.fixture + def fix_a(fix_b): pass + + @pytest.fixture + def fix_b(fix_a): pass + + # Indirect circular dependency through multiple fixtures. + @pytest.fixture + def fix_x(fix_y): pass + + @pytest.fixture + def fix_y(fix_z): pass + + @pytest.fixture + def fix_z(fix_x): pass + + def test_circular_deps(fix_a, fix_x): + pass + """ + ) + items, _hookrec = pytester.inline_genitems() + assert isinstance(items[0], Function) + assert items[0].fixturenames == ["fix_a", "fix_x", "fix_b", "fix_y", "fix_z"] + + +def test_fixture_closure_handles_diamond_dependencies(pytester: Pytester) -> None: + """Test that getfixtureclosure properly handles diamond dependencies.""" + pytester.makepyfile( + """ + import pytest + + @pytest.fixture + def db(): pass + + @pytest.fixture + def user(db): pass + + @pytest.fixture + def session(db): pass + + @pytest.fixture + def app(user, session): pass + + def test_diamond_deps(request, app): + assert request.node.fixturenames == ["request", "app", "user", "db", "session"] + assert request.fixturenames == ["request", "app", "user", "db", "session"] + """ + ) + result = pytester.runpytest("-v") + result.assert_outcomes(passed=1) + + +def test_fixture_closure_with_complex_override_and_shared_deps( + pytester: Pytester, +) -> None: + """Test that shared dependencies in override chains are processed only once.""" + pytester.makeconftest( + """ + import pytest + + @pytest.fixture + def db(): pass + + @pytest.fixture + def cache(): pass + + @pytest.fixture + def settings(): pass + + @pytest.fixture + def app(db, cache, settings): pass + """ + ) + pytester.makepyfile( + """ + import pytest + + # Override app, but also directly use cache and settings. + # This creates multiple paths to the same fixtures. + @pytest.fixture + def app(app, cache, settings): pass + + class TestClass: + # Another override that uses both app and cache. + @pytest.fixture + def app(self, app, cache): pass + + def test_shared_deps(self, request, app): + assert request.node.fixturenames == ["request", "app", "db", "cache", "settings"] + """ + ) + result = pytester.runpytest("-v") + result.assert_outcomes(passed=1) + + +def test_fixture_closure_with_parametrize_ignore(pytester: Pytester) -> None: + """Test that getfixtureclosure properly handles parametrization argnames + which override a fixture.""" + pytester.makepyfile( + """ + import pytest + + @pytest.fixture + def fix1(fix2): pass + + @pytest.fixture + def fix2(fix3): pass + + @pytest.fixture + def fix3(): pass + + @pytest.mark.parametrize('fix2', ['2']) + def test_it(request, fix1): + assert request.node.fixturenames == ["request", "fix1", "fix2"] + assert request.fixturenames == ["request", "fix1", "fix2"] + """ + ) + result = pytester.runpytest("-v") + result.assert_outcomes(passed=1)