From bd45ccd2ca399121fa91baa3fa0d25c2c36b6671 Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Tue, 27 Feb 2024 10:37:40 +0200
Subject: [PATCH] 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.
---
 src/_pytest/fixtures.py | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py
index 86a8eef04..fff955218 100644
--- a/src/_pytest/fixtures.py
+++ b/src/_pytest/fixtures.py
@@ -343,7 +343,6 @@ class FixtureRequest(abc.ABC):
         pyfuncitem: "Function",
         fixturename: Optional[str],
         arg2fixturedefs: Dict[str, Sequence["FixtureDef[Any]"]],
-        arg2index: Dict[str, int],
         fixture_defs: Dict[str, "FixtureDef[Any]"],
         *,
         _ispytest: bool = False,
@@ -357,16 +356,6 @@ class FixtureRequest(abc.ABC):
         # collection. Dynamically requested fixtures (using
         # `request.getfixturevalue("foo")`) are added dynamically.
         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
         # to.
         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.
         if not fixturedefs:
             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):
             raise FixtureLookupError(argname, self)
-        self._arg2index[argname] = index
+
         return fixturedefs[index]
 
     @property
@@ -660,7 +662,6 @@ class TopRequest(FixtureRequest):
             fixturename=None,
             pyfuncitem=pyfuncitem,
             arg2fixturedefs=pyfuncitem._fixtureinfo.name2fixturedefs.copy(),
-            arg2index={},
             fixture_defs={},
             _ispytest=_ispytest,
         )
@@ -706,7 +707,6 @@ class SubRequest(FixtureRequest):
             fixturename=fixturedef.argname,
             fixture_defs=request._fixture_defs,
             arg2fixturedefs=request._arg2fixturedefs,
-            arg2index=request._arg2index,
             _ispytest=_ispytest,
         )
         self._parent_request: Final[FixtureRequest] = request