Skip to content

Commit 3f831e6

Browse files
committed
fixtures: fix closure computation with indirect override chains
Continuation of 72ae3db. The previous fix was a minimal change to make the existing code at least consider fixtures overrides, and handles the common case (direct override chains), but still wasn't correct, as it didn't handle override chains involving an intermediary (not the overridden) fixture. To make this work, the algorithm needs to change. Now instead of a simple breadth-first search, we do a depth-first search, which more closely simulates the runtime behavior, and allows us the to check the "stack" of fixtures so we can use the correct fixture index (depth in the override chain). This is more expensive but should be OK. Refs #13773.
1 parent 8606fdc commit 3f831e6

File tree

2 files changed

+65
-26
lines changed

2 files changed

+65
-26
lines changed

src/_pytest/fixtures.py

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1634,29 +1634,44 @@ def getfixtureclosure(
16341634
fixturenames_closure = list(initialnames)
16351635

16361636
arg2fixturedefs: dict[str, Sequence[FixtureDef[Any]]] = {}
1637-
lastlen = -1
1638-
while lastlen != len(fixturenames_closure):
1639-
lastlen = len(fixturenames_closure)
1640-
for argname in fixturenames_closure:
1641-
if argname in ignore_args:
1642-
continue
1643-
if argname in arg2fixturedefs:
1644-
continue
1637+
1638+
# Track the index for each fixture name in the simulated stack.
1639+
# Needed for handling override chains correctly, similar to _get_active_fixturedef.
1640+
# Using negative indices: -1 is the most specific (last), -2 is second to last, etc.
1641+
current_indices: dict[str, int] = {}
1642+
1643+
def process_argname(argname: str) -> None:
1644+
# Optimization: already processed this argname.
1645+
if current_indices.get(argname) == -1:
1646+
return
1647+
1648+
if argname not in fixturenames_closure:
1649+
fixturenames_closure.append(argname)
1650+
1651+
if argname in ignore_args:
1652+
return
1653+
1654+
fixturedefs = arg2fixturedefs.get(argname)
1655+
if not fixturedefs:
16451656
fixturedefs = self.getfixturedefs(argname, parentnode)
1646-
if fixturedefs:
1647-
arg2fixturedefs[argname] = fixturedefs
1648-
1649-
# Add dependencies from this fixture.
1650-
# If it overrides a fixture with the same name and requests
1651-
# it, also add dependencies from the overridden fixtures in
1652-
# the chain. See also similar dealing in _get_active_fixturedef().
1653-
for fixturedef in reversed(fixturedefs): # pragma: no cover
1654-
for arg in fixturedef.argnames:
1655-
if arg not in fixturenames_closure:
1656-
fixturenames_closure.append(arg)
1657-
if argname not in fixturedef.argnames:
1658-
# Overrides, but doesn't request super.
1659-
break
1657+
if not fixturedefs:
1658+
# Fixture not defined or not visible (will error during runtest).
1659+
return
1660+
arg2fixturedefs[argname] = fixturedefs
1661+
1662+
index = current_indices.get(argname, -1)
1663+
if -index > len(fixturedefs):
1664+
# Exhausted the override chain (will error during runtest).
1665+
return
1666+
fixturedef = fixturedefs[index]
1667+
1668+
current_indices[argname] = index - 1
1669+
for dep in fixturedef.argnames:
1670+
process_argname(dep)
1671+
current_indices[argname] = index
1672+
1673+
for name in initialnames:
1674+
process_argname(name)
16601675

16611676
def sort_by_scope(arg_name: str) -> Scope:
16621677
try:

testing/python/fixtures.py

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5110,7 +5110,6 @@ def test_something(self, request, app):
51105110
result.assert_outcomes(passed=1)
51115111

51125112

5113-
@pytest.mark.xfail(reason="not currently handled correctly")
51145113
def test_fixture_closure_with_overrides_and_intermediary(pytester: Pytester) -> None:
51155114
"""Test that an item's static fixture closure properly includes transitive
51165115
dependencies through overridden fixtures (#13773).
@@ -5255,15 +5254,14 @@ def session(db): pass
52555254
def app(user, session): pass
52565255
52575256
def test_diamond_deps(request, app):
5258-
assert request.node.fixturenames == ["request", "app", "user", "session", "db"]
5259-
assert request.fixturenames == ["request", "app", "user", "session", "db"]
5257+
assert request.node.fixturenames == ["request", "app", "user", "db", "session"]
5258+
assert request.fixturenames == ["request", "app", "user", "db", "session"]
52605259
"""
52615260
)
52625261
result = pytester.runpytest("-v")
52635262
result.assert_outcomes(passed=1)
52645263

52655264

5266-
@pytest.mark.xfail(reason="not currently handled correctly")
52675265
def test_fixture_closure_with_complex_override_and_shared_deps(
52685266
pytester: Pytester,
52695267
) -> None:
@@ -5305,3 +5303,29 @@ def test_shared_deps(self, request, app):
53055303
)
53065304
result = pytester.runpytest("-v")
53075305
result.assert_outcomes(passed=1)
5306+
5307+
5308+
def test_fixture_closure_with_parametrize_ignore(pytester: Pytester) -> None:
5309+
"""Test that getfixtureclosure properly handles parametrization argnames
5310+
which override a fixture."""
5311+
pytester.makepyfile(
5312+
"""
5313+
import pytest
5314+
5315+
@pytest.fixture
5316+
def fix1(fix2): pass
5317+
5318+
@pytest.fixture
5319+
def fix2(fix3): pass
5320+
5321+
@pytest.fixture
5322+
def fix3(): pass
5323+
5324+
@pytest.mark.parametrize('fix2', ['2'])
5325+
def test_it(request, fix1):
5326+
assert request.node.fixturenames == ["request", "fix1", "fix2"]
5327+
assert request.fixturenames == ["request", "fix1", "fix2"]
5328+
"""
5329+
)
5330+
result = pytester.runpytest("-v")
5331+
result.assert_outcomes(passed=1)

0 commit comments

Comments
 (0)