From ef8ef5d2c27a9b11fe0faca06ae34f04d9e45e24 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 28 Aug 2026 20:18:14 +0200 Subject: [PATCH 1/6] fixtures: avoid hashing Nodes when matching fixturedefs pytest 9.1.0 moved `_matchfactories()` from matching a fixturedef's baseid string against the requesting node's parent nodeids to matching its node against the parent nodes themselves. `Node.__hash__` is a Python-level function (`hash(self._nodeid)`), so the inner loop went from one attribute access plus a `str` set lookup to a Python call per fixturedef -- and it runs once per fixturedef per lookup. On a suite that defines one fixture name on 4000 nodes that is 8M iterations: 0.363s before, 1.080s after. Precompute a `_match_key` on `FixtureDef`: `id()` of the node for node-based fixturedefs, the baseid string for legacy ones. Nodes compare by identity and the fixturedef holds its node alive, so the id cannot be reused while it is in use as a key; `int` and `str` never compare equal, so a single set holds both kinds and the loop is one set lookup again. Ref #14942 Co-Authored-By: Claude Opus 5 (1M context) Co-Authored-By: Claude Code --- changelog/14942.improvement.rst | 1 + src/_pytest/fixtures.py | 27 ++++++++++++++++++--------- 2 files changed, 19 insertions(+), 9 deletions(-) create mode 100644 changelog/14942.improvement.rst diff --git a/changelog/14942.improvement.rst b/changelog/14942.improvement.rst new file mode 100644 index 00000000000..7770eaebde3 --- /dev/null +++ b/changelog/14942.improvement.rst @@ -0,0 +1 @@ +Matching a fixture definition against the node requesting it no longer hashes collection tree nodes, which was a measurable cost during collection since it runs once per fixture definition per lookup. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 05537ec01b2..5234351498e 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1156,6 +1156,14 @@ def __init__( # # Deprecated: replaced by ``node``. self.baseid: Final = node.nodeid if node is not NOTSET else (baseid or "") + # Precomputed key for visibility matching, see + # `FixtureManager._matchfactories`. `id()` of the node for node-based + # fixtures (nodes compare by identity, and `self.node` keeps the node -- + # and hence its id -- alive), the baseid string for legacy ones. The two + # kinds can never compare equal, so a single set can hold both. + self._match_key: Final[int | str] = ( + id(node) if node is not NOTSET else self.baseid + ) # Whether the fixture was found from a node or a conftest in the # collection tree. Will be false for fixtures defined in non-conftest # plugins. @@ -2361,17 +2369,18 @@ def getfixturedefs( def _matchfactories( self, fixturedefs: Iterable[FixtureDef[Any]], node: nodes.Node ) -> Iterator[FixtureDef[Any]]: - # Collect parent nodes and their IDs for matching - parent_nodes = set(node.iter_parents()) - parentnodeids = {n.nodeid for n in parent_nodes} + # Match against the parent nodes by identity (`id()`) for node-based + # fixturedefs, and against their nodeids for legacy string-baseid ones. + # This loop runs once per fixturedef per lookup, so it is kept to a + # single set lookup per fixturedef -- in particular it avoids hashing + # `Node`s, whose `__hash__` is a Python-level function (#14942). + match_keys: set[int | str] = set() + for parent in node.iter_parents(): + match_keys.add(id(parent)) + match_keys.add(parent.nodeid) for fixturedef in fixturedefs: - if fixturedef.node is not None: - # Node-based matching: check if fixture's node is a parent - if fixturedef.node in parent_nodes: - yield fixturedef - elif fixturedef.baseid in parentnodeids: - # Fallback to string-based matching for legacy/plugins + if fixturedef._match_key in match_keys: yield fixturedef From 79e7b1c4286ab15cf8562c056e8662a42ed2f19e Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 28 Aug 2026 20:19:34 +0200 Subject: [PATCH 2/6] fixtures: index fixturedefs by visibility instead of scanning `getfixturedefs()` filtered the full list of fixturedefs registered under a name down to the ones visible to the requesting node. That list is as long as the number of definitions of that name in the whole suite, so a fixture inherited from a base class by thousands of test classes made collection quadratic. The fixturedefs visible to a node are exactly those defined on the node or one of its ancestors, and the ancestor chain is short. So bucket the fixturedefs by their visibility key and walk the parent chain instead of the definition list. Since the visible set is a chain, it is totally ordered by visibility, so `getfixturedefs()` can order the override chain itself: most general first, ties broken by registration order. That subsumes the partial-order insertion `_register_fixture()` was maintaining, which can now just append, and makes `is_visibility_more_specific()` unnecessary. The index cannot simply mirror `_register_fixture()`, because that is not the only way fixturedefs enter the manager: `_arg2fixturedefs` is de-facto public API that plugins mutate directly to inject fixtures. A survey of the plugin list found seven doing so -- pytest-bdd, pytest-bdd-ng, pytest-psqlgraph, pytest-codspeed, pytest-keyring, pytest-fixture-forms and pykiso -- between them using whole-key assignment, `del`, `setdefault().append()`, `insert(0, ...)` and `remove()`. So `_arg2fixturedefs` and its lists notice. An append extends the index in place, since it only adds a fixturedef with the next ordinal -- that is the hot path, taken once per fixture in the suite. Every other mutation drops the name's index, which `_index_for()` rebuilds on the next lookup. Validating by list length instead would have been cheaper and wrong: pytest-bdd appends a fixturedef and removes it again in a finalizer, leaving the same list object at the same length with different contents. Collection of N sibling classes each defining the same fixture name, and of 4000 such classes spread over a tree of varying depth: 9.0.3 9.1.x this flat, N=4000 1.52s 7.41s 1.36s flat, N=16000 10.95s 10.83s 5.25s varying depth 1.70s 8.72s 1.57s Fix #14942 Co-Authored-By: Claude Opus 5 (1M context) Co-Authored-By: Claude Code --- changelog/14942.improvement.rst | 5 + src/_pytest/fixtures.py | 250 +++++++++++++++++++++++++------- testing/python/fixtures.py | 73 ++++++++++ 3 files changed, 277 insertions(+), 51 deletions(-) diff --git a/changelog/14942.improvement.rst b/changelog/14942.improvement.rst index 7770eaebde3..720f2ba739d 100644 --- a/changelog/14942.improvement.rst +++ b/changelog/14942.improvement.rst @@ -1 +1,6 @@ Matching a fixture definition against the node requesting it no longer hashes collection tree nodes, which was a measurable cost during collection since it runs once per fixture definition per lookup. + +Looking up the fixture definitions visible to a node no longer scans every fixture definition registered under that name, but walks the node's parent chain instead. +Collection of suites which define the same fixture name on very many nodes -- for example a fixture inherited from a base class by thousands of test classes -- is no longer quadratic in the number of definitions. + +Plugins which inject fixtures by mutating ``FixtureManager._arg2fixturedefs`` directly keep working: the lookup index follows those mutations. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 5234351498e..89ef4a1ffb6 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -28,6 +28,7 @@ from typing import Literal from typing import NoReturn from typing import overload +from typing import SupportsIndex from typing import TYPE_CHECKING from typing import TypeVar import warnings @@ -84,6 +85,8 @@ if TYPE_CHECKING: + from typing_extensions import Self + from _pytest.python import CallSpec from _pytest.python import Function from _pytest.python import Metafunc @@ -136,31 +139,6 @@ def get_scope_package( return node.session -def is_visibility_more_specific( - candidate: FixtureDef[Any], other: FixtureDef[Any] -) -> bool: - """Return whether the visibility of ``candidate`` is strictly more specific - than that of ``other``, i.e. ``candidate`` is defined on a strict descendant - in the collection tree of where ``other`` is defined.""" - if candidate.node is None or other.node is None: - # Fallback for fixtures registered with a string nodeid (deprecated). - # In this case compare baseids, which are nodeid prefixes. - # This branch can be removed once baseid deprecation is done (pytest 10). - if candidate.baseid == other.baseid: - return False - if other.baseid == "": - return True - # `candidate.baseid` must continue with a node separator for it to be a - # true descendant. - return candidate.baseid.startswith(other.baseid) and candidate.baseid[ - len(other.baseid) - ] in ("/", ":") - - return ( - candidate.node is not other.node and other.node in candidate.node.iter_parents() - ) - - def get_scope_node(node: nodes.Node, scope: Scope) -> nodes.Node | None: """Get the closest parent node (including self) which matches the given scope. @@ -999,9 +977,8 @@ def formatrepr(self) -> FixtureLookupErrorRepr: available = set() parent = self.request._pyfuncitem.parent assert parent is not None - for name, fixturedefs in fm._arg2fixturedefs.items(): - faclist = list(fm._matchfactories(fixturedefs, parent)) - if faclist: + for name in fm._arg2fixturedefs: + if fm.getfixturedefs(name, parent): available.add(name) if self.argname in available: msg = ( @@ -1761,6 +1738,144 @@ def deduplicate_names(*seqs: Iterable[str]) -> tuple[str, ...]: return tuple(dict.fromkeys(name for seq in seqs for name in seq)) +class _FixtureDefsList(list["FixtureDef[Any]"]): + """The fixturedef list of one fixture name in `FixtureManager._arg2fixturedefs`. + + `getfixturedefs()` answers from a per-name index bucketed by visibility key + (see `FixtureManager._index_for`). Plugins are known to mutate + `_arg2fixturedefs` directly rather than going through `_register_fixture()` + -- pytest-bdd, pytest-bdd-ng, pytest-psqlgraph, pytest-codspeed, + pytest-keyring, pytest-fixture-forms and pykiso all do -- so the list keeps + that index in sync: an append extends it in place, and anything else drops + it, to be rebuilt on the next lookup. + """ + + __slots__ = ("_manager", "_name") + + def __init__( + self, + iterable: Iterable[FixtureDef[Any]] = (), + *, + manager: FixtureManager, + name: str, + ) -> None: + super().__init__(iterable) + self._manager = manager + self._name = name + + def _invalidate(self) -> None: + self._manager._arg2fixturedefs_by_key.pop(self._name, None) + + def append(self, fixturedef: FixtureDef[Any]) -> None: + # Appending only adds a fixturedef with the next ordinal, which the + # index can absorb without being rebuilt. This is the hot path: it is + # how every fixture in the suite is registered. + super().append(fixturedef) + index = self._manager._arg2fixturedefs_by_key.get(self._name) + if index is not None: + index.setdefault(fixturedef._match_key, []).append( + (len(self) - 1, fixturedef) + ) + + def insert(self, index: SupportsIndex, fixturedef: FixtureDef[Any]) -> None: + super().insert(index, fixturedef) + self._invalidate() + + def extend(self, iterable: Iterable[FixtureDef[Any]]) -> None: + super().extend(iterable) + self._invalidate() + + def remove(self, fixturedef: FixtureDef[Any]) -> None: + super().remove(fixturedef) + self._invalidate() + + def pop(self, index: SupportsIndex = -1) -> FixtureDef[Any]: + try: + return super().pop(index) + finally: + self._invalidate() + + def clear(self) -> None: + super().clear() + self._invalidate() + + def sort(self, **kwargs: Any) -> None: + super().sort(**kwargs) + self._invalidate() + + def reverse(self) -> None: + super().reverse() + self._invalidate() + + def __setitem__(self, index: Any, value: Any) -> None: + super().__setitem__(index, value) + self._invalidate() + + def __delitem__(self, index: Any) -> None: + super().__delitem__(index) + self._invalidate() + + def __iadd__(self, other: Iterable[FixtureDef[Any]]) -> Self: # type: ignore[misc,override] + super().__iadd__(other) + self._invalidate() + return self + + +class _Arg2FixtureDefs(dict[str, "_FixtureDefsList"]): + """`FixtureManager._arg2fixturedefs`, keeping the visibility index in sync. + + Values are coerced to `_FixtureDefsList` so that in-place mutation by + plugins is noticed too. See `_FixtureDefsList`. + """ + + __slots__ = ("_manager",) + + def __init__(self, manager: FixtureManager) -> None: + super().__init__() + self._manager = manager + + def _wrap(self, name: str, value: Iterable[FixtureDef[Any]]) -> _FixtureDefsList: + if isinstance(value, _FixtureDefsList) and value._name == name: + return value + return _FixtureDefsList(value, manager=self._manager, name=name) + + def __setitem__(self, name: str, value: Iterable[FixtureDef[Any]]) -> None: + super().__setitem__(name, self._wrap(name, value)) + self._manager._arg2fixturedefs_by_key.pop(name, None) + + def __delitem__(self, name: str) -> None: + super().__delitem__(name) + self._manager._arg2fixturedefs_by_key.pop(name, None) + + def setdefault( + self, name: str, default: Iterable[FixtureDef[Any]] = () + ) -> _FixtureDefsList: + try: + return self[name] + except KeyError: + self[name] = default + return self[name] + + def pop(self, name: str, *args: Any) -> Any: + try: + return super().pop(name, *args) + finally: + self._manager._arg2fixturedefs_by_key.pop(name, None) + + def popitem(self) -> tuple[str, _FixtureDefsList]: + name, value = super().popitem() + self._manager._arg2fixturedefs_by_key.pop(name, None) + return name, value + + def clear(self) -> None: + super().clear() + self._manager._arg2fixturedefs_by_key.clear() + + def update(self, *args: Any, **kwargs: Any) -> None: + for name, value in dict(*args, **kwargs).items(): + self[name] = value + + class FixtureManager: """pytest fixture definitions and information is stored and managed from this class. @@ -1799,7 +1914,18 @@ def __init__(self, session: Session) -> None: # suite/plugins defined with this name. Populated by parsefactories(). # TODO: The order of the FixtureDefs list of each arg is significant, # explain. - self._arg2fixturedefs: Final[dict[str, list[FixtureDef[Any]]]] = {} + # Cache of the visibility index, keyed by fixture name; must be + # assigned before `_arg2fixturedefs`, which keeps it in sync. + # Each entry buckets the name's fixturedefs by the visibility key they + # are matched on (see `FixtureDef._match_key`), as ``(registration + # ordinal, fixturedef)`` pairs, so that `getfixturedefs()` can walk the + # requesting node's parent chain instead of scanning every fixturedef + # registered under the name. Entries are built on demand by + # `_index_for()` and dropped when the name's fixturedefs change. + self._arg2fixturedefs_by_key: Final[ + dict[str, dict[int | str, list[tuple[int, FixtureDef[Any]]]]] + ] = {} + self._arg2fixturedefs: Final[_Arg2FixtureDefs] = _Arg2FixtureDefs(self) # A mapping from a node to a list of autouse fixture names it defines. # The Session entry holds global usefixtures from config. self._node_autousenames: Final[dict[nodes.Node, list[str]]] = { @@ -2103,24 +2229,10 @@ def _register_fixture( node=node, ) - faclist = self._arg2fixturedefs.setdefault(name, []) - # Insert the fixturedef into the list while maintaining a partial order - # based on visibility: a fixturedef whose visibility is more specific - # sorts after a more general one, so that it takes precedence in the - # override chain (the last applicable fixturedef in the list is used - # first, see getfixturedefs). - # fixturedefs with the same visibility keep registration order, i.e. the - # last registered wins. - # The order between non-comparable fixturedefs doesn't matter since they - # cannot be visible together. - # The idea is that a fixture that is defined closer to the item should - # take precedence. - for i, existing in enumerate(faclist): - if is_visibility_more_specific(existing, fixture_def): - faclist.insert(i, fixture_def) - break - else: - faclist.append(fixture_def) + # Registration order is kept as-is; the override chain is ordered by + # visibility at lookup time instead, see getfixturedefs(). The append + # keeps the visibility index in sync, see `_FixtureDefsList`. + self._arg2fixturedefs.setdefault(name).append(fixture_def) if autouse: if node is not NOTSET: self._node_autousenames.setdefault(node, []).append(name) @@ -2360,11 +2472,47 @@ def getfixturedefs( :param argname: Name of the fixture to search for. :param node: The requesting Node. """ - try: - fixturedefs = self._arg2fixturedefs[argname] - except KeyError: + index = self._index_for(argname) + if index is None: return None - return tuple(self._matchfactories(fixturedefs, node)) + + # The fixturedefs visible to `node` are exactly those defined on `node` + # or on one of its ancestors, so walk the (short) parent chain rather + # than scanning every fixturedef registered under `argname`. + # + # They are returned ordered by visibility -- most general first, so that + # the last one is the most specific and takes precedence in the override + # chain. Since the ancestors form a chain, this is a total order; ties + # (fixturedefs on the same node) are broken by registration order, i.e. + # the last registered wins. + matches: list[tuple[int, int, FixtureDef[Any]]] = [] + # `iter_parents()` yields `node` first, so rank it last. + for rank, parent in enumerate(reversed(list(node.iter_parents()))): + for key in (id(parent), parent.nodeid): + for regindex, fixturedef in index.get(key, ()): + matches.append((rank, regindex, fixturedef)) + matches.sort(key=lambda match: match[:2]) + return tuple(fixturedef for _, _, fixturedef in matches) + + def _index_for( + self, argname: str + ) -> dict[int | str, list[tuple[int, FixtureDef[Any]]]] | None: + """Return the visibility index of ``argname``, building it if needed. + + Returns None if no fixture is defined with that name at all. + """ + index = self._arg2fixturedefs_by_key.get(argname) + if index is None: + faclist = self._arg2fixturedefs.get(argname) + if faclist is None: + return None + index = {} + for ordinal, fixturedef in enumerate(faclist): + index.setdefault(fixturedef._match_key, []).append( + (ordinal, fixturedef) + ) + self._arg2fixturedefs_by_key[argname] = index + return index def _matchfactories( self, fixturedefs: Iterable[FixtureDef[Any]], node: nodes.Node diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index c0b49948152..a4d3753c952 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -1950,6 +1950,79 @@ def test(fix): reprec = pytester.inline_run() reprec.assertoutcome(passed=1) + def test_arg2fixturedefs_mutated_by_plugins(self, pytester: Pytester) -> None: + """Plugins mutate `FixtureManager._arg2fixturedefs` directly instead of + going through `_register_fixture()` -- pytest-bdd, pytest-bdd-ng, + pytest-psqlgraph, pytest-codspeed, pytest-keyring, pytest-fixture-forms + and pykiso all do. `getfixturedefs()` must reflect those mutations + (#14942).""" + pytester.makeconftest( + """ + import pytest + from _pytest.fixtures import FixtureDef + + + def make(fm, name, value, node): + return FixtureDef( + config=fm.config, + baseid=None, + argname=name, + func=lambda: value, + scope="function", + params=None, + node=node, + _ispytest=True, + ) + + + @pytest.hookimpl(wrapper=True) + def pytest_collection(session): + result = yield + fm = session._fixturemanager + item = session.items[0] + + def values(name): + return [d.func() for d in fm.getfixturedefs(name, item) or ()] + + # Whole-key assignment (pytest-codspeed, pytest-keyring, pykiso). + fm._arg2fixturedefs["a"] = [make(fm, "a", "a1", session)] + assert values("a") == ["a1"], values("a") + fm._arg2fixturedefs["a"] = [make(fm, "a", "a2", session)] + assert values("a") == ["a2"], values("a") + + # setdefault(...).append(...) followed by remove(...), as + # pytest-bdd does around each step. Same list object at the same + # length before and after, so a cache keyed on length alone + # would serve the removed fixturedef. + base = make(fm, "b", "b0", session) + fm._arg2fixturedefs["b"] = [base] + extra = make(fm, "b", "b1", session) + fm._arg2fixturedefs.setdefault("b", []).append(extra) + assert values("b") == ["b0", "b1"], values("b") + fm._arg2fixturedefs["b"].remove(extra) + assert values("b") == ["b0"], values("b") + + # insert(0, ...) (pytest-bdd-ng, pytest-psqlgraph). + front = make(fm, "b", "b-front", session) + fm._arg2fixturedefs.setdefault("b", []).insert(0, front) + assert values("b") == ["b-front", "b0"], values("b") + + # del (pytest-bdd, once the scenario is done). + del fm._arg2fixturedefs["a"] + assert fm.getfixturedefs("a", item) is None + + return result + """ + ) + pytester.makepyfile( + """ + def test(): + pass + """ + ) + reprec = pytester.inline_run() + reprec.assertoutcome(passed=1) + def test_parsefactories_relative_node_ids( self, pytester: Pytester, monkeypatch: MonkeyPatch ) -> None: From 4fa696739dfc6e539832892e08c21c5ad197c0a6 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 28 Aug 2026 20:22:39 +0200 Subject: [PATCH 3/6] fixtures: cover the whole _arg2fixturedefs mutation surface The previous commit tracks every way `_arg2fixturedefs` and its lists can be mutated, but only four of those ways are used by the plugins that prompted it, so the rest went untested -- and an untested invalidation path is exactly the kind that rots into a stale index. Drive each of them through the live fixture manager and assert the lookup result after every step. `list.clear()` is exercised on a throwaway mapping since on the live one it would drop every fixture in the session. Together with the plugin-shape test this covers every line of `_FixtureDefsList`, `_Arg2FixtureDefs`, `_index_for()` and `getfixturedefs()`. Co-Authored-By: Claude Opus 5 (1M context) Co-Authored-By: Claude Code --- testing/python/fixtures.py | 127 +++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index a4d3753c952..87e6b287b93 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -2023,6 +2023,133 @@ def test(): reprec = pytester.inline_run() reprec.assertoutcome(passed=1) + def test_arg2fixturedefs_mutation_surface(self, pytester: Pytester) -> None: + """Every way of mutating `_arg2fixturedefs` and its lists keeps the + visibility index that `getfixturedefs()` answers from correct (#14942). + + `test_arg2fixturedefs_mutated_by_plugins` covers the shapes plugins are + known to use; this covers the rest of the `list`/`dict` mutation + surface, so that a plugin reaching for one of them is not silently + served a stale index. + """ + pytester.makepyfile( + """ + import pytest + + from _pytest.fixtures import _Arg2FixtureDefs + from _pytest.fixtures import FixtureDef + + + @pytest.fixture + def real(): + return "real" + + + def test_surface(request, real): + fm = request.session._fixturemanager + session = request.session + + def make(value): + return FixtureDef( + config=fm.config, + baseid=None, + argname="probe", + func=lambda: value, + scope="function", + params=None, + node=session, + _ispytest=True, + ) + + def values(name="probe"): + defs = fm.getfixturedefs(name, request.node) + return None if defs is None else [d.func() for d in defs] + + # Seed via whole-key assignment, and look up once so that an + # index exists to be kept in sync or dropped. + fm._arg2fixturedefs["probe"] = [make("a")] + assert values() == ["a"] + probe = fm._arg2fixturedefs["probe"] + + probe.append(make("b")) + assert values() == ["a", "b"] + + probe.insert(0, make("c")) + assert values() == ["c", "a", "b"] + + probe.extend([make("d")]) + assert values() == ["c", "a", "b", "d"] + + probe.remove(probe[0]) + assert values() == ["a", "b", "d"] + + assert probe.pop().func() == "d" + assert values() == ["a", "b"] + + probe[0] = make("e") + assert values() == ["e", "b"] + + del probe[0] + assert values() == ["b"] + + probe += [make("f")] + assert values() == ["b", "f"] + + probe.sort(key=lambda d: d.func(), reverse=True) + assert values() == ["f", "b"] + + probe.reverse() + assert values() == ["b", "f"] + + probe.clear() + assert values() == [] + + # Dict-level mutation. + fm._arg2fixturedefs.setdefault("probe").append(make("g")) + assert values() == ["g"] + + fm._arg2fixturedefs.update({"probe": [make("h")]}) + assert values() == ["h"] + + assert fm._arg2fixturedefs.pop("probe") + assert values() is None + assert fm._arg2fixturedefs.pop("probe", None) is None + + fm._arg2fixturedefs["probe"] = [make("i")] + assert values() == ["i"] + del fm._arg2fixturedefs["probe"] + assert values() is None + + # Re-assigning a list already tracked under this name keeps + # it as-is rather than wrapping it again. + fm._arg2fixturedefs["probe"] = [make("l")] + same = fm._arg2fixturedefs["probe"] + fm._arg2fixturedefs["probe"] = same + assert fm._arg2fixturedefs["probe"] is same + assert values() == ["l"] + + # `popitem()` pops the most recently inserted key. + fm._arg2fixturedefs["probe"] = [make("j")] + name, popped = fm._arg2fixturedefs.popitem() + assert name == "probe" + assert [d.func() for d in popped] == ["j"] + assert values() is None + + # `clear()` is exercised on a throwaway mapping: on the live one + # it would drop every fixture in the session. It clears the + # whole index cache, which lookups then rebuild. + throwaway = _Arg2FixtureDefs(fm) + throwaway["probe"] = [make("k")] + throwaway.clear() + assert not throwaway + assert fm._arg2fixturedefs_by_key == {} + # The cleared cache is rebuilt on the next lookup. + assert values("real") == ["real"] + """ + ) + reprec = pytester.inline_run() + reprec.assertoutcome(passed=1) + def test_parsefactories_relative_node_ids( self, pytester: Pytester, monkeypatch: MonkeyPatch ) -> None: From d92376889142cc91a360b66d3eb894e29afd7f12 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 28 Aug 2026 20:27:45 +0200 Subject: [PATCH 4/6] fixtures: property-test the visibility index against a naive lookup The two deterministic tests cover the mutation shapes one at a time. What they do not cover is sequences: an append onto a list whose index was just dropped, an assignment followed by an append, a remove between two lookups. That is where a stale index actually comes from. Drive those sequences with a hypothesis state machine, and after every step assert that `getfixturedefs()` agrees with the same question answered the naive way -- scan the authoritative `_arg2fixturedefs` list, keep what is defined on the requesting node or an ancestor, order it most general first. The nodes are stand-ins rather than collected ones: visibility only depends on identity, nodeid and the parent chain, so a real tree would cost a collection per example without testing anything more. Dropping the `_invalidate()` from any single `_FixtureDefsList` method fails this test; the deterministic ones catch only their own shape. Co-Authored-By: Claude Opus 5 (1M context) Co-Authored-By: Claude Code --- testing/python/fixtures.py | 141 +++++++++++++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 87e6b287b93..5585b2d99c1 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -10,6 +10,7 @@ from _pytest.compat import getfuncargnames from _pytest.config import ExitCode from _pytest.fixtures import deduplicate_names +from _pytest.fixtures import FixtureDef from _pytest.fixtures import ParamValueKey from _pytest.fixtures import TopRequest from _pytest.monkeypatch import MonkeyPatch @@ -2150,6 +2151,146 @@ def values(name="probe"): reprec = pytester.inline_run() reprec.assertoutcome(passed=1) + def test_visibility_index_matches_naive_lookup(self, request) -> None: + """`getfixturedefs()` answers from an index bucketed by visibility key. + + Property-test it against the same question answered the naive way -- + scan the authoritative `_arg2fixturedefs` list, keep the fixturedefs + defined on the requesting node or one of its ancestors, order them most + general first -- under arbitrary sequences of the mutations plugins + perform on `_arg2fixturedefs` (#14942). + + This is what catches a missing invalidation: dropping the + `_invalidate()` from any single `_FixtureDefsList` method fails here. + """ + hypothesis = pytest.importorskip("hypothesis") + from hypothesis import stateful + import hypothesis.strategies as st + + fm = request.session._fixturemanager + names = ["_probe_a", "_probe_b"] + + class FakeNode: + """Enough of a `Node` for visibility: identity, nodeid, parents.""" + + def __init__(self, nodeid: str, parent: FakeNode | None) -> None: + self.nodeid = nodeid + self.parent = parent + + def iter_parents(self): + node: FakeNode | None = self + while node is not None: + yield node + node = node.parent + + # A tree with two branches, so that some fixturedefs are registered on + # nodes which are not ancestors of the node being queried. + tree = [FakeNode("", None)] + for parent_index, part in [ + (0, "d1"), + (1, "d2"), + (2, "m.py"), + (3, "Cls"), + (0, "e1"), + (5, "n.py"), + ]: + parent = tree[parent_index] + tree.append(FakeNode(f"{parent.nodeid}/{part}".lstrip("/"), parent)) + + def naive(argname, node): + faclist = fm._arg2fixturedefs.get(argname) + if faclist is None: + return None + parents = list(node.iter_parents())[::-1] # most general first + rank_by_id = {id(p): r for r, p in enumerate(parents)} + rank_by_nodeid = {p.nodeid: r for r, p in enumerate(parents)} + matches = [] + for ordinal, fixturedef in enumerate(faclist): + if fixturedef.node is not None: + rank = rank_by_id.get(id(fixturedef.node)) + else: + rank = rank_by_nodeid.get(fixturedef.baseid) + if rank is not None: + matches.append((rank, ordinal, fixturedef)) + matches.sort(key=lambda match: match[:2]) + return tuple(fixturedef for _, _, fixturedef in matches) + + def drop_probes(): + for name in names: + fm._arg2fixturedefs.pop(name, None) + + NAME = st.sampled_from(names) + NODE = st.integers(0, len(tree) - 1) + + class VisibilityIndex(stateful.RuleBasedStateMachine): + def __init__(self): + super().__init__() + drop_probes() + + def _make(self, name, node_index): + return FixtureDef( + config=fm.config, + baseid=None, + argname=name, + func=lambda: None, + scope="function", + params=None, + node=tree[node_index], + _ispytest=True, + ) + + @stateful.rule(name=NAME, node_index=NODE) + def register(self, name, node_index): + fm._register_fixture( + name=name, func=lambda: None, node=tree[node_index] + ) + + @stateful.rule(name=NAME, node_index=NODE) + def append(self, name, node_index): + fm._arg2fixturedefs.setdefault(name, []).append( + self._make(name, node_index) + ) + + @stateful.rule(name=NAME, node_index=NODE) + def insert_front(self, name, node_index): + fm._arg2fixturedefs.setdefault(name, []).insert( + 0, self._make(name, node_index) + ) + + @stateful.rule(name=NAME, node_index=NODE) + def assign(self, name, node_index): + fm._arg2fixturedefs[name] = [self._make(name, node_index)] + + @stateful.rule(name=NAME, index=st.integers(0, 5)) + def remove(self, name, index): + faclist = fm._arg2fixturedefs.get(name) + if faclist: + faclist.remove(faclist[index % len(faclist)]) + + @stateful.rule(name=NAME) + def delete(self, name): + fm._arg2fixturedefs.pop(name, None) + + @stateful.invariant() + def index_agrees_with_naive_lookup(self): + for name in names: + for node in tree: + assert fm.getfixturedefs(name, node) == naive(name, node), ( + name, + node.nodeid, + ) + + def teardown(self): + drop_probes() + + try: + stateful.run_state_machine_as_test( + VisibilityIndex, + settings=hypothesis.settings(max_examples=100, deadline=None), + ) + finally: + drop_probes() + def test_parsefactories_relative_node_ids( self, pytester: Pytester, monkeypatch: MonkeyPatch ) -> None: From a6479fe0fc680f63c0638fe6140cd720ad0779d7 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 28 Aug 2026 20:48:22 +0200 Subject: [PATCH 5/6] fixtures: cover legacy baseid visibility, and drop dead _matchfactories Two loose ends the coverage of the previous commits exposed. The property test only built node-based fixturedefs, so the reference lookup's legacy branch -- rank by baseid string rather than by node identity -- was never taken. Add a rule that registers fixturedefs with a string baseid and no node, as plugins which have not moved off the deprecated API still produce. That covers the deprecated path on both sides of the comparison. `_matchfactories()` has had no caller since `getfixturedefs()` started answering from the index. It is not part of any public API, nothing in the tree uses it, and a scan of the plugin list found no plugin calling it either -- the only hits were vendored copies of pytest itself. Remove it rather than leave a second, now-untested implementation of visibility matching for the two to drift apart. Its test in deprecated_test.py keeps testing what it was really about, which is baseid string matching, through the real lookup path. Co-Authored-By: Claude Opus 5 (1M context) Co-Authored-By: Claude Code --- src/_pytest/fixtures.py | 19 +------------------ testing/deprecated_test.py | 7 ++++--- testing/python/fixtures.py | 23 +++++++++++++++++++++++ 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 89ef4a1ffb6..c19fef1666f 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1134,7 +1134,7 @@ def __init__( # Deprecated: replaced by ``node``. self.baseid: Final = node.nodeid if node is not NOTSET else (baseid or "") # Precomputed key for visibility matching, see - # `FixtureManager._matchfactories`. `id()` of the node for node-based + # `FixtureManager._index_for`. `id()` of the node for node-based # fixtures (nodes compare by identity, and `self.node` keeps the node -- # and hence its id -- alive), the baseid string for legacy ones. The two # kinds can never compare equal, so a single set can hold both. @@ -2514,23 +2514,6 @@ def _index_for( self._arg2fixturedefs_by_key[argname] = index return index - def _matchfactories( - self, fixturedefs: Iterable[FixtureDef[Any]], node: nodes.Node - ) -> Iterator[FixtureDef[Any]]: - # Match against the parent nodes by identity (`id()`) for node-based - # fixturedefs, and against their nodeids for legacy string-baseid ones. - # This loop runs once per fixturedef per lookup, so it is kept to a - # single set lookup per fixturedef -- in particular it avoids hashing - # `Node`s, whose `__hash__` is a Python-level function (#14942). - match_keys: set[int | str] = set() - for parent in node.iter_parents(): - match_keys.add(id(parent)) - match_keys.add(parent.nodeid) - - for fixturedef in fixturedefs: - if fixturedef._match_key in match_keys: - yield fixturedef - def show_fixtures_per_test(config: Config) -> int | ExitCode: from _pytest.main import wrap_session diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index 96c6fe61dba..8287310806b 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -182,7 +182,7 @@ class TestFixtureNodeidDeprecations: - parsefactories() with no args raises TypeError - _register_fixture(nodeid=string) deprecation warning - _nodeid_autousenames population and _getautousenames yield - - _matchfactories string-based fallback (match + non-match branches) + - baseid string-based visibility matching (match + non-match branches) """ def test_parsefactories_nodeid_deprecation(self, pytester: Pytester) -> None: @@ -289,8 +289,9 @@ def test_autouse_yielded(request): result = pytester.runpytest("-W", "default::pytest.PytestRemovedIn10Warning") result.assert_outcomes(passed=1) - def test_matchfactories_string_fallback(self, pytester: Pytester) -> None: - """_matchfactories uses baseid string matching for legacy fixtures. + def test_baseid_string_fallback(self, pytester: Pytester) -> None: + """Fixture visibility falls back to baseid string matching for legacy + fixturedefs, which have no node. Exercises both branches: - baseid="" matches all nodes (global fixture) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 5585b2d99c1..9976b529de4 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -2163,6 +2163,8 @@ def test_visibility_index_matches_naive_lookup(self, request) -> None: This is what catches a missing invalidation: dropping the `_invalidate()` from any single `_FixtureDefsList` method fails here. """ + import warnings + hypothesis = pytest.importorskip("hypothesis") from hypothesis import stateful import hypothesis.strategies as st @@ -2239,6 +2241,21 @@ def _make(self, name, node_index): _ispytest=True, ) + def _make_legacy(self, name, node_index): + """A fixturedef with a string baseid and no node, as plugins + which have not moved off the deprecated API still produce.""" + with warnings.catch_warnings(): + warnings.simplefilter("ignore", pytest.PytestRemovedIn10Warning) + return FixtureDef( + config=fm.config, + baseid=tree[node_index].nodeid, + argname=name, + func=lambda: None, + scope="function", + params=None, + _ispytest=True, + ) + @stateful.rule(name=NAME, node_index=NODE) def register(self, name, node_index): fm._register_fixture( @@ -2251,6 +2268,12 @@ def append(self, name, node_index): self._make(name, node_index) ) + @stateful.rule(name=NAME, node_index=NODE) + def append_legacy(self, name, node_index): + fm._arg2fixturedefs.setdefault(name, []).append( + self._make_legacy(name, node_index) + ) + @stateful.rule(name=NAME, node_index=NODE) def insert_front(self, name, node_index): fm._arg2fixturedefs.setdefault(name, []).insert( From 9871f7225ea77dd8d2946189ef7ea81270a3914d Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 28 Aug 2026 21:36:18 +0200 Subject: [PATCH 6/6] fixtures: document the aliasing change and the id() workaround Two things a reader of the index code should not have to reconstruct. Coercing assigned values to `_FixtureDefsList` copies, so assigning one list under two fixture names now yields two independent lists, and a list held from before the assignment no longer tracks the stored one. `_arg2fixturedefs` is internal and some churn there is expected, but this particular change fails silently rather than loudly, so it is spelled out in the class docstring, in the changelog and pinned by a test. Assigning back a list read out of the mapping under the same name stays a no-op, so the usual plugin patterns are unaffected. And `id()` is a workaround, not the natural key. The natural key is the node itself, since nodes compare by identity -- but `Node.__hash__` is a Python-level function hashing `nodeid`, so keying on nodes costs a Python call per fixturedef per lookup, which is what made this path a bottleneck. Say so, along with what would let it go away: an identity hash on `Node`, which its identity `__eq__` implies anyway. Co-Authored-By: Claude Opus 5 (1M context) Co-Authored-By: Claude Code --- changelog/14942.improvement.rst | 3 ++- src/_pytest/fixtures.py | 34 +++++++++++++++++++++++++++++---- testing/python/fixtures.py | 10 ++++++++++ 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/changelog/14942.improvement.rst b/changelog/14942.improvement.rst index 720f2ba739d..29491c76c2a 100644 --- a/changelog/14942.improvement.rst +++ b/changelog/14942.improvement.rst @@ -3,4 +3,5 @@ Matching a fixture definition against the node requesting it no longer hashes co Looking up the fixture definitions visible to a node no longer scans every fixture definition registered under that name, but walks the node's parent chain instead. Collection of suites which define the same fixture name on very many nodes -- for example a fixture inherited from a base class by thousands of test classes -- is no longer quadratic in the number of definitions. -Plugins which inject fixtures by mutating ``FixtureManager._arg2fixturedefs`` directly keep working: the lookup index follows those mutations. +Plugins which inject fixtures by mutating the internal ``FixtureManager._arg2fixturedefs`` directly keep working: the lookup index follows those mutations. +One behavior there did change: a list stored in that mapping is copied rather than aliased, so assigning the same list under two fixture names now gives two independent lists. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index c19fef1666f..a58e623ea63 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1134,10 +1134,21 @@ def __init__( # Deprecated: replaced by ``node``. self.baseid: Final = node.nodeid if node is not NOTSET else (baseid or "") # Precomputed key for visibility matching, see - # `FixtureManager._index_for`. `id()` of the node for node-based - # fixtures (nodes compare by identity, and `self.node` keeps the node -- - # and hence its id -- alive), the baseid string for legacy ones. The two - # kinds can never compare equal, so a single set can hold both. + # `FixtureManager._index_for`: `id()` of the node for node-based + # fixtures, the baseid string for legacy ones. The two kinds can never + # compare equal, so a single mapping can hold both. + # + # `id()` is a workaround, not the natural key. The natural key is the + # node itself -- nodes compare by identity -- but `Node.__hash__` is a + # Python-level function hashing `nodeid`, so using nodes as dict keys + # costs a Python call per fixturedef per lookup, which is what made this + # a bottleneck in the first place (#14942). If `Node` ever gets an + # identity hash (which is what its identity `__eq__` implies anyway), + # this can go back to keying on the node. + # + # Reusing an id would mean matching the wrong node, so it must not + # outlive the object: `self.node` holds the node for as long as this + # fixturedef exists, and the fixturedef is what carries the key around. self._match_key: Final[int | str] = ( id(node) if node is not NOTSET else self.baseid ) @@ -1826,6 +1837,21 @@ class _Arg2FixtureDefs(dict[str, "_FixtureDefsList"]): Values are coerced to `_FixtureDefsList` so that in-place mutation by plugins is noticed too. See `_FixtureDefsList`. + + That coercion copies: a list assigned under one name is a different object + from the one handed in, so assigning the same list under two names gives two + independent lists rather than one shared by both, and a list held from + before the assignment no longer tracks the stored one:: + + fm._arg2fixturedefs["a"] = shared + fm._arg2fixturedefs["b"] = shared # "a" and "b" are now separate + shared.append(fixturedef) # visible under neither + + No known plugin relies on that aliasing, and `_arg2fixturedefs` is internal, + so some churn is expected; it is called out because the failure is silent. + Assigning a list already stored under the same name is a no-op, so the + common ``fm._arg2fixturedefs[name].append(...)`` and re-assignment of a list + read back out of the mapping keep working unchanged. """ __slots__ = ("_manager",) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 9976b529de4..df930989769 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -2129,6 +2129,16 @@ def values(name="probe"): assert fm._arg2fixturedefs["probe"] is same assert values() == ["l"] + # Wrapping copies, so one list assigned under two names gives + # two independent lists, and the original tracks neither. + shared = [make("m")] + fm._arg2fixturedefs["probe"] = shared + fm._arg2fixturedefs["probe2"] = shared + shared.append(make("n")) + assert values() == ["m"] + assert values("probe2") == ["m"] + del fm._arg2fixturedefs["probe2"] + # `popitem()` pops the most recently inserted key. fm._arg2fixturedefs["probe"] = [make("j")] name, popped = fm._arg2fixturedefs.popitem()