Skip to content

Commit af80fa5

Browse files
committed
fixtures: remove dirty optimization for request.getfixturevalue()
Currently for each `Function` we copy the `FunctionDefinition`'s `_arg2fixturedefs`. This is done due to an ineffective optimization for a dynamic `request.getfixturevalue()` when the fixture name wasn't statically requested. In this case, we would save the dynamically-found `FixtureDef` in `_arg2fixturedef`, such that if it is requested again in the same item, it is returned immediately instead of doing a `_matchfactories` check again. But this case is already covered by the `_fixture_defs` optimization. I've always disliked this copy and mutation. The `_arg2fixturedefs` shenanigans performed during collection are hard enough to follow, and this only adds to the complexity, due to the mutability and having multiple different `_arg2fixturedefs` with different contents. So summing up: Pros: faster repeated `request.getfixturevalue()` in same test (ineffective) Cons: complexity (reasoning about mutability), extra copy Even without the `_fixture_defs` optimization, since `request.getfixturevalue()` is mostly a last-resort thing, so shouldn't be too common, and *repeated* calls to it in the same test should be even less common, and if so `_matchfactories` shouldn't be *that* slow, it should be fine to remove it.
1 parent d889ca0 commit af80fa5

1 file changed

Lines changed: 5 additions & 8 deletions

File tree

src/_pytest/fixtures.py

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ def __init__(
363363
self,
364364
pyfuncitem: Function,
365365
fixturename: str | None,
366-
arg2fixturedefs: dict[str, Sequence[FixtureDef[Any]]],
366+
arg2fixturedefs: Mapping[str, Sequence[FixtureDef[Any]]],
367367
fixture_defs: dict[str, FixtureDef[Any]],
368368
*,
369369
_ispytest: bool = False,
@@ -372,10 +372,9 @@ def __init__(
372372
#: Fixture for which this request is being performed.
373373
self.fixturename: Final = fixturename
374374
self._pyfuncitem: Final = pyfuncitem
375-
# The FixtureDefs for each fixture name requested by this item.
376-
# Starts from the statically-known fixturedefs resolved during
377-
# collection. Dynamically requested fixtures (using
378-
# `request.getfixturevalue("foo")`) are added dynamically.
375+
# The FixtureDefs for each fixture name statically requested by this
376+
# item (computed during collection). Dynamically requested fixtures
377+
# (using `request.getfixturevalue("foo")`) are not included here.
379378
self._arg2fixturedefs: Final = arg2fixturedefs
380379
# The evaluated argnames so far, mapping to the FixtureDef they resolved
381380
# to.
@@ -564,8 +563,6 @@ def _get_active_fixturedef(self, argname: str) -> FixtureDef[object]:
564563
# getfixturevalue(argname) which was naturally
565564
# not known at parsing/collection time.
566565
fixturedefs = self._fixturemanager.getfixturedefs(argname, self._pyfuncitem)
567-
if fixturedefs is not None:
568-
self._arg2fixturedefs[argname] = fixturedefs
569566
# No fixtures defined with this name.
570567
if fixturedefs is None:
571568
raise FixtureLookupError(argname, self)
@@ -669,7 +666,7 @@ def __init__(self, pyfuncitem: Function, *, _ispytest: bool = False) -> None:
669666
super().__init__(
670667
fixturename=None,
671668
pyfuncitem=pyfuncitem,
672-
arg2fixturedefs=pyfuncitem._fixtureinfo.name2fixturedefs.copy(),
669+
arg2fixturedefs=pyfuncitem._fixtureinfo.name2fixturedefs,
673670
fixture_defs={},
674671
_ispytest=_ispytest,
675672
)

0 commit comments

Comments
 (0)