Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/13773.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed the static fixture closure calculation to properly consider transitive dependencies requested by overridden fixtures.
15 changes: 12 additions & 3 deletions src/_pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
129 changes: 129 additions & 0 deletions testing/python/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When working on this I thought of this more complicated situation, which unfortunately still fails. It needs the analysis to include visibility, which is more difficult. But this change is still an improvement.

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)