Remove prune_dependency_tree and reuse getfixtureclosure logic

src/_pytest/fixtures.py

@@ -313,33 +313,6 @@ class FuncFixtureInfo:
     # sequence is ordered from furthest to closes to the function.
     name2fixturedefs: Dict[str, Sequence["FixtureDef[Any]"]]
 
-    def prune_dependency_tree(self) -> None:
-        """Recompute names_closure from initialnames and name2fixturedefs.
-
-        Can only reduce names_closure, which means that the new closure will
-        always be a subset of the old one. The order is preserved.
-
-        This method is needed because direct parametrization may shadow some
-        of the fixtures that were included in the originally built dependency
-        tree. In this way the dependency tree can get pruned, and the closure
-        of argnames may get reduced.
-        """
-        closure: Set[str] = set()
-        working_set = set(self.initialnames)
-        while working_set:
-            argname = working_set.pop()
-            # Argname may be smth not included in the original names_closure,
-            # in which case we ignore it. This currently happens with pseudo
-            # FixtureDefs which wrap 'get_direct_param_fixture_func(request)'.
-            # So they introduce the new dependency 'request' which might have
-            # been missing in the original tree (closure).
-            if argname not in closure and argname in self.names_closure:
-                closure.add(argname)
-                if argname in self.name2fixturedefs:
-                    working_set.update(self.name2fixturedefs[argname][-1].argnames)
-
-        self.names_closure[:] = sorted(closure, key=self.names_closure.index)
-
 
 class FixtureRequest(abc.ABC):
     """The type of the ``request`` fixture.
@@ -958,7 +931,6 @@ def _eval_scope_callable(
     return result
 
 
-@final
 class FixtureDef(Generic[FixtureValue]):
     """A container for a fixture definition.
 
@@ -1101,6 +1073,26 @@ def __repr__(self) -> str:
         )
 
 
+class IdentityFixture(FixtureDef[FixtureValue]):
+    def __init__(
+        self,
+        fixturemanager: "FixtureManager",
+        argname: str,
+        scope: Union[Scope, _ScopeName, Callable[[str, Config], _ScopeName], None],
+        *,
+        _ispytest: bool = False,
+    ):
+        super().__init__(
+            fixturemanager,
+            "",
+            argname,
+            lambda request: request.param,
+            scope,
+            None,
+            _ispytest=_ispytest,
+        )
+
+
 def resolve_fixture_function(
     fixturedef: FixtureDef[FixtureValue], request: FixtureRequest
 ) -> "_FixtureFunc[FixtureValue]":
@@ -1402,6 +1394,12 @@ def _get_direct_parametrize_args(node: nodes.Node) -> List[str]:
     return parametrize_argnames
 
 
+def deduplicate_names(seq: Iterable[str]) -> Tuple[str, ...]:
+    """De-duplicate the sequence of names while keeping the original order."""
+    # Ideally we would use a set, but it does not preserve insertion order.
+    return tuple(dict.fromkeys(seq))
+
+
 class FixtureManager:
     """pytest fixture definitions and information is stored and managed
     from this class.
@@ -1480,11 +1478,22 @@ def getfixtureinfo(
         usefixtures = tuple(
             arg for mark in node.iter_markers(name="usefixtures") for arg in mark.args
         )
-        initialnames = usefixtures + argnames
-        initialnames, names_closure, arg2fixturedefs = self.getfixtureclosure(
-            initialnames, node, ignore_args=_get_direct_parametrize_args(node)
+        initialnames = deduplicate_names(
+            tuple(self._getautousenames(node.nodeid)) + usefixtures + argnames
+        )
+
+        names_closure, arg2fixturedefs = self.getfixtureclosure(
+            node,
+            initialnames,
+            None,
+            ignore_args=_get_direct_parametrize_args(node),
+        )
+        return FuncFixtureInfo(
+            argnames,
+            initialnames,
+            names_closure,
+            arg2fixturedefs,
         )
-        return FuncFixtureInfo(argnames, initialnames, names_closure, arg2fixturedefs)
 
     def pytest_plugin_registered(self, plugin: _PluggyPlugin) -> None:
         nodeid = None
@@ -1517,45 +1526,40 @@ def _getautousenames(self, nodeid: str) -> Iterator[str]:
 
     def getfixtureclosure(
         self,
-        fixturenames: Tuple[str, ...],
         parentnode: nodes.Node,
+        initialnames: Tuple[str, ...],
+        arg2fixturedefs: Union[Dict[str, Sequence[FixtureDef[Any]]], None],
         ignore_args: Sequence[str] = (),
-    ) -> Tuple[Tuple[str, ...], List[str], Dict[str, Sequence[FixtureDef[Any]]]]:
+    ) -> Tuple[List[str], Dict[str, Sequence[FixtureDef[Any]]]]:
         # Collect the closure of all fixtures, starting with the given
-        # fixturenames as the initial set.  As we have to visit all
-        # factory definitions anyway, we also return an arg2fixturedefs
-        # mapping so that the caller can reuse it and does not have
-        # to re-discover fixturedefs again for each fixturename
+        # initialnames containing function arguments, `usefixture` markers
+        # and `autouse` fixtures as the initial set.  As we have to visit all
+        # factory definitions anyway, we also populate arg2fixturedefs mapping
+        # for the args missing therein so that the caller can reuse it and does
+        # not have to re-discover fixturedefs again for each fixturename
         # (discovering matching fixtures for a given name/node is expensive).
 
-        parentid = parentnode.nodeid
-        fixturenames_closure = list(self._getautousenames(parentid))
-
-        def merge(otherlist: Iterable[str]) -> None:
-            for arg in otherlist:
-                if arg not in fixturenames_closure:
-                    fixturenames_closure.append(arg)
-
-        merge(fixturenames)
+        fixturenames_closure = initialnames
 
-        # At this point, fixturenames_closure contains what we call "initialnames",
-        # which is a set of fixturenames the function immediately requests. We
-        # need to return it as well, so save this.
-        initialnames = tuple(fixturenames_closure)
-
-        arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]] = {}
+        if arg2fixturedefs is None:
+            arg2fixturedefs = {}
         lastlen = -1
+        parentid = parentnode.nodeid
         while lastlen != len(fixturenames_closure):
             lastlen = len(fixturenames_closure)
             for argname in fixturenames_closure:
                 if argname in ignore_args:
                     continue
-                if argname in arg2fixturedefs:
-                    continue
-                fixturedefs = self.getfixturedefs(argname, parentid)
-                if fixturedefs:
-                    arg2fixturedefs[argname] = fixturedefs
-                    merge(fixturedefs[-1].argnames)
+                if argname not in arg2fixturedefs:
+                    fixturedefs = self.getfixturedefs(argname, parentid)
+                    if fixturedefs:
+                        arg2fixturedefs[argname] = fixturedefs
+                else:
+                    fixturedefs = arg2fixturedefs[argname]
+                if fixturedefs and not isinstance(fixturedefs[-1], IdentityFixture):
+                    fixturenames_closure = deduplicate_names(
+                        fixturenames_closure + arg2fixturedefs[argname][-1].argnames
+                    )
 
         def sort_by_scope(arg_name: str) -> Scope:
             try:
@@ -1565,8 +1569,10 @@ def sort_by_scope(arg_name: str) -> Scope:
             else:
                 return fixturedefs[-1]._scope
 
-        fixturenames_closure.sort(key=sort_by_scope, reverse=True)
-        return initialnames, fixturenames_closure, arg2fixturedefs
+        return (
+            sorted(fixturenames_closure, key=sort_by_scope, reverse=True),
+            arg2fixturedefs,
+        )
 
     def pytest_generate_tests(self, metafunc: "Metafunc") -> None:
         """Generate new tests based on parametrized fixtures used by the given metafunc"""

src/_pytest/python.py

@@ -14,6 +14,7 @@
 from pathlib import Path
 from typing import Any
 from typing import Callable
+from typing import cast
 from typing import Dict
 from typing import final
 from typing import Generator
@@ -58,10 +59,11 @@
 from _pytest.deprecated import check_ispytest
 from _pytest.deprecated import INSTANCE_COLLECTOR
 from _pytest.deprecated import NOSE_SUPPORT_METHOD
+from _pytest.fixtures import _get_direct_parametrize_args
 from _pytest.fixtures import FixtureDef
-from _pytest.fixtures import FixtureRequest
 from _pytest.fixtures import FuncFixtureInfo
 from _pytest.fixtures import get_scope_node
+from _pytest.fixtures import IdentityFixture
 from _pytest.main import Session
 from _pytest.mark import MARK_GEN
 from _pytest.mark import ParameterSet
@@ -476,8 +478,6 @@
         definition = FunctionDefinition.from_parent(self, name=name, callobj=funcobj)
         fixtureinfo = definition._fixtureinfo
 
-        # pytest_generate_tests impls call metafunc.parametrize() which fills
-        # metafunc._calls, the outcome of the hook.
         metafunc = Metafunc(
             definition=definition,
             fixtureinfo=fixtureinfo,
@@ -486,23 +486,28 @@
             module=module,
             _ispytest=True,
         )
-        methods = []
+
+        def prune_dependency_tree_if_test_is_directly_parametrized(metafunc: Metafunc):
+            # Direct (those with `indirect=False`) parametrizations taking place in
+            # module/class-specific `pytest_generate_tests` hooks, a.k.a dynamic direct
+            # parametrizations, may have shadowed some fixtures so make sure we update what
+            # the function really needs.
+            if metafunc.has_direct_parametrization:
+                metafunc.update_dependency_tree()
+
+        methods = [prune_dependency_tree_if_test_is_directly_parametrized]
         if hasattr(module, "pytest_generate_tests"):
             methods.append(module.pytest_generate_tests)
         if cls is not None and hasattr(cls, "pytest_generate_tests"):
             methods.append(cls().pytest_generate_tests)
+
+        # pytest_generate_tests impls call metafunc.parametrize() which fills
+        # metafunc._calls, the outcome of the hook.
         self.ihook.pytest_generate_tests.call_extra(methods, dict(metafunc=metafunc))
 
         if not metafunc._calls:
             yield Function.from_parent(self, name=name, fixtureinfo=fixtureinfo)
         else:
-            # Direct parametrizations taking place in module/class-specific
-            # `metafunc.parametrize` calls may have shadowed some fixtures, so make sure
-            # we update what the function really needs a.k.a its fixture closure. Note that
-            # direct parametrizations using `@pytest.mark.parametrize` have already been considered
-            # into making the closure using `ignore_args` arg to `getfixtureclosure`.
-            fixtureinfo.prune_dependency_tree()
-
             for callspec in metafunc._calls:
                 subname = f"{name}[{callspec.id}]"
                 yield Function.from_parent(
@@ -1167,10 +1172,6 @@
         return "-".join(self._idlist)
 
 
-def get_direct_param_fixture_func(request: FixtureRequest) -> Any:
-    return request.param
-
-
 # Used for storing pseudo fixturedefs for direct parametrization.
 name2pseudofixturedef_key = StashKey[Dict[str, FixtureDef[Any]]]()
 
@@ -1219,6 +1220,9 @@
         # Result of parametrize().
         self._calls: List[CallSpec2] = []
 
+        # Whether it's ever been directly parametrized, i.e. with `indirect=False`.
+        self.has_direct_parametrization = False
+
     def parametrize(
         self,
         argnames: Union[str, Sequence[str]],
@@ -1367,18 +1371,14 @@
         for argname in argnames:
             if arg_directness[argname] == "indirect":
                 continue
+            self.has_direct_parametrization = True
             if name2pseudofixturedef is not None and argname in name2pseudofixturedef:
                 fixturedef = name2pseudofixturedef[argname]
             else:
-                fixturedef = FixtureDef(
-                    fixturemanager=self.definition.session._fixturemanager,
-                    baseid="",
-                    argname=argname,
-                    func=get_direct_param_fixture_func,
-                    scope=scope_,
-                    params=None,
-                    unittest=False,
-                    ids=None,
+                fixturedef = IdentityFixture(
+                    self.definition.session._fixturemanager,
+                    argname,
+                    scope_,
                     _ispytest=True,
                 )
                 if name2pseudofixturedef is not None:
@@ -1544,6 +1544,21 @@
                         pytrace=False,
                     )
 
+    def update_dependency_tree(self) -> None:
+        definition = self.definition
+        (
+            fixture_closure,
+            _,
+        ) = cast(
+            nodes.Node, definition.parent
+        ).session._fixturemanager.getfixtureclosure(
+            definition,
+            definition._fixtureinfo.initialnames,
+            definition._fixtureinfo.name2fixturedefs,
+            ignore_args=_get_direct_parametrize_args(definition),
+        )
+        definition._fixtureinfo.names_closure[:] = fixture_closure
+
 
 def _find_parametrized_scope(
     argnames: Sequence[str],