Skip to content

Commit bd45ccd

Browse files
committed
fixtures: avoid mutable arg2index state in favor of looking up the request chain
pytest allows a fixture to request its own name (directly or indirectly), in which case the fixture with the same name but one level up is used. To know which fixture should be used next, pytest keeps a mutable item-global dict `_arg2index` which maintains this state. This is not great: - Mutable state like this is hard to understand and reason about. - It is conceptually buggy; the indexing is global (e.g. if requesting `fix1` and `fix2`, the indexing is shared between them), but actually different branches of the subrequest tree should not affect each other. This is not an issue in practice because pytest keeps a cache of the fixturedefs it resolved anyway (`_fixture_defs`), but if the cache is removed it becomes evident. Instead of the `_arg2index` state, count how many `argname`s deep we are in the subrequest tree ("the fixture stack") and use that for the index. This way, no global mutable state and the logic is very localized and easier to understand. This is slower, however fixture stacks should not be so deep that this matters much, I hope.
1 parent c83c1c4 commit bd45ccd

File tree

1 file changed

+16
-16
lines changed

1 file changed

+16
-16
lines changed

src/_pytest/fixtures.py

+16-16
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,6 @@ def __init__(
343343
pyfuncitem: "Function",
344344
fixturename: Optional[str],
345345
arg2fixturedefs: Dict[str, Sequence["FixtureDef[Any]"]],
346-
arg2index: Dict[str, int],
347346
fixture_defs: Dict[str, "FixtureDef[Any]"],
348347
*,
349348
_ispytest: bool = False,
@@ -357,16 +356,6 @@ def __init__(
357356
# collection. Dynamically requested fixtures (using
358357
# `request.getfixturevalue("foo")`) are added dynamically.
359358
self._arg2fixturedefs: Final = arg2fixturedefs
360-
# A fixture may override another fixture with the same name, e.g. a fixture
361-
# in a module can override a fixture in a conftest, a fixture in a class can
362-
# override a fixture in the module, and so on.
363-
# An overriding fixture can request its own name; in this case it gets
364-
# the value of the fixture it overrides, one level up.
365-
# The _arg2index state keeps the current depth in the overriding chain.
366-
# The fixturedefs list in _arg2fixturedefs for a given name is ordered from
367-
# furthest to closest, so we use negative indexing -1, -2, ... to go from
368-
# last to first.
369-
self._arg2index: Final = arg2index
370359
# The evaluated argnames so far, mapping to the FixtureDef they resolved
371360
# to.
372361
self._fixture_defs: Final = fixture_defs
@@ -424,11 +413,24 @@ def _getnextfixturedef(self, argname: str) -> "FixtureDef[Any]":
424413
# The are no fixtures with this name applicable for the function.
425414
if not fixturedefs:
426415
raise FixtureLookupError(argname, self)
427-
index = self._arg2index.get(argname, 0) - 1
428-
# The fixture requested its own name, but no remaining to override.
416+
417+
# A fixture may override another fixture with the same name, e.g. a
418+
# fixture in a module can override a fixture in a conftest, a fixture in
419+
# a class can override a fixture in the module, and so on.
420+
# An overriding fixture can request its own name (possibly indirectly);
421+
# in this case it gets the value of the fixture it overrides, one level
422+
# up.
423+
# Check how many `argname`s deep we are, and take the next one.
424+
# `fixturedefs` is sorted from furthest to closest, so use negative
425+
# indexing to go in reverse.
426+
index = -1
427+
for request in self._iter_chain():
428+
if request.fixturename == argname:
429+
index -= 1
430+
# If already consumed all of the available levels, fail.
429431
if -index > len(fixturedefs):
430432
raise FixtureLookupError(argname, self)
431-
self._arg2index[argname] = index
433+
432434
return fixturedefs[index]
433435

434436
@property
@@ -660,7 +662,6 @@ def __init__(self, pyfuncitem: "Function", *, _ispytest: bool = False) -> None:
660662
fixturename=None,
661663
pyfuncitem=pyfuncitem,
662664
arg2fixturedefs=pyfuncitem._fixtureinfo.name2fixturedefs.copy(),
663-
arg2index={},
664665
fixture_defs={},
665666
_ispytest=_ispytest,
666667
)
@@ -706,7 +707,6 @@ def __init__(
706707
fixturename=fixturedef.argname,
707708
fixture_defs=request._fixture_defs,
708709
arg2fixturedefs=request._arg2fixturedefs,
709-
arg2index=request._arg2index,
710710
_ispytest=_ispytest,
711711
)
712712
self._parent_request: Final[FixtureRequest] = request

0 commit comments

Comments
 (0)