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.
This commit is contained in:
Ran Benita 2024-02-27 10:37:40 +02:00
parent c83c1c4bda
commit bd45ccd2ca
1 changed files with 16 additions and 16 deletions

View File

@ -343,7 +343,6 @@ class FixtureRequest(abc.ABC):
pyfuncitem: "Function", pyfuncitem: "Function",
fixturename: Optional[str], fixturename: Optional[str],
arg2fixturedefs: Dict[str, Sequence["FixtureDef[Any]"]], arg2fixturedefs: Dict[str, Sequence["FixtureDef[Any]"]],
arg2index: Dict[str, int],
fixture_defs: Dict[str, "FixtureDef[Any]"], fixture_defs: Dict[str, "FixtureDef[Any]"],
*, *,
_ispytest: bool = False, _ispytest: bool = False,
@ -357,16 +356,6 @@ class FixtureRequest(abc.ABC):
# collection. Dynamically requested fixtures (using # collection. Dynamically requested fixtures (using
# `request.getfixturevalue("foo")`) are added dynamically. # `request.getfixturevalue("foo")`) are added dynamically.
self._arg2fixturedefs: Final = arg2fixturedefs self._arg2fixturedefs: Final = arg2fixturedefs
# A fixture may override another fixture with the same name, e.g. a fixture
# in a module can override a fixture in a conftest, a fixture in a class can
# override a fixture in the module, and so on.
# An overriding fixture can request its own name; in this case it gets
# the value of the fixture it overrides, one level up.
# The _arg2index state keeps the current depth in the overriding chain.
# The fixturedefs list in _arg2fixturedefs for a given name is ordered from
# furthest to closest, so we use negative indexing -1, -2, ... to go from
# last to first.
self._arg2index: Final = arg2index
# The evaluated argnames so far, mapping to the FixtureDef they resolved # The evaluated argnames so far, mapping to the FixtureDef they resolved
# to. # to.
self._fixture_defs: Final = fixture_defs self._fixture_defs: Final = fixture_defs
@ -424,11 +413,24 @@ class FixtureRequest(abc.ABC):
# The are no fixtures with this name applicable for the function. # The are no fixtures with this name applicable for the function.
if not fixturedefs: if not fixturedefs:
raise FixtureLookupError(argname, self) raise FixtureLookupError(argname, self)
index = self._arg2index.get(argname, 0) - 1
# The fixture requested its own name, but no remaining to override. # A fixture may override another fixture with the same name, e.g. a
# fixture in a module can override a fixture in a conftest, a fixture in
# a class can override a fixture in the module, and so on.
# An overriding fixture can request its own name (possibly indirectly);
# in this case it gets the value of the fixture it overrides, one level
# up.
# Check how many `argname`s deep we are, and take the next one.
# `fixturedefs` is sorted from furthest to closest, so use negative
# indexing to go in reverse.
index = -1
for request in self._iter_chain():
if request.fixturename == argname:
index -= 1
# If already consumed all of the available levels, fail.
if -index > len(fixturedefs): if -index > len(fixturedefs):
raise FixtureLookupError(argname, self) raise FixtureLookupError(argname, self)
self._arg2index[argname] = index
return fixturedefs[index] return fixturedefs[index]
@property @property
@ -660,7 +662,6 @@ class TopRequest(FixtureRequest):
fixturename=None, fixturename=None,
pyfuncitem=pyfuncitem, pyfuncitem=pyfuncitem,
arg2fixturedefs=pyfuncitem._fixtureinfo.name2fixturedefs.copy(), arg2fixturedefs=pyfuncitem._fixtureinfo.name2fixturedefs.copy(),
arg2index={},
fixture_defs={}, fixture_defs={},
_ispytest=_ispytest, _ispytest=_ispytest,
) )
@ -706,7 +707,6 @@ class SubRequest(FixtureRequest):
fixturename=fixturedef.argname, fixturename=fixturedef.argname,
fixture_defs=request._fixture_defs, fixture_defs=request._fixture_defs,
arg2fixturedefs=request._arg2fixturedefs, arg2fixturedefs=request._arg2fixturedefs,
arg2index=request._arg2index,
_ispytest=_ispytest, _ispytest=_ispytest,
) )
self._parent_request: Final[FixtureRequest] = request self._parent_request: Final[FixtureRequest] = request