Fix a bug in tests;Add docstring to them;Apply review comments

Author: sadra-barikbin

src/_pytest/fixtures.py

@@ -1074,7 +1074,7 @@ def __repr__(self) -> str:
         )
 
 
-class IdentityFixture(FixtureDef[FixtureValue]):
+class IdentityFixtureDef(FixtureDef[FixtureValue]):
     def __init__(
         self,
         fixturemanager: "FixtureManager",
@@ -1476,11 +1476,11 @@ def getfixtureinfo(
         initialnames = deduplicate_names(autousenames, usefixturesnames, argnames)
 
         direct_parametrize_args = _get_direct_parametrize_args(node)
-
-        names_closure, arg2fixturedefs = self.getfixtureclosure(
+        arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]] = {}
+        names_closure = self.getfixtureclosure(
             parentnode=node,
             initialnames=initialnames,
-            arg2fixturedefs=None,
+            arg2fixturedefs=arg2fixturedefs,
             ignore_args=direct_parametrize_args,
         )
 
@@ -1524,9 +1524,9 @@ def getfixtureclosure(
         self,
         parentnode: nodes.Node,
         initialnames: Tuple[str, ...],
-        arg2fixturedefs: Union[Dict[str, Sequence[FixtureDef[Any]]], None],
+        arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]],
         ignore_args: AbstractSet[str],
-    ) -> Tuple[List[str], Dict[str, Sequence[FixtureDef[Any]]]]:
+    ) -> List[str]:
         # Collect the closure of all fixtures, starting with the given
         # initialnames containing function arguments, `usefixture` markers
         # and `autouse` fixtures as the initial set.  As we have to visit all
@@ -1535,8 +1535,6 @@ def getfixtureclosure(
         # not have to re-discover fixturedefs again for each fixturename
         # (discovering matching fixtures for a given name/node is expensive).
 
-        if arg2fixturedefs is None:
-            arg2fixturedefs = {}
         parentid = parentnode.nodeid
         fixturenames_closure = list(initialnames)
 
@@ -1552,7 +1550,7 @@ def getfixtureclosure(
                         arg2fixturedefs[argname] = fixturedefs
                 else:
                     fixturedefs = arg2fixturedefs[argname]
-                if fixturedefs and not isinstance(fixturedefs[-1], IdentityFixture):
+                if fixturedefs and not isinstance(fixturedefs[-1], IdentityFixtureDef):
                     for arg in fixturedefs[-1].argnames:
                         if arg not in fixturenames_closure:
                             fixturenames_closure.append(arg)
@@ -1566,7 +1564,7 @@ def sort_by_scope(arg_name: str) -> Scope:
                 return fixturedefs[-1]._scope
 
         fixturenames_closure.sort(key=sort_by_scope, reverse=True)
-        return 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

@@ -14,7 +14,6 @@
 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
@@ -63,7 +62,7 @@
 from _pytest.fixtures import FixtureDef
 from _pytest.fixtures import FuncFixtureInfo
 from _pytest.fixtures import get_scope_node
-from _pytest.fixtures import IdentityFixture
+from _pytest.fixtures import IdentityFixtureDef
 from _pytest.main import Session
 from _pytest.mark import MARK_GEN
 from _pytest.mark import ParameterSet
@@ -492,10 +491,14 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]:
         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()
+            # parametrizations using `metafunc.parametrize`, may have shadowed some
+            # fixtures, making some fixtures no longer reachable. Update the dependency
+            # tree to reflect what the item really needs.
+            # Note that direct parametrizations using `@pytest.mark.parametrize` have
+            # already been considered into making the closure using the `ignore_args`
+            # arg to `getfixtureclosure`.
+            if metafunc._has_direct_parametrization:
+                metafunc._update_dependency_tree()
 
         methods = [prune_dependency_tree_if_test_is_directly_parametrized]
         if hasattr(module, "pytest_generate_tests"):
@@ -1223,7 +1226,7 @@ def __init__(
         self._calls: List[CallSpec2] = []
 
         # Whether it's ever been directly parametrized, i.e. with `indirect=False`.
-        self.has_direct_parametrization = False
+        self._has_direct_parametrization = False
 
     def parametrize(
         self,
@@ -1373,11 +1376,11 @@ def parametrize(
         for argname in argnames:
             if arg_directness[argname] == "indirect":
                 continue
-            self.has_direct_parametrization = True
+            self._has_direct_parametrization = True
             if name2pseudofixturedef is not None and argname in name2pseudofixturedef:
                 fixturedef = name2pseudofixturedef[argname]
             else:
-                fixturedef = IdentityFixture(
+                fixturedef = IdentityFixtureDef(
                     self.definition.session._fixturemanager,
                     argname,
                     scope_,
@@ -1546,10 +1549,11 @@ def _validate_if_using_arg_names(
                         pytrace=False,
                     )
 
-    def update_dependency_tree(self) -> None:
+    def _update_dependency_tree(self) -> None:
         definition = self.definition
-        fm = cast(nodes.Node, definition.parent).session._fixturemanager
-        fixture_closure, _ = fm.getfixtureclosure(
+        assert definition.parent is not None
+        fm = definition.parent.session._fixturemanager
+        fixture_closure = fm.getfixtureclosure(
             parentnode=definition,
             initialnames=definition._fixtureinfo.initialnames,
             arg2fixturedefs=definition._fixtureinfo.name2fixturedefs,

testing/python/fixtures.py

@@ -4535,6 +4535,11 @@ def test_fixt(custom):
 
 
 def test_fixture_info_after_dynamic_parametrize(pytester: Pytester) -> None:
+    """
+    Item dependency tree should get prunned before `FixtureManager::pytest_generate_tests`
+    hook implementation because it attempts to parametrize on the fixtures in the
+    fixture closure.
+    """
     pytester.makeconftest(
         """
         import pytest
@@ -4559,11 +4564,7 @@ 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):
+        def fixture2(fixture3):
             pass
 
         def test(fixture2):
@@ -4575,6 +4576,8 @@ def test(fixture2):
 
 
 def test_reordering_after_dynamic_parametrize(pytester: Pytester):
+    """Make sure that prunning dependency tree takes place correctly, regarding from
+    reordering's viewpoint."""
     pytester.makepyfile(
         """
         import pytest
@@ -4583,7 +4586,7 @@ def pytest_generate_tests(metafunc):
             if metafunc.definition.name == "test_0":
                 metafunc.parametrize("fixture2", [0])
 
-        @pytest.fixture(scope='module')
+        @pytest.fixture(scope='module', params=[0])
         def fixture1():
             pass
 
@@ -4615,6 +4618,9 @@ def test_2(fixture1):
 def test_request_shouldnt_be_in_closure_after_pruning_dep_tree_when_its_not_in_initial_closure(
     pytester: Pytester,
 ):
+    """Make sure that fixture `request` doesn't show up in the closure after prunning dependency
+    tree when it has not been there beforehand.
+    """
     pytester.makepyfile(
         """
         import pytest
@@ -4641,6 +4647,10 @@ def test(fixture, arg):
 def test_dont_recompute_dependency_tree_if_no_direct_dynamic_parametrize(
     pytester: Pytester,
 ):
+    """We should not update item's dependency tree when there's no direct dynamic
+    parametrization, i.e. `metafunc.parametrize(indirect=False)`s in module/class specific
+    `pytest_generate_tests` hooks, for the sake of efficiency.
+    """
     pytester.makeconftest(
         """
         import pytest