diff --git a/changelog/14942.bugfix.rst b/changelog/14942.bugfix.rst new file mode 100644 index 00000000000..1bcbf7d93d5 --- /dev/null +++ b/changelog/14942.bugfix.rst @@ -0,0 +1,2 @@ +Fixed a regression in 9.1.0 that made collection very slow (quadratic) when many fixtures are defined with the same name. +Then can particularly happen when many tests are defined in separate classes which inherit from a base class which defines fixtures (the fixtures are repeated per class). diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 05537ec01b2..7656fca2f5b 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -473,9 +473,9 @@ class FuncFixtureInfo: # Note: can't include dynamic dependencies (`request.getfixturevalue` calls). names_closure: list[str] # A map from a fixture name in the transitive closure to the FixtureDefs - # matching the name which are applicable to this function. + # matching the name which are visible to this item. # There may be multiple overriding fixtures with the same name. The - # sequence is ordered from furthest to closes to the function. + # sequence is ordered from furthest to closes to the item. name2fixturedefs: dict[str, Sequence[FixtureDef[Any]]] def prune_dependency_tree(self) -> None: @@ -726,7 +726,7 @@ def _get_active_fixturedef(self, argname: str) -> FixtureDef[object]: # No fixtures defined with this name. if fixturedefs is None: raise FixtureLookupError(argname, self) - # The are no fixtures with this name applicable for the function. + # The are no fixtures with this name visible for the item. if not fixturedefs: raise FixtureLookupError(argname, self) @@ -999,10 +999,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: - available.add(name) + for fixturedef in fm._get_all_fixture_defs_for_node(parent): + available.add(fixturedef.argname) if self.argname in available: msg = ( f" recursive dependency involving fixture '{self.argname}' detected" @@ -1788,10 +1786,20 @@ def __init__(self, session: Session) -> None: self.session = session self.config: Config = session.config # Maps a fixture name (argname) to all of the FixtureDefs in the test - # 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]]]] = {} + # suite/plugins defined with this name. + # For each name, there is a mapping from a Node to the fixtures with the + # name registered under that Node. The node determines the FixtureDef's + # visibility (equal to the fixturedef.node). + # Populated by parsefactories(). + self._arg2node2fixturedefs: Final[ + dict[str, dict[nodes.Node, list[FixtureDef[Any]]]] + ] = {} + # Legacy fallback, for plugins still using the deprecated nodeid-based + # API without a node reference. + # Part of FIXTURE_NODEID_DEPRECATED deprecation. + self._arg2nodeid2fixturedefs: Final[ + dict[str, dict[str, list[FixtureDef[Any]]]] + ] = {} # 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]]] = { @@ -1799,6 +1807,7 @@ def __init__(self, session: Session) -> None: } # Legacy fallback: nodeid string -> autouse names, for plugins still # using the deprecated nodeid-based API without a node reference. + # Part of FIXTURE_NODEID_DEPRECATED deprecation. self._nodeid_autousenames: Final[dict[str, list[str]]] = {} # Pending conftest modules waiting to be parsed when their Directory is collected. # Maps directory path -> conftest plugin module. @@ -1928,7 +1937,7 @@ def pytest_collection_finish(self) -> None: self._pending_conftests.clear() def _getautousenames(self, node: nodes.Node) -> Iterator[str]: - """Return the names of autouse fixtures applicable to node.""" + """Return the names of autouse fixtures visible to node.""" for parentnode in node.listchain(): basenames = self._node_autousenames.get(parentnode) if basenames: @@ -1939,7 +1948,7 @@ def _getautousenames(self, node: nodes.Node) -> Iterator[str]: yield from nodeid_basenames def _getusefixturesnames(self, node: nodes.Item) -> Iterator[str]: - """Return the names of usefixtures fixtures applicable to node.""" + """Return the names of usefixtures fixtures visible to node.""" for marker_node, mark in node.iter_markers_with_node(name="usefixtures"): if not mark.args: marker_node.warn( @@ -2095,24 +2104,16 @@ 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 + if node is not NOTSET: + node2fixturedefs = self._arg2node2fixturedefs.setdefault(name, {}) + node2fixturedefs.setdefault(node, []).insert(0, fixture_def) + elif nodeid is not NOTSET and nodeid is not None: + nodeid2fixturedefs = self._arg2nodeid2fixturedefs.setdefault(name, {}) + nodeid2fixturedefs.setdefault(nodeid, []).insert(0, fixture_def) else: - faclist.append(fixture_def) + # Global plugin autouse fixtures go under Session. + node2fixturedefs = self._arg2node2fixturedefs.setdefault(name, {}) + node2fixturedefs.setdefault(self.session, []).insert(0, fixture_def) if autouse: if node is not NOTSET: self._node_autousenames.setdefault(node, []).append(name) @@ -2338,41 +2339,64 @@ def parsefactories( nodeid=effective_nodeid, ) + def _get_all_fixture_defs(self) -> Iterable[FixtureDef[Any]]: + """Get all FixtureDefs. + + The order is not guaranteed. + """ + for node2fixturedefs in self._arg2node2fixturedefs.values(): + for fixturedefs in node2fixturedefs.values(): + yield from fixturedefs + for nodeid2fixturedefs in self._arg2nodeid2fixturedefs.values(): + for fixturedefs in nodeid2fixturedefs.values(): + yield from fixturedefs + + def _get_all_fixture_defs_for_node( + self, node: nodes.Node + ) -> Iterable[FixtureDef[Any]]: + """Get all FixtureDefs visible to a node. + + The order is not guaranteed. + """ + for node2fixturedefs in self._arg2node2fixturedefs.values(): + for parent in node.iter_parents(): + yield from node2fixturedefs.get(parent, ()) + for nodeid2fixturedefs in self._arg2nodeid2fixturedefs.values(): + for parent in node.iter_parents(): + yield from nodeid2fixturedefs.get(parent.nodeid, ()) + def getfixturedefs( self, argname: str, node: nodes.Node ) -> Sequence[FixtureDef[Any]] | None: - """Get FixtureDefs for a fixture name which are applicable + """Get FixtureDefs for a fixture name which are visible to a given node. Returns None if there are no fixtures at all defined with the given name. (This is different from the case in which there are fixtures - with the given name, but none applicable to the node. In this case, + with the given name, but none visible to the node. In this case, an empty result is returned). + The returned FixtureDefs are ordered from least specific (registered + higher in the collection tree) to most specific. For FixtureDefs + registered at the same Node, registered later => more specific. + :param argname: Name of the fixture to search for. :param node: The requesting Node. """ - try: - fixturedefs = self._arg2fixturedefs[argname] - except KeyError: + node2fixturedefs = self._arg2node2fixturedefs.get(argname, {}) + nodeid2fixturedefs = self._arg2nodeid2fixturedefs.get(argname, {}) + if not node2fixturedefs and not nodeid2fixturedefs: return None - return tuple(self._matchfactories(fixturedefs, node)) - - 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} - - 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 - yield fixturedef + fixturedefs = [ + fixturedef + for parent in node.iter_parents() + for fixturedef in [ + *node2fixturedefs.get(parent, ()), + *nodeid2fixturedefs.get(parent.nodeid, ()), + ] + ] + fixturedefs.reverse() + return fixturedefs def show_fixtures_per_test(config: Config) -> int | ExitCode: @@ -2487,30 +2511,24 @@ def _showfixtures_main(config: Config, session: Session) -> None: verbose = config.get_verbosity() fm = session._fixturemanager - available = [] seen: set[tuple[str, str]] = set() - - for argname, fixturedefs in fm._arg2fixturedefs.items(): - assert fixturedefs is not None - if not fixturedefs: + for fixturedef in fm._get_all_fixture_defs(): + loc = getlocation(fixturedef.func, invocation_dir) + if (fixturedef.argname, loc) in seen: continue - for fixturedef in fixturedefs: - loc = getlocation(fixturedef.func, invocation_dir) - if (fixturedef.argname, loc) in seen: - continue - seen.add((fixturedef.argname, loc)) - available.append( - ( - len(fixturedef.baseid), - fixturedef.func.__module__, - _pretty_fixture_path(invocation_dir, fixturedef.func), - fixturedef.argname, - fixturedef, - ) + seen.add((fixturedef.argname, loc)) + available.append( + ( + len(fixturedef.baseid), + fixturedef.func.__module__, + _pretty_fixture_path(invocation_dir, fixturedef.func), + fixturedef.argname, + fixturedef, ) - + ) available.sort() + currentmodule = None for baseid, module, prettypath, argname, fixturedef in available: if currentmodule != module: diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index 96c6fe61dba..ee91880d23a 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -323,7 +323,7 @@ def test_scoped_invisible(request): defs = request.session._fixturemanager.getfixturedefs( "scoped_legacy", request._pyfuncitem ) - assert defs == () + assert defs == [] """ ) result = pytester.runpytest("-W", "ignore::pytest.PytestRemovedIn10Warning") diff --git a/tox.ini b/tox.ini index e831bde84f2..75d22d6e2c1 100644 --- a/tox.ini +++ b/tox.ini @@ -187,7 +187,8 @@ setenv = PYTHONPATH=. commands = uv pip check - pytest bdd_wallet.py + ; Currently broken by _arg2fixturedefs -> _arg2node2fixturedefs change. + ; pytest bdd_wallet.py pytest --cov=. simple_integration.py pytest --ds=django_settings simple_integration.py pytest --html=simple.html simple_integration.py