Merge pull request #13438 from bluetech/reorder-items-comments

Some small cleanup/comment changes

Author: bluetech

src/_pytest/fixtures.py

@@ -132,6 +132,12 @@ def get_scope_package(
 
 
 def get_scope_node(node: nodes.Node, scope: Scope) -> nodes.Node | None:
+    """Get the closest parent node (including self) which matches the given
+    scope.
+
+    If there is no parent node for the scope (e.g. asking for class scope on a
+    Module, or on a Function when not defined in a class), returns None.
+    """
     import _pytest.python
 
     if scope is Scope.Function:
@@ -165,22 +171,30 @@ def getfixturemarker(obj: object) -> FixtureFunctionMarker | None:
 
 
 @dataclasses.dataclass(frozen=True)
-class FixtureArgKey:
+class ParamArgKey:
+    """A key for a high-scoped parameter used by an item.
+
+    For use as a hashable key in `reorder_items`. The combination of fields
+    is meant to uniquely identify a particular "instance" of a param,
+    potentially shared by multiple items in a scope.
+    """
+
+    #: The param name.
     argname: str
     param_index: int
+    #: For scopes Package, Module, Class, the path to the file (directory in
+    #: Package's case) of the package/module/class where the item is defined.
     scoped_item_path: Path | None
+    #: For Class scope, the class where the item is defined.
     item_cls: type | None
 
 
 _V = TypeVar("_V")
 OrderedSet = dict[_V, None]
 
 
-def get_parametrized_fixture_argkeys(
-    item: nodes.Item, scope: Scope
-) -> Iterator[FixtureArgKey]:
-    """Return list of keys for all parametrized arguments which match
-    the specified scope."""
+def get_param_argkeys(item: nodes.Item, scope: Scope) -> Iterator[ParamArgKey]:
+    """Return all ParamArgKeys for item matching the specified high scope."""
     assert scope is not Scope.Function
 
     try:
@@ -206,19 +220,17 @@ def get_parametrized_fixture_argkeys(
         if callspec._arg2scope[argname] != scope:
             continue
         param_index = callspec.indices[argname]
-        yield FixtureArgKey(argname, param_index, scoped_item_path, item_cls)
+        yield ParamArgKey(argname, param_index, scoped_item_path, item_cls)
 
 
 def reorder_items(items: Sequence[nodes.Item]) -> list[nodes.Item]:
-    argkeys_by_item: dict[Scope, dict[nodes.Item, OrderedSet[FixtureArgKey]]] = {}
-    items_by_argkey: dict[
-        Scope, dict[FixtureArgKey, OrderedDict[nodes.Item, None]]
-    ] = {}
+    argkeys_by_item: dict[Scope, dict[nodes.Item, OrderedSet[ParamArgKey]]] = {}
+    items_by_argkey: dict[Scope, dict[ParamArgKey, OrderedDict[nodes.Item, None]]] = {}
     for scope in HIGH_SCOPES:
         scoped_argkeys_by_item = argkeys_by_item[scope] = {}
         scoped_items_by_argkey = items_by_argkey[scope] = defaultdict(OrderedDict)
         for item in items:
-            argkeys = dict.fromkeys(get_parametrized_fixture_argkeys(item, scope))
+            argkeys = dict.fromkeys(get_param_argkeys(item, scope))
             if argkeys:
                 scoped_argkeys_by_item[item] = argkeys
                 for argkey in argkeys:
@@ -234,9 +246,9 @@ def reorder_items(items: Sequence[nodes.Item]) -> list[nodes.Item]:
 
 def reorder_items_atscope(
     items: OrderedSet[nodes.Item],
-    argkeys_by_item: Mapping[Scope, Mapping[nodes.Item, OrderedSet[FixtureArgKey]]],
+    argkeys_by_item: Mapping[Scope, Mapping[nodes.Item, OrderedSet[ParamArgKey]]],
     items_by_argkey: Mapping[
-        Scope, Mapping[FixtureArgKey, OrderedDict[nodes.Item, None]]
+        Scope, Mapping[ParamArgKey, OrderedDict[nodes.Item, None]]
     ],
     scope: Scope,
 ) -> OrderedSet[nodes.Item]:
@@ -246,7 +258,7 @@ def reorder_items_atscope(
     scoped_items_by_argkey = items_by_argkey[scope]
     scoped_argkeys_by_item = argkeys_by_item[scope]
 
-    ignore: set[FixtureArgKey] = set()
+    ignore: set[ParamArgKey] = set()
     items_deque = deque(items)
     items_done: OrderedSet[nodes.Item] = {}
     while items_deque:

src/_pytest/mark/structures.py

@@ -78,6 +78,33 @@ def get_empty_parameterset_mark(
 
 
 class ParameterSet(NamedTuple):
+    """A set of values for a set of parameters along with associated marks and
+    an optional ID for the set.
+
+    Examples::
+
+        pytest.param(1, 2, 3)
+        # ParameterSet(values=(1, 2, 3), marks=(), id=None)
+
+        pytest.param("hello", id="greeting")
+        # ParameterSet(values=("hello",), marks=(), id="greeting")
+
+        # Parameter set with marks
+        pytest.param(42, marks=pytest.mark.xfail)
+        # ParameterSet(values=(42,), marks=(MarkDecorator(...),), id=None)
+
+        # From parametrize mark (parameter names + list of parameter sets)
+        pytest.mark.parametrize(
+            ("a", "b", "expected"),
+            [
+                (1, 2, 3),
+                pytest.param(40, 2, 42, id="everything"),
+            ],
+        )
+        # ParameterSet(values=(1, 2, 3), marks=(), id=None)
+        # ParameterSet(values=(2, 2, 3), marks=(), id="everything")
+    """
+
     values: Sequence[object | NotSetType]
     marks: Collection[MarkDecorator | Mark]
     id: str | _HiddenParam | None

src/_pytest/python.py

@@ -1056,6 +1056,7 @@ class CallSpec2:
     params: dict[str, object] = dataclasses.field(default_factory=dict)
     # arg name -> arg index.
     indices: dict[str, int] = dataclasses.field(default_factory=dict)
+    # arg name -> parameter scope.
     # Used for sorting parametrized resources.
     _arg2scope: Mapping[str, Scope] = dataclasses.field(default_factory=dict)
     # Parts which will be added to the item's name in `[..]` separated by "-".
@@ -1168,7 +1169,7 @@ def parametrize(
         """Add new invocations to the underlying test function using the list
         of argvalues for the given argnames. Parametrization is performed
         during the collection phase. If you need to setup expensive resources
-        see about setting indirect to do it rather than at test setup time.
+        see about setting ``indirect`` to do it at test setup time instead.
 
         Can be called multiple times per test function (but only on different
         argument names), in which case each call parametrizes all previous
@@ -1192,7 +1193,7 @@ def parametrize(
             If N argnames were specified, argvalues must be a list of
             N-tuples, where each tuple-element specifies a value for its
             respective argname.
-        :type argvalues: Iterable[_pytest.mark.structures.ParameterSet | Sequence[object] | object]
+
         :param indirect:
             A list of arguments' names (subset of argnames) or a boolean.
             If True the list contains all names from the argnames. Each
@@ -1270,15 +1271,20 @@ def parametrize(
         if _param_mark and _param_mark._param_ids_from and generated_ids is None:
             object.__setattr__(_param_mark._param_ids_from, "_param_ids_generated", ids)
 
-        # Add funcargs as fixturedefs to fixtureinfo.arg2fixturedefs by registering
-        # artificial "pseudo" FixtureDef's so that later at test execution time we can
-        # rely on a proper FixtureDef to exist for fixture setup.
+        # Calculate directness.
+        arg_directness = self._resolve_args_directness(argnames, indirect)
+        self._params_directness.update(arg_directness)
+
+        # Add direct parametrizations as fixturedefs to arg2fixturedefs by
+        # registering artificial "pseudo" FixtureDef's such that later at test
+        # setup time we can rely on FixtureDefs to exist for all argnames.
         node = None
-        # If we have a scope that is higher than function, we need
-        # to make sure we only ever create an according fixturedef on
-        # a per-scope basis. We thus store and cache the fixturedef on the
-        # node related to the scope.
-        if scope_ is not Scope.Function:
+        # For scopes higher than function, a "pseudo" FixtureDef might have
+        # already been created for the scope. We thus store and cache the
+        # FixtureDef on the node related to the scope.
+        if scope_ is Scope.Function:
+            name2pseudofixturedef = None
+        else:
             collector = self.definition.parent
             assert collector is not None
             node = get_scope_node(collector, scope_)
@@ -1294,15 +1300,10 @@ def parametrize(
                     node = collector.session
                 else:
                     assert False, f"Unhandled missing scope: {scope}"
-        if node is None:
-            name2pseudofixturedef = None
-        else:
             default: dict[str, FixtureDef[Any]] = {}
             name2pseudofixturedef = node.stash.setdefault(
                 name2pseudofixturedef_key, default
             )
-        arg_directness = self._resolve_args_directness(argnames, indirect)
-        self._params_directness.update(arg_directness)
         for argname in argnames:
             if arg_directness[argname] == "indirect":
                 continue