diff --git a/AUTHORS b/AUTHORS index d1a2d3e7911..d1c2edd85fb 100644 --- a/AUTHORS +++ b/AUTHORS @@ -483,6 +483,7 @@ Terje Runde Thomas Grainger Thomas Hisch Tianyu Dongfang +Tim Anderson Tim Hoffmann Tim Strazny TJ Bruno diff --git a/changelog/14942.bugfix.rst b/changelog/14942.bugfix.rst new file mode 100644 index 00000000000..3f1fd6e4f43 --- /dev/null +++ b/changelog/14942.bugfix.rst @@ -0,0 +1,2 @@ +Fixed quadratic fixture-registration behavior when many unrelated collection +nodes define fixtures with the same name. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 05537ec01b2..84aac0a67b4 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1792,6 +1792,17 @@ def __init__(self, session: Session) -> None: # TODO: The order of the FixtureDefs list of each arg is significant, # explain. self._arg2fixturedefs: Final[dict[str, list[FixtureDef[Any]]]] = {} + # Maps a fixture name to nodes which are strict ancestors of at least + # one node where a fixture with that name is registered. This lets the + # overwhelmingly common case (fixtures on unrelated sibling nodes) be + # appended without scanning the entire override chain. + self._arg2fixturedefs_ancestor_nodes: Final[dict[str, set[nodes.Node]]] = ( + defaultdict(set) + ) + # Fixture definitions using the deprecated string-nodeid API cannot + # participate in the node index above, so names containing one retain + # the complete comparison path. + self._arg2fixturedefs_with_legacy_visibility: Final[set[str]] = set() # 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]]] = { @@ -2107,12 +2118,27 @@ def _register_fixture( # 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: + ancestor_nodes = self._arg2fixturedefs_ancestor_nodes[name] + if ( + fixture_def.node is not None + and name not in self._arg2fixturedefs_with_legacy_visibility + and fixture_def.node not in ancestor_nodes + ): faclist.append(fixture_def) + else: + 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) + + if fixture_def.node is not None: + parents = fixture_def.node.iter_parents() + next(parents) # The fixture's own node is not a strict ancestor. + ancestor_nodes.update(parents) + else: + self._arg2fixturedefs_with_legacy_visibility.add(name) if autouse: if node is not NOTSET: self._node_autousenames.setdefault(node, []).append(name) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index e4334af2145..2331a73da4b 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -1950,6 +1950,53 @@ def test(fix): reprec = pytester.inline_run() reprec.assertoutcome(passed=1) + def test_register_sibling_fixtures_avoids_quadratic_comparisons( + self, pytester: Pytester + ) -> None: + pytester.makeconftest( + """ + import pytest + import _pytest.fixtures + + @pytest.hookimpl(wrapper=True) + def pytest_collection(): + original = _pytest.fixtures.is_visibility_more_specific + comparisons = 0 + + def counted(candidate, other): + nonlocal comparisons + comparisons += 1 + return original(candidate, other) + + _pytest.fixtures.is_visibility_more_specific = counted + try: + result = yield + finally: + _pytest.fixtures.is_visibility_more_specific = original + + # Sibling class fixtures cannot override one another. Their + # registration should not compare every pair (#14942). + assert comparisons < 40 + return result + """ + ) + classes = "\n".join( + f"""class Test{i}: + @pytest.fixture + def shared(self): + return {i} + + def test_shared(self, shared): + assert shared == {i} +""" + for i in range(40) + ) + pytester.makepyfile(f"import pytest\n{classes}") + + result = pytester.runpytest() + + result.assert_outcomes(passed=40) + def test_parsefactories_relative_node_ids( self, pytester: Pytester, monkeypatch: MonkeyPatch ) -> None: