Skip to content

Commit 9ef765f

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 9ef765f

File tree

2 files changed

+39
-26
lines changed

2 files changed

+39
-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 in ignore_args:
1649+
return
1650+
1651+
if argname not in fixturenames_closure:
1652+
fixturenames_closure.append(argname)
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: 2 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:

0 commit comments

Comments
 (0)