Merge pull request #13976 from bluetech/duplicate-parametrization-fix

fixtures: fix incorrect "duplicate parametrization" when using `indirect=[...]`

Author: bluetech

src/_pytest/fixtures.py

@@ -25,6 +25,7 @@
 from typing import Final
 from typing import final
 from typing import Generic
+from typing import Literal
 from typing import NoReturn
 from typing import overload
 from typing import TYPE_CHECKING
@@ -1472,6 +1473,45 @@ def pytest_cmdline_main(config: Config) -> int | ExitCode | None:
     return None
 
 
+def _resolve_args_directness(
+    argnames: Sequence[str],
+    indirect: bool | Sequence[str],
+    nodeid: str,
+) -> dict[str, Literal["indirect", "direct"]]:
+    """Resolve if each parametrized argument must be considered an indirect
+    parameter to a fixture of the same name, or a direct parameter to the
+    parametrized function, based on the ``indirect`` parameter of the
+    parametrize() call.
+
+    :param argnames:
+        List of argument names passed to ``parametrize()``.
+    :param indirect:
+        Same as the ``indirect`` parameter of ``parametrize()``.
+    :param nodeid:
+        Node ID to which the parametrization is applied.
+    :returns:
+        A dict mapping each arg name to either "indirect" or "direct".
+    """
+    arg_directness: dict[str, Literal["indirect", "direct"]]
+    if isinstance(indirect, bool):
+        arg_directness = dict.fromkeys(argnames, "indirect" if indirect else "direct")
+    elif isinstance(indirect, Sequence):
+        arg_directness = dict.fromkeys(argnames, "direct")
+        for arg in indirect:
+            if arg not in argnames:
+                fail(
+                    f"In {nodeid}: indirect fixture '{arg}' doesn't exist",
+                    pytrace=False,
+                )
+            arg_directness[arg] = "indirect"
+    else:
+        fail(
+            f"In {nodeid}: expected Sequence or boolean for indirect, got {type(indirect).__name__}",
+            pytrace=False,
+        )
+    return arg_directness
+
+
 def _get_direct_parametrize_args(node: nodes.Node) -> set[str]:
     """Return all direct parametrization arguments of a node, so we don't
     mistake them for fixtures.
@@ -1483,11 +1523,16 @@ def _get_direct_parametrize_args(node: nodes.Node) -> set[str]:
     """
     parametrize_argnames: set[str] = set()
     for marker in node.iter_markers(name="parametrize"):
-        if not marker.kwargs.get("indirect", False):
-            p_argnames, _ = ParameterSet._parse_parametrize_args(
-                *marker.args, **marker.kwargs
-            )
-            parametrize_argnames.update(p_argnames)
+        indirect = marker.kwargs.get("indirect", False)
+        p_argnames, _ = ParameterSet._parse_parametrize_args(
+            *marker.args, **marker.kwargs
+        )
+        p_directness = _resolve_args_directness(p_argnames, indirect, node.nodeid)
+        parametrize_argnames.update(
+            argname
+            for argname, directness in p_directness.items()
+            if directness == "direct"
+        )
     return parametrize_argnames
 
 

src/_pytest/python.py

@@ -53,6 +53,7 @@
 from _pytest.config import hookimpl
 from _pytest.config.argparsing import Parser
 from _pytest.deprecated import check_ispytest
+from _pytest.fixtures import _resolve_args_directness
 from _pytest.fixtures import FixtureDef
 from _pytest.fixtures import FixtureRequest
 from _pytest.fixtures import FuncFixtureInfo
@@ -870,7 +871,6 @@ class IdMaker:
     __slots__ = (
         "argnames",
         "config",
-        "func_name",
         "idfn",
         "ids",
         "nodeid",
@@ -893,9 +893,6 @@ class IdMaker:
     # Optionally, the ID of the node being parametrized.
     # Used only for clearer error messages.
     nodeid: str | None
-    # Optionally, the ID of the function being parametrized.
-    # Used only for clearer error messages.
-    func_name: str | None
 
     def make_unique_parameterset_ids(self) -> list[str | _HiddenParam]:
         """Make a unique identifier for each ParameterSet, that may be used to
@@ -1083,9 +1080,7 @@ def _complain_multiple_hidden_parameter_sets(self) -> NoReturn:
         )
 
     def _make_error_prefix(self) -> str:
-        if self.func_name is not None:
-            return f"In {self.func_name}: "
-        elif self.nodeid is not None:
+        if self.nodeid is not None:
             return f"In {self.nodeid}: "
         else:
             return ""
@@ -1333,7 +1328,9 @@ def parametrize(
             object.__setattr__(_param_mark._param_ids_from, "_param_ids_generated", ids)
 
         # Calculate directness.
-        arg_directness = self._resolve_args_directness(argnames, indirect)
+        arg_directness = _resolve_args_directness(
+            argnames, indirect, self.definition.nodeid
+        )
         self._params_directness.update(arg_directness)
 
         # Add direct parametrizations as fixturedefs to arg2fixturedefs by
@@ -1435,23 +1432,21 @@ def _resolve_parameter_set_ids(
             ids_ = None
         else:
             idfn = None
-            ids_ = self._validate_ids(ids, parametersets, self.function.__name__)
+            ids_ = self._validate_ids(ids, parametersets)
         id_maker = IdMaker(
             argnames,
             parametersets,
             idfn,
             ids_,
             self.config,
             nodeid=nodeid,
-            func_name=self.function.__name__,
         )
         return id_maker.make_unique_parameterset_ids()
 
     def _validate_ids(
         self,
         ids: Iterable[object | None],
         parametersets: Sequence[ParameterSet],
-        func_name: str,
     ) -> list[object | None]:
         try:
             num_ids = len(ids)  # type: ignore[arg-type]
@@ -1464,49 +1459,13 @@ def _validate_ids(
 
         # num_ids == 0 is a special case: https://github.com/pytest-dev/pytest/issues/1849
         if num_ids != len(parametersets) and num_ids != 0:
-            msg = "In {}: {} parameter sets specified, with different number of ids: {}"
-            fail(msg.format(func_name, len(parametersets), num_ids), pytrace=False)
-
-        return list(itertools.islice(ids, num_ids))
-
-    def _resolve_args_directness(
-        self,
-        argnames: Sequence[str],
-        indirect: bool | Sequence[str],
-    ) -> dict[str, Literal["indirect", "direct"]]:
-        """Resolve if each parametrized argument must be considered an indirect
-        parameter to a fixture of the same name, or a direct parameter to the
-        parametrized function, based on the ``indirect`` parameter of the
-        parametrized() call.
-
-        :param argnames:
-            List of argument names passed to ``parametrize()``.
-        :param indirect:
-            Same as the ``indirect`` parameter of ``parametrize()``.
-        :returns
-            A dict mapping each arg name to either "indirect" or "direct".
-        """
-        arg_directness: dict[str, Literal["indirect", "direct"]]
-        if isinstance(indirect, bool):
-            arg_directness = dict.fromkeys(
-                argnames, "indirect" if indirect else "direct"
-            )
-        elif isinstance(indirect, Sequence):
-            arg_directness = dict.fromkeys(argnames, "direct")
-            for arg in indirect:
-                if arg not in argnames:
-                    fail(
-                        f"In {self.function.__name__}: indirect fixture '{arg}' doesn't exist",
-                        pytrace=False,
-                    )
-                arg_directness[arg] = "indirect"
-        else:
+            nodeid = self.definition.nodeid
             fail(
-                f"In {self.function.__name__}: expected Sequence or boolean"
-                f" for indirect, got {type(indirect).__name__}",
+                f"In {nodeid}: {len(parametersets)} parameter sets specified, with different number of ids: {num_ids}",
                 pytrace=False,
             )
-        return arg_directness
+
+        return list(itertools.islice(ids, num_ids))
 
     def _validate_if_using_arg_names(
         self,
@@ -1520,12 +1479,12 @@ def _validate_if_using_arg_names(
         :raises ValueError: If validation fails.
         """
         default_arg_names = set(get_default_arg_names(self.function))
-        func_name = self.function.__name__
+        nodeid = self.definition.nodeid
         for arg in argnames:
             if arg not in self.fixturenames:
                 if arg in default_arg_names:
                     fail(
-                        f"In {func_name}: function already takes an argument '{arg}' with a default value",
+                        f"In {nodeid}: function already takes an argument '{arg}' with a default value",
                         pytrace=False,
                     )
                 else:
@@ -1534,7 +1493,7 @@ def _validate_if_using_arg_names(
                     else:
                         name = "fixture" if indirect else "argument"
                     fail(
-                        f"In {func_name}: function uses no {name} '{arg}'",
+                        f"In {nodeid}: function uses no {name} '{arg}'",
                         pytrace=False,
                     )
 

testing/python/collect.py

@@ -524,6 +524,42 @@ def test_overridden_via_param(value):
         rec = pytester.inline_run()
         rec.assertoutcome(passed=1)
 
+    def test_parametrize_overrides_parametrized_fixture_with_unrelated_indirect(
+        self, pytester: Pytester
+    ) -> None:
+        """Test parametrization when parameter overrides existing parametrized fixture with same name,
+        and there is an unrelated indirect param.
+
+        Regression test for #13974.
+        """
+        pytester.makepyfile(
+            """
+            import pytest
+
+            @pytest.fixture(params=["a", "b"])
+            def target(request):
+                return request.param
+
+            @pytest.fixture
+            def val(request):
+                return int(request.param)
+
+            @pytest.mark.parametrize(
+                ["val", "target"],
+                [
+                    ("1", 1),
+                    ("2", 2),
+                ],
+                indirect=["val"],
+            )
+            def test(val, target):
+                assert val == target
+            """
+        )
+        result = pytester.runpytest()
+        assert result.ret == 0
+        result.assert_outcomes(passed=2)
+
     def test_parametrize_overrides_indirect_dependency_fixture(
         self, pytester: Pytester
     ) -> None: