diff --git a/changelog/14513.improvement.rst b/changelog/14513.improvement.rst new file mode 100644 index 00000000000..7bd07fdcee8 --- /dev/null +++ b/changelog/14513.improvement.rst @@ -0,0 +1,5 @@ +The order in which fixture definitions overriding each other are resolved is now determined first by their *visibility* in the collection tree rather than by the order in which they are registered. + +A fixture defined for a more specific node (e.g. a module or an item) now always takes precedence over one with the same name defined for a more general node (e.g. the session), even when the more general one was registered later. +Fixtures with non-comparable visibility or the same visibility keep the existing behavior of "last registered wins". +This change is supposed to only affect plugins which register multiple fixtures programmatically with the same name. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 12b7da71acc..cbff9455e9d 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -133,6 +133,32 @@ 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) or + # with global visibility (no node). 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. @@ -1948,15 +1974,23 @@ def _register_fixture( ) faclist = self._arg2fixturedefs.setdefault(name, []) - if fixture_def.has_location: - faclist.append(fixture_def) + # 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: - # fixturedefs with no location are at the front - # so this inserts the current fixturedef after the - # existing fixturedefs from external plugins but - # before the fixturedefs provided in conftests. - i = len([f for f in faclist if not f.has_location]) - faclist.insert(i, fixture_def) + faclist.append(fixture_def) if autouse: if node is not None: self._node_autousenames.setdefault(node, []).append(name) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 9b85e1b388d..d442acaed12 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -1903,6 +1903,38 @@ def test_hello(self, item, fm): reprec = pytester.inline_run("-s") reprec.assertoutcome(passed=1) + def test_register_fixture_ordered_by_visibility(self, pytester: Pytester) -> None: + """A fixturedef registered for a more specific node takes precedence + over one registered for a more general (ancestor) node, regardless of + the order in which they were registered (#14513).""" + pytester.makeconftest( + """ + import pytest + + @pytest.hookimpl(wrapper=True) + def pytest_collection(session): + result = yield + fm = session._fixturemanager + item = session.items[0] + fm._register_fixture(name="fix", func=lambda: "session1", node=session) + # For coverage; can be removed once nodeid= deprecation is over. + fm._register_fixture(name="fix", func=lambda: "session-legacy", nodeid="") + fm._register_fixture(name="fix", func=lambda: "broken-legacy", nodeid="broken") + fm._register_fixture(name="fix", func=lambda fix: f"item1-{fix}", node=item) + fm._register_fixture(name="fix", func=lambda fix: f"item2-{fix}", node=item) + fm._register_fixture(name="fix", func=lambda: "session2", node=session) + return result + """ + ) + pytester.makepyfile( + """ + def test(fix): + assert fix == "item2-item1-session2" + """ + ) + reprec = pytester.inline_run() + reprec.assertoutcome(passed=1) + def test_parsefactories_relative_node_ids( self, pytester: Pytester, monkeypatch: MonkeyPatch ) -> None: