From 72ae3dbf9608a70df683d081cf53146a8d79f1ea Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 1 Oct 2025 11:36:00 +0300 Subject: [PATCH] fixtures: fix fixture closure calculation to properly consider overridden fixtures Fix #13773. --- changelog/13773.bugfix.rst | 1 + src/_pytest/fixtures.py | 15 ++++- testing/python/fixtures.py | 129 +++++++++++++++++++++++++++++++++++++ 3 files changed, 142 insertions(+), 3 deletions(-) create mode 100644 changelog/13773.bugfix.rst diff --git a/changelog/13773.bugfix.rst b/changelog/13773.bugfix.rst new file mode 100644 index 00000000000..e3a9ff4b7a1 --- /dev/null +++ b/changelog/13773.bugfix.rst @@ -0,0 +1 @@ +Fixed the static fixture closure calculation to properly consider transitive dependencies requested by overridden fixtures. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 91f1b3a67f6..b0ce620c13a 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1645,9 +1645,18 @@ def getfixtureclosure( fixturedefs = self.getfixturedefs(argname, parentnode) if fixturedefs: arg2fixturedefs[argname] = fixturedefs - for arg in fixturedefs[-1].argnames: - if arg not in fixturenames_closure: - fixturenames_closure.append(arg) + + # 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 def sort_by_scope(arg_name: str) -> Scope: try: diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 8b97d35c21e..d673eac7742 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -5069,3 +5069,132 @@ def test_method(self, /, fix): ) result = pytester.runpytest() result.assert_outcomes(passed=1) + + +def test_fixture_closure_with_overrides(pytester: Pytester) -> None: + """Test that an item's static fixture closure properly includes transitive + dependencies through overridden fixtures (#13773).""" + 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 + + def test_something(self, request, app): + # 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) + + +@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). + + A more complicated case than test_fixture_closure_with_overrides, adds an + intermediary so the override chain is not direct. + """ + pytester.makeconftest( + """ + import pytest + + @pytest.fixture + def db(): pass + + @pytest.fixture + def app(db): pass + + @pytest.fixture + def intermediate(app): pass + """ + ) + pytester.makepyfile( + """ + import pytest + + # Overrides conftest-level `app` and requests it. + @pytest.fixture + def app(intermediate): pass + + class TestClass: + # Overrides module-level `app` and requests it. + @pytest.fixture + def app(self, app): pass + + def test_something(self, request, app): + # 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). + + A more complicated case than test_fixture_closure_with_overrides, one of the + fixtures in the chain doesn't call its super, so it shouldn't be included. + """ + pytester.makeconftest( + """ + import pytest + + @pytest.fixture + def db(): pass + + @pytest.fixture + def app(db): pass + """ + ) + pytester.makepyfile( + """ + import pytest + + # Overrides conftest-level `app` and *doesn't* request it. + @pytest.fixture + def app(): pass + + class TestClass: + # Overrides module-level `app` and requests it. + @pytest.fixture + def app(self, app): pass + + def test_something(self, request, app): + # Both dynamic and static fixture closures should include 'db'. + assert 'db' not in request.fixturenames + assert 'db' not 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)