Merge pull request #9547 from bluetech/refactor-idmaker

Refactor idmaker functions into class IdMaker

src/_pytest/python.py

@@ -929,6 +929,139 @@ def hasnew(obj: object) -> bool:
     return False
 
 
+@final
+@attr.s(frozen=True, auto_attribs=True, slots=True)
+class IdMaker:
+    """Make IDs for a parametrization."""
+
+    # The argnames of the parametrization.
+    argnames: Sequence[str]
+    # The ParameterSets of the parametrization.
+    parametersets: Sequence[ParameterSet]
+    # Optionally, a user-provided callable to make IDs for parameters in a
+    # ParameterSet.
+    idfn: Optional[Callable[[Any], Optional[object]]]
+    # Optionally, explicit IDs for ParameterSets by index.
+    ids: Optional[Sequence[Union[None, str]]]
+    # Optionally, the pytest config.
+    # Used for controlling ASCII escaping, and for calling the
+    # :hook:`pytest_make_parametrize_id` hook.
+    config: Optional[Config]
+    # Optionally, the ID of the node being parametrized.
+    # Used only for clearer error messages.
+    nodeid: Optional[str]
+
+    def make_unique_parameterset_ids(self) -> List[str]:
+        """Make a unique identifier for each ParameterSet, that may be used to
+        identify the parametrization in a node ID.
+
+        Format is <prm_1_token>-...-<prm_n_token>[counter], where prm_x_token is
+        - user-provided id, if given
+        - else an id derived from the value, applicable for certain types
+        - else <argname><parameterset index>
+        The counter suffix is appended only in case a string wouldn't be unique
+        otherwise.
+        """
+        resolved_ids = list(self._resolve_ids())
+        # All IDs must be unique!
+        if len(resolved_ids) != len(set(resolved_ids)):
+            # Record the number of occurrences of each ID.
+            id_counts = Counter(resolved_ids)
+            # Map the ID to its next suffix.
+            id_suffixes: Dict[str, int] = defaultdict(int)
+            # Suffix non-unique IDs to make them unique.
+            for index, id in enumerate(resolved_ids):
+                if id_counts[id] > 1:
+                    resolved_ids[index] = f"{id}{id_suffixes[id]}"
+                    id_suffixes[id] += 1
+        return resolved_ids
+
+    def _resolve_ids(self) -> Iterable[str]:
+        """Resolve IDs for all ParameterSets (may contain duplicates)."""
+        for idx, parameterset in enumerate(self.parametersets):
+            if parameterset.id is not None:
+                # ID provided directly - pytest.param(..., id="...")
+                yield parameterset.id
+            elif self.ids and idx < len(self.ids) and self.ids[idx] is not None:
+                # ID provided in the IDs list - parametrize(..., ids=[...]).
+                id = self.ids[idx]
+                assert id is not None
+                yield _ascii_escaped_by_config(id, self.config)
+            else:
+                # ID not provided - generate it.
+                yield "-".join(
+                    self._idval(val, argname, idx)
+                    for val, argname in zip(parameterset.values, self.argnames)
+                )
+
+    def _idval(self, val: object, argname: str, idx: int) -> str:
+        """Make an ID for a parameter in a ParameterSet."""
+        idval = self._idval_from_function(val, argname, idx)
+        if idval is not None:
+            return idval
+        idval = self._idval_from_hook(val, argname)
+        if idval is not None:
+            return idval
+        idval = self._idval_from_value(val)
+        if idval is not None:
+            return idval
+        return self._idval_from_argname(argname, idx)
+
+    def _idval_from_function(
+        self, val: object, argname: str, idx: int
+    ) -> Optional[str]:
+        """Try to make an ID for a parameter in a ParameterSet using the
+        user-provided id callable, if given."""
+        if self.idfn is None:
+            return None
+        try:
+            id = self.idfn(val)
+        except Exception as e:
+            prefix = f"{self.nodeid}: " if self.nodeid is not None else ""
+            msg = "error raised while trying to determine id of parameter '{}' at position {}"
+            msg = prefix + msg.format(argname, idx)
+            raise ValueError(msg) from e
+        if id is None:
+            return None
+        return self._idval_from_value(id)
+
+    def _idval_from_hook(self, val: object, argname: str) -> Optional[str]:
+        """Try to make an ID for a parameter in a ParameterSet by calling the
+        :hook:`pytest_make_parametrize_id` hook."""
+        if self.config:
+            id: Optional[str] = self.config.hook.pytest_make_parametrize_id(
+                config=self.config, val=val, argname=argname
+            )
+            return id
+        return None
+
+    def _idval_from_value(self, val: object) -> Optional[str]:
+        """Try to make an ID for a parameter in a ParameterSet from its value,
+        if the value type is supported."""
+        if isinstance(val, STRING_TYPES):
+            return _ascii_escaped_by_config(val, self.config)
+        elif val is None or isinstance(val, (float, int, bool, complex)):
+            return str(val)
+        elif isinstance(val, Pattern):
+            return ascii_escaped(val.pattern)
+        elif val is NOTSET:
+            # Fallback to default. Note that NOTSET is an enum.Enum.
+            pass
+        elif isinstance(val, enum.Enum):
+            return str(val)
+        elif isinstance(getattr(val, "__name__", None), str):
+            # Name of a class, function, module, etc.
+            name: str = getattr(val, "__name__")
+            return name
+        return None
+
+    @staticmethod
+    def _idval_from_argname(argname: str, idx: int) -> str:
+        """Make an ID for a parameter in a ParameterSet from the argument name
+        and the index of the ParameterSet."""
+        return str(argname) + str(idx)
+
+
 @final
 @attr.s(frozen=True, slots=True, auto_attribs=True)
 class CallSpec2:
@@ -1217,12 +1350,15 @@ class Metafunc:
         else:
             idfn = None
             ids_ = self._validate_ids(ids, parametersets, self.function.__name__)
-        return idmaker(argnames, parametersets, idfn, ids_, self.config, nodeid=nodeid)
+        id_maker = IdMaker(
+            argnames, parametersets, idfn, ids_, self.config, nodeid=nodeid
+        )
+        return id_maker.make_unique_parameterset_ids()
 
     def _validate_ids(
         self,
         ids: Iterable[Union[None, str, float, int, bool]],
-        parameters: Sequence[ParameterSet],
+        parametersets: Sequence[ParameterSet],
         func_name: str,
     ) -> List[Union[None, str]]:
         try:
@@ -1232,12 +1368,12 @@ class Metafunc:
                 iter(ids)
             except TypeError as e:
                 raise TypeError("ids must be a callable or an iterable") from e
-            num_ids = len(parameters)
+            num_ids = len(parametersets)
 
         # num_ids == 0 is a special case: https://github.com/pytest-dev/pytest/issues/1849
-        if num_ids != len(parameters) and num_ids != 0:
+        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(parameters), num_ids), pytrace=False)
+            fail(msg.format(func_name, len(parametersets), num_ids), pytrace=False)
 
         new_ids = []
         for idx, id_value in enumerate(itertools.islice(ids, num_ids)):
@@ -1374,105 +1510,6 @@ def _ascii_escaped_by_config(val: Union[str, bytes], config: Optional[Config]) -
     return val if escape_option else ascii_escaped(val)  # type: ignore
 
 
-def _idval(
-    val: object,
-    argname: str,
-    idx: int,
-    idfn: Optional[Callable[[Any], Optional[object]]],
-    nodeid: Optional[str],
-    config: Optional[Config],
-) -> str:
-    if idfn:
-        try:
-            generated_id = idfn(val)
-            if generated_id is not None:
-                val = generated_id
-        except Exception as e:
-            prefix = f"{nodeid}: " if nodeid is not None else ""
-            msg = "error raised while trying to determine id of parameter '{}' at position {}"
-            msg = prefix + msg.format(argname, idx)
-            raise ValueError(msg) from e
-    elif config:
-        hook_id: Optional[str] = config.hook.pytest_make_parametrize_id(
-            config=config, val=val, argname=argname
-        )
-        if hook_id:
-            return hook_id
-
-    if isinstance(val, STRING_TYPES):
-        return _ascii_escaped_by_config(val, config)
-    elif val is None or isinstance(val, (float, int, bool, complex)):
-        return str(val)
-    elif isinstance(val, Pattern):
-        return ascii_escaped(val.pattern)
-    elif val is NOTSET:
-        # Fallback to default. Note that NOTSET is an enum.Enum.
-        pass
-    elif isinstance(val, enum.Enum):
-        return str(val)
-    elif isinstance(getattr(val, "__name__", None), str):
-        # Name of a class, function, module, etc.
-        name: str = getattr(val, "__name__")
-        return name
-    return str(argname) + str(idx)
-
-
-def _idvalset(
-    idx: int,
-    parameterset: ParameterSet,
-    argnames: Iterable[str],
-    idfn: Optional[Callable[[Any], Optional[object]]],
-    ids: Optional[List[Union[None, str]]],
-    nodeid: Optional[str],
-    config: Optional[Config],
-) -> str:
-    if parameterset.id is not None:
-        return parameterset.id
-    id = None if ids is None or idx >= len(ids) else ids[idx]
-    if id is None:
-        this_id = [
-            _idval(val, argname, idx, idfn, nodeid=nodeid, config=config)
-            for val, argname in zip(parameterset.values, argnames)
-        ]
-        return "-".join(this_id)
-    else:
-        return _ascii_escaped_by_config(id, config)
-
-
-def idmaker(
-    argnames: Iterable[str],
-    parametersets: Iterable[ParameterSet],
-    idfn: Optional[Callable[[Any], Optional[object]]] = None,
-    ids: Optional[List[Union[None, str]]] = None,
-    config: Optional[Config] = None,
-    nodeid: Optional[str] = None,
-) -> List[str]:
-    resolved_ids = [
-        _idvalset(
-            valindex, parameterset, argnames, idfn, ids, config=config, nodeid=nodeid
-        )
-        for valindex, parameterset in enumerate(parametersets)
-    ]
-
-    # All IDs must be unique!
-    unique_ids = set(resolved_ids)
-    if len(unique_ids) != len(resolved_ids):
-
-        # Record the number of occurrences of each test ID.
-        test_id_counts = Counter(resolved_ids)
-
-        # Map the test ID to its next suffix.
-        test_id_suffixes: Dict[str, int] = defaultdict(int)
-
-        # Suffix non-unique IDs to make them unique.
-        for index, test_id in enumerate(resolved_ids):
-            if test_id_counts[test_id] > 1:
-                resolved_ids[index] = f"{test_id}{test_id_suffixes[test_id]}"
-                test_id_suffixes[test_id] += 1
-
-    return resolved_ids
-
-
 def _pretty_fixture_path(func) -> str:
     cwd = Path.cwd()
     loc = Path(getlocation(func, str(cwd)))

testing/python/metafunc.py

@@ -24,8 +24,7 @@ from _pytest.compat import getfuncargnames
 from _pytest.compat import NOTSET
 from _pytest.outcomes import fail
 from _pytest.pytester import Pytester
-from _pytest.python import _idval
+from _pytest.python import IdMaker
-from _pytest.python import idmaker
 from _pytest.scope import Scope
 
 
@@ -286,7 +285,7 @@ class TestMetafunc:
         deadline=400.0
     )  # very close to std deadline and CI boxes are not reliable in CPU power
     def test_idval_hypothesis(self, value) -> None:
-        escaped = _idval(value, "a", 6, None, nodeid=None, config=None)
+        escaped = IdMaker([], [], None, None, None, None)._idval(value, "a", 6)
         assert isinstance(escaped, str)
         escaped.encode("ascii")
 
@@ -308,7 +307,9 @@ class TestMetafunc:
             ),
         ]
         for val, expected in values:
-            assert _idval(val, "a", 6, None, nodeid=None, config=None) == expected
+            assert (
+                IdMaker([], [], None, None, None, None)._idval(val, "a", 6) == expected
+            )
 
     def test_unicode_idval_with_config(self) -> None:
         """Unit test for expected behavior to obtain ids with
@@ -336,7 +337,7 @@ class TestMetafunc:
             ("ação", MockConfig({option: False}), "a\\xe7\\xe3o"),
         ]
         for val, config, expected in values:
-            actual = _idval(val, "a", 6, None, nodeid=None, config=config)
+            actual = IdMaker([], [], None, None, config, None)._idval(val, "a", 6)
             assert actual == expected
 
     def test_bytes_idval(self) -> None:
@@ -349,7 +350,9 @@ class TestMetafunc:
             ("αρά".encode(), r"\xce\xb1\xcf\x81\xce\xac"),
         ]
         for val, expected in values:
-            assert _idval(val, "a", 6, idfn=None, nodeid=None, config=None) == expected
+            assert (
+                IdMaker([], [], None, None, None, None)._idval(val, "a", 6) == expected
+            )
 
     def test_class_or_function_idval(self) -> None:
         """Unit test for the expected behavior to obtain ids for parametrized
@@ -363,7 +366,9 @@ class TestMetafunc:
 
         values = [(TestClass, "TestClass"), (test_function, "test_function")]
         for val, expected in values:
-            assert _idval(val, "a", 6, None, nodeid=None, config=None) == expected
+            assert (
+                IdMaker([], [], None, None, None, None)._idval(val, "a", 6) == expected
+            )
 
     def test_notset_idval(self) -> None:
         """Test that a NOTSET value (used by an empty parameterset) generates
@@ -371,29 +376,43 @@ class TestMetafunc:
 
         Regression test for #7686.
         """
-        assert _idval(NOTSET, "a", 0, None, nodeid=None, config=None) == "a0"
+        assert IdMaker([], [], None, None, None, None)._idval(NOTSET, "a", 0) == "a0"
 
     def test_idmaker_autoname(self) -> None:
         """#250"""
-        result = idmaker(
+        result = IdMaker(
-            ("a", "b"), [pytest.param("string", 1.0), pytest.param("st-ring", 2.0)]
+            ("a", "b"),
-        )
+            [pytest.param("string", 1.0), pytest.param("st-ring", 2.0)],
+            None,
+            None,
+            None,
+            None,
+        ).make_unique_parameterset_ids()
         assert result == ["string-1.0", "st-ring-2.0"]
 
-        result = idmaker(
+        result = IdMaker(
-            ("a", "b"), [pytest.param(object(), 1.0), pytest.param(object(), object())]
+            ("a", "b"),
-        )
+            [pytest.param(object(), 1.0), pytest.param(object(), object())],
+            None,
+            None,
+            None,
+            None,
+        ).make_unique_parameterset_ids()
         assert result == ["a0-1.0", "a1-b1"]
         # unicode mixing, issue250
-        result = idmaker(("a", "b"), [pytest.param({}, b"\xc3\xb4")])
+        result = IdMaker(
+            ("a", "b"), [pytest.param({}, b"\xc3\xb4")], None, None, None, None
+        ).make_unique_parameterset_ids()
         assert result == ["a0-\\xc3\\xb4"]
 
     def test_idmaker_with_bytes_regex(self) -> None:
-        result = idmaker(("a"), [pytest.param(re.compile(b"foo"), 1.0)])
+        result = IdMaker(
+            ("a"), [pytest.param(re.compile(b"foo"), 1.0)], None, None, None, None
+        ).make_unique_parameterset_ids()
         assert result == ["foo"]
 
     def test_idmaker_native_strings(self) -> None:
-        result = idmaker(
+        result = IdMaker(
             ("a", "b"),
             [
                 pytest.param(1.0, -1.1),
@@ -410,7 +429,11 @@ class TestMetafunc:
                 pytest.param(b"\xc3\xb4", "other"),
                 pytest.param(1.0j, -2.0j),
             ],
-        )
+            None,
+            None,
+            None,
+            None,
+        ).make_unique_parameterset_ids()
         assert result == [
             "1.0--1.1",
             "2--202",
@@ -428,7 +451,7 @@ class TestMetafunc:
         ]
 
     def test_idmaker_non_printable_characters(self) -> None:
-        result = idmaker(
+        result = IdMaker(
             ("s", "n"),
             [
                 pytest.param("\x00", 1),
@@ -438,23 +461,33 @@ class TestMetafunc:
                 pytest.param("\t", 5),
                 pytest.param(b"\t", 6),
             ],
-        )
+            None,
+            None,
+            None,
+            None,
+        ).make_unique_parameterset_ids()
         assert result == ["\\x00-1", "\\x05-2", "\\x00-3", "\\x05-4", "\\t-5", "\\t-6"]
 
     def test_idmaker_manual_ids_must_be_printable(self) -> None:
-        result = idmaker(
+        result = IdMaker(
             ("s",),
             [
                 pytest.param("x00", id="hello \x00"),
                 pytest.param("x05", id="hello \x05"),
             ],
-        )
+            None,
+            None,
+            None,
+            None,
+        ).make_unique_parameterset_ids()
         assert result == ["hello \\x00", "hello \\x05"]
 
     def test_idmaker_enum(self) -> None:
         enum = pytest.importorskip("enum")
         e = enum.Enum("Foo", "one, two")
-        result = idmaker(("a", "b"), [pytest.param(e.one, e.two)])
+        result = IdMaker(
+            ("a", "b"), [pytest.param(e.one, e.two)], None, None, None, None
+        ).make_unique_parameterset_ids()
         assert result == ["Foo.one-Foo.two"]
 
     def test_idmaker_idfn(self) -> None:
@@ -465,15 +498,18 @@ class TestMetafunc:
                 return repr(val)
             return None
 
-        result = idmaker(
+        result = IdMaker(
             ("a", "b"),
             [
                 pytest.param(10.0, IndexError()),
                 pytest.param(20, KeyError()),
                 pytest.param("three", [1, 2, 3]),
             ],
-            idfn=ids,
+            ids,
-        )
+            None,
+            None,
+            None,
+        ).make_unique_parameterset_ids()
         assert result == ["10.0-IndexError()", "20-KeyError()", "three-b2"]
 
     def test_idmaker_idfn_unique_names(self) -> None:
@@ -482,15 +518,18 @@ class TestMetafunc:
         def ids(val: object) -> str:
             return "a"
 
-        result = idmaker(
+        result = IdMaker(
             ("a", "b"),
             [
                 pytest.param(10.0, IndexError()),
                 pytest.param(20, KeyError()),
                 pytest.param("three", [1, 2, 3]),
             ],
-            idfn=ids,
+            ids,
-        )
+            None,
+            None,
+            None,
+        ).make_unique_parameterset_ids()
         assert result == ["a-a0", "a-a1", "a-a2"]
 
     def test_idmaker_with_idfn_and_config(self) -> None:
@@ -520,12 +559,9 @@ class TestMetafunc:
             (MockConfig({option: False}), "a\\xe7\\xe3o"),
         ]
         for config, expected in values:
-            result = idmaker(
+            result = IdMaker(
-                ("a",),
+                ("a",), [pytest.param("string")], lambda _: "ação", None, config, None
-                [pytest.param("string")],
+            ).make_unique_parameterset_ids()
-                idfn=lambda _: "ação",
-                config=config,
-            )
             assert result == [expected]
 
     def test_idmaker_with_ids_and_config(self) -> None:
@@ -555,12 +591,9 @@ class TestMetafunc:
             (MockConfig({option: False}), "a\\xe7\\xe3o"),
         ]
         for config, expected in values:
-            result = idmaker(
+            result = IdMaker(
-                ("a",),
+                ("a",), [pytest.param("string")], None, ["ação"], config, None
-                [pytest.param("string")],
+            ).make_unique_parameterset_ids()
-                ids=["ação"],
-                config=config,
-            )
             assert result == [expected]
 
     def test_parametrize_ids_exception(self, pytester: Pytester) -> None:
@@ -617,23 +650,36 @@ class TestMetafunc:
         )
 
     def test_idmaker_with_ids(self) -> None:
-        result = idmaker(
+        result = IdMaker(
-            ("a", "b"), [pytest.param(1, 2), pytest.param(3, 4)], ids=["a", None]
+            ("a", "b"),
-        )
+            [pytest.param(1, 2), pytest.param(3, 4)],
+            None,
+            ["a", None],
+            None,
+            None,
+        ).make_unique_parameterset_ids()
         assert result == ["a", "3-4"]
 
     def test_idmaker_with_paramset_id(self) -> None:
-        result = idmaker(
+        result = IdMaker(
             ("a", "b"),
             [pytest.param(1, 2, id="me"), pytest.param(3, 4, id="you")],
-            ids=["a", None],
+            None,
-        )
+            ["a", None],
+            None,
+            None,
+        ).make_unique_parameterset_ids()
         assert result == ["me", "you"]
 
     def test_idmaker_with_ids_unique_names(self) -> None:
-        result = idmaker(
+        result = IdMaker(
-            ("a"), map(pytest.param, [1, 2, 3, 4, 5]), ids=["a", "a", "b", "c", "b"]
+            ("a"),
-        )
+            list(map(pytest.param, [1, 2, 3, 4, 5])),
+            None,
+            ["a", "a", "b", "c", "b"],
+            None,
+            None,
+        ).make_unique_parameterset_ids()
         assert result == ["a0", "a1", "b0", "c", "b1"]
 
     def test_parametrize_indirect(self) -> None: