Apply comments and and an improvement

Author: sadra-barikbin

src/_pytest/fixtures.py

@@ -1404,6 +1404,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.
@@ -1482,13 +1488,8 @@ def getfixtureinfo(
         usefixtures = tuple(
             arg for mark in node.iter_markers(name="usefixtures") for arg in mark.args
         )
-        initialnames = cast(
-            Tuple[str],
-            tuple(
-                dict.fromkeys(
-                    tuple(self._getautousenames(node.nodeid)) + usefixtures + argnames
-                )
-            ),
+        initialnames = deduplicate_names(
+            tuple(self._getautousenames(node.nodeid)) + usefixtures + argnames
         )
 
         arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]] = {}
@@ -1537,23 +1538,19 @@ def _getautousenames(self, nodeid: str) -> Iterator[str]:
     def getfixtureclosure(
         self,
         parentnode: nodes.Node,
-        initialnames: Tuple[str],
+        initialnames: Tuple[str, ...],
         arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]],
         ignore_args: Sequence[str] = (),
     ) -> List[str]:
         # Collect the closure of all fixtures, starting with the given
-        # 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
+        # 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).
 
-        fixturenames_closure = list(initialnames)
-
-        def merge(otherlist: Iterable[str]) -> None:
-            for arg in otherlist:
-                if arg not in fixturenames_closure:
-                    fixturenames_closure.append(arg)
+        fixturenames_closure = initialnames
 
         lastlen = -1
         parentid = parentnode.nodeid
@@ -1567,7 +1564,9 @@ def merge(otherlist: Iterable[str]) -> None:
                     if fixturedefs:
                         arg2fixturedefs[argname] = fixturedefs
                 if argname in arg2fixturedefs:
-                    merge(arg2fixturedefs[argname][-1].argnames)
+                    fixturenames_closure = deduplicate_names(
+                        fixturenames_closure + arg2fixturedefs[argname][-1].argnames
+                    )
 
         def sort_by_scope(arg_name: str) -> Scope:
             try:
@@ -1577,8 +1576,7 @@ def sort_by_scope(arg_name: str) -> Scope:
             else:
                 return fixturedefs[-1]._scope
 
-        fixturenames_closure.sort(key=sort_by_scope, reverse=True)
-        return fixturenames_closure
+        return sorted(fixturenames_closure, key=sort_by_scope, reverse=True)
 
     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,7 +11,6 @@
 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
@@ -379,12 +378,13 @@ 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:
+def prune_dependency_tree_if_test_is_dynamically_parametrized(metafunc):
+    if metafunc._calls:
         # Dynamic direct parametrization may have shadowed some fixtures
-        # so make sure we update what the function really needs.
+        # so make sure we update what the function really needs. Note that
+        # we didn't need to do this if only indirect dynamic parametrization
+        # had taken place, but anyway we did it as differentiating between direct
+        # and indirect requires a dirty hack.
         definition = metafunc.definition
         fixture_closure = definition.parent.session._fixturemanager.getfixtureclosure(
             definition,
@@ -393,7 +393,6 @@ def unwrap_metafunc_parametrize_and_possibly_prune_dependency_tree(metafunc):
             ignore_args=_get_direct_parametrize_args(definition) + ["request"],
         )
         definition._fixtureinfo.names_closure[:] = fixture_closure
-    del metafunc.has_dynamic_parametrize
 
 
 class PyCollector(PyobjMixin, nodes.Collector):
@@ -500,22 +499,12 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]:
             module=module,
             _ispytest=True,
         )
-        methods = [unwrap_metafunc_parametrize_and_possibly_prune_dependency_tree]
+        methods = [prune_dependency_tree_if_test_is_dynamically_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)
 
-        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))

testing/python/fixtures.py

@@ -4683,3 +4683,10 @@ def test(fm):
     )
     reprec = pytester.inline_run()
     reprec.assertoutcome(passed=5)
+
+
+def test_deduplicate_names(pytester: Pytester) -> None:
+    items = fixtures.deduplicate_names("abacd")
+    assert items == ("a", "b", "c", "d")
+    items = fixtures.deduplicate_names(items + ("g", "f", "g", "e", "b"))
+    assert items == ("a", "b", "c", "d", "g", "f", "e")