Remove prune_dependency_tree and reuse getfixtureclosure logic

Author: sadra-barikbin

src/_pytest/fixtures.py

@@ -383,33 +383,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:
     """A request for a fixture from a test or fixture function.
@@ -1502,11 +1475,28 @@ 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 = cast(
+            Tuple[str],
+            tuple(
+                dict.fromkeys(
+                    tuple(self._getautousenames(node.nodeid)) + usefixtures + argnames
+                )
+            ),
+        )
+
+        arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]] = {}
+        names_closure = self.getfixtureclosure(
+            node,
+            initialnames,
+            arg2fixturedefs,
+            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
@@ -1539,45 +1529,38 @@ def _getautousenames(self, nodeid: str) -> Iterator[str]:
 
     def getfixtureclosure(
         self,
-        fixturenames: Tuple[str, ...],
         parentnode: nodes.Node,
+        initialnames: Tuple[str],
+        arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]],
         ignore_args: Sequence[str] = (),
-    ) -> Tuple[Tuple[str, ...], List[str], Dict[str, Sequence[FixtureDef[Any]]]]:
+    ) -> List[str]:
         # 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
+        # initialnames as the initial set.  As we have to visit all
+        # factory definitions anyway, we also populate arg2fixturedefs
         # mapping 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))
+        fixturenames_closure = list(initialnames)
 
         def merge(otherlist: Iterable[str]) -> None:
             for arg in otherlist:
                 if arg not in fixturenames_closure:
                     fixturenames_closure.append(arg)
 
-        merge(fixturenames)
-
-        # 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]]] = {}
         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 not in arg2fixturedefs:
+                    fixturedefs = self.getfixturedefs(argname, parentid)
+                    if fixturedefs:
+                        arg2fixturedefs[argname] = fixturedefs
                 if argname in arg2fixturedefs:
-                    continue
-                fixturedefs = self.getfixturedefs(argname, parentid)
-                if fixturedefs:
-                    arg2fixturedefs[argname] = fixturedefs
-                    merge(fixturedefs[-1].argnames)
+                    merge(arg2fixturedefs[argname][-1].argnames)
 
         def sort_by_scope(arg_name: str) -> Scope:
             try:
@@ -1588,7 +1571,7 @@ def sort_by_scope(arg_name: str) -> Scope:
                 return fixturedefs[-1]._scope
 
         fixturenames_closure.sort(key=sort_by_scope, reverse=True)
-        return initialnames, fixturenames_closure, arg2fixturedefs
+        return fixturenames_closure
 
     def pytest_generate_tests(self, metafunc: "Metafunc") -> None:
         """Generate new tests based on parametrized fixtures used by the given metafunc"""

src/_pytest/python.py

@@ -11,6 +11,7 @@
 from collections import Counter
 from collections import defaultdict
 from functools import partial
+from functools import wraps
 from pathlib import Path
 from typing import Any
 from typing import Callable
@@ -59,6 +60,7 @@
 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 FuncFixtureInfo
 from _pytest.main import Session
 from _pytest.mark import MARK_GEN
@@ -380,6 +382,23 @@ class _EmptyClass: pass  # noqa: E701
 # fmt: on
 
 
+def unwrap_metafunc_parametrize_and_possibly_prune_dependency_tree(metafunc):
+    metafunc.parametrize = metafunc._parametrize
+    del metafunc._parametrize
+    if metafunc.has_dynamic_parametrize:
+        # Dynamic direct parametrization may have shadowed some fixtures
+        # so make sure we update what the function really needs.
+        definition = metafunc.definition
+        fixture_closure = definition.parent.session._fixturemanager.getfixtureclosure(
+            definition,
+            definition._fixtureinfo.initialnames,
+            definition._fixtureinfo.name2fixturedefs,
+            ignore_args=_get_direct_parametrize_args(definition) + ["request"],
+        )
+        definition._fixtureinfo.names_closure[:] = fixture_closure
+    del metafunc.has_dynamic_parametrize
+
+
 class PyCollector(PyobjMixin, nodes.Collector):
     def funcnamefilter(self, name: str) -> bool:
         return self._matches_prefix_or_glob_option("python_functions", name)
@@ -476,8 +495,6 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]:
         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,11 +503,24 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]:
             module=module,
             _ispytest=True,
         )
-        methods = []
+        methods = [unwrap_metafunc_parametrize_and_possibly_prune_dependency_tree]
         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)
+
+        setattr(metafunc, "has_dynamic_parametrize", False)
+
+        @wraps(metafunc.parametrize)
+        def set_has_dynamic_parametrize(*args, **kwargs):
+            setattr(metafunc, "has_dynamic_parametrize", True)
+            metafunc._parametrize(*args, **kwargs)  # type: ignore[attr-defined]
+
+        setattr(metafunc, "_parametrize", metafunc.parametrize)
+        setattr(metafunc, "parametrize", set_has_dynamic_parametrize)
+
+        # 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:
@@ -500,11 +530,6 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]:
             fm = self.session._fixturemanager
             fixtures.add_funcarg_pseudo_fixture_def(self, metafunc, fm)
 
-            # Add_funcarg_pseudo_fixture_def may have shadowed some fixtures
-            # with direct parametrization, so make sure we update what the
-            # function really needs.
-            fixtureinfo.prune_dependency_tree()
-
             for callspec in metafunc._calls:
                 subname = f"{name}[{callspec.id}]"
                 yield Function.from_parent(

testing/python/fixtures.py

@@ -4536,3 +4536,150 @@ def test_fixt(custom):
     result.assert_outcomes(errors=1)
     result.stdout.fnmatch_lines([expected])
     assert result.ret == ExitCode.TESTS_FAILED
+
+
+@pytest.mark.xfail(
+    reason="arg2fixturedefs should get updated on dynamic parametrize. This gets solved by PR#11220"
+)
+def test_fixture_info_after_dynamic_parametrize(pytester: Pytester) -> None:
+    pytester.makeconftest(
+        """
+        import pytest
+
+        @pytest.fixture(scope='session', params=[0, 1])
+        def fixture1(request):
+            pass
+
+        @pytest.fixture(scope='session')
+        def fixture2(fixture1):
+            pass
+
+        @pytest.fixture(scope='session', params=[2, 3])
+        def fixture3(request, fixture2):
+            pass
+        """
+    )
+    pytester.makepyfile(
+        """
+        import pytest
+        def pytest_generate_tests(metafunc):
+            metafunc.parametrize("fixture2", [4, 5], scope='session')
+
+        @pytest.fixture(scope='session')
+        def fixture4():
+            pass
+
+        @pytest.fixture(scope='session')
+        def fixture2(fixture3, fixture4):
+            pass
+
+        def test(fixture2):
+            assert fixture2 in (4, 5)
+        """
+    )
+    res = pytester.inline_run("-s")
+    res.assertoutcome(passed=2)
+
+
+def test_reordering_after_dynamic_parametrize(pytester: Pytester):
+    pytester.makepyfile(
+        """
+        import pytest
+
+        def pytest_generate_tests(metafunc):
+            if metafunc.definition.name == "test_0":
+                metafunc.parametrize("fixture2", [0])
+
+        @pytest.fixture(scope='module')
+        def fixture1():
+            pass
+
+        @pytest.fixture(scope='module')
+        def fixture2(fixture1):
+            pass
+
+        def test_0(fixture2):
+            pass
+
+        def test_1():
+            pass
+
+        def test_2(fixture1):
+            pass
+        """
+    )
+    result = pytester.runpytest("--collect-only")
+    result.stdout.fnmatch_lines(
+        [
+            "*test_0*",
+            "*test_1*",
+            "*test_2*",
+        ],
+        consecutive=True,
+    )
+
+
+def test_dont_recompute_dependency_tree_if_no_dynamic_parametrize(pytester: Pytester):
+    pytester.makeconftest(
+        """
+        import pytest
+        from _pytest.config import hookimpl
+        from unittest.mock import Mock
+
+        original_method = None
+
+        @hookimpl(trylast=True)
+        def pytest_sessionstart(session):
+            global original_method
+            original_method = session._fixturemanager.getfixtureclosure
+            session._fixturemanager.getfixtureclosure = Mock(wraps=original_method)
+
+        @hookimpl(tryfirst=True)
+        def pytest_sessionfinish(session, exitstatus):
+            global original_method
+            session._fixturemanager.getfixtureclosure = original_method
+        """
+    )
+    pytester.makepyfile(
+        """
+        import pytest
+
+        def pytest_generate_tests(metafunc):
+            if metafunc.definition.name == "test_0":
+                metafunc.parametrize("fixture", [0])
+
+        @pytest.fixture(scope='module')
+        def fixture():
+            pass
+
+        def test_0(fixture):
+            pass
+
+        def test_1():
+            pass
+
+        @pytest.mark.parametrize("fixture", [0])
+        def test_2(fixture):
+            pass
+
+        @pytest.mark.parametrize("fixture", [0], indirect=True)
+        def test_3(fixture):
+            pass
+
+        @pytest.fixture
+        def fm(request):
+            yield request._fixturemanager
+
+        def test(fm):
+            method = fm.getfixtureclosure
+            assert len(method.call_args_list) == 6
+            assert method.call_args_list[0].args[0].nodeid.endswith("test_0")
+            assert method.call_args_list[1].args[0].nodeid.endswith("test_0")
+            assert method.call_args_list[2].args[0].nodeid.endswith("test_1")
+            assert method.call_args_list[3].args[0].nodeid.endswith("test_2")
+            assert method.call_args_list[4].args[0].nodeid.endswith("test_3")
+            assert method.call_args_list[5].args[0].nodeid.endswith("test")
+        """
+    )
+    reprec = pytester.inline_run()
+    reprec.assertoutcome(passed=5)