From 7186cd46553a14bfc1ee0e6e77e5daa6c3882bd4 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 29 May 2026 21:32:19 +0300 Subject: [PATCH] fixtures: order fixture override chains by visibility The override chain for a fixture name was determined by the semi-incidental order in which fixturedefs were registered, with an ad-hoc tweak that pushed fixturedefs with no location to the front of the list (the last one is used first). Order the fixturedef list by partial order based on visibility instead: a fixturedef whose visibility is more specific (a descendant in the collection tree) sorts after a more general (ancestor) one, so it takes precedence in the override chain. Fixturedefs with non-comparable visibility keep registration order (last registered wins). The idea is that a fixture that defined closer to the item should take precedence. This generalizes the previous "no location" handling and makes precedence robust for imperatively registered fixtures. The code is complicated by the need to support FixtureDefs with legacy string nodeids rather than Nodes visibility, but these are deprecated and will removed in pytest 10, then we can remove the compat code. Also, comparing Nodes by visibility is not a super fast operation, since we need to look at the actual tree. However, I think overrides are not widely used and when they do, the chains are not very long, so hopefully that's fine. But it can be optimized using some tricks if necessary. Fix #14513 --- changelog/14513.improvement.rst | 5 ++++ src/_pytest/fixtures.py | 50 +++++++++++++++++++++++++++------ testing/python/fixtures.py | 32 +++++++++++++++++++++ 3 files changed, 79 insertions(+), 8 deletions(-) create mode 100644 changelog/14513.improvement.rst 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: