From 3c2263613165828421aa482b9d7939bbe3cfa0b9 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Mon, 17 Nov 2025 22:35:50 +0200 Subject: [PATCH 1/3] python: use nodeid in error messages instead of function name The function name is ambiguous, full nodeid is more helpful. --- src/_pytest/python.py | 31 +++++++++------------ testing/python/metafunc.py | 56 +++++++++++++++----------------------- 2 files changed, 35 insertions(+), 52 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 257dd78f462..e9d7fab0413 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -870,7 +870,6 @@ class IdMaker: __slots__ = ( "argnames", "config", - "func_name", "idfn", "ids", "nodeid", @@ -893,9 +892,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 +1079,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 "" @@ -1435,7 +1429,7 @@ 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, @@ -1443,7 +1437,6 @@ def _resolve_parameter_set_ids( ids_, self.config, nodeid=nodeid, - func_name=self.function.__name__, ) return id_maker.make_unique_parameterset_ids() @@ -1451,7 +1444,6 @@ 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,8 +1456,11 @@ 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) + nodeid = self.definition.nodeid + fail( + f"In {nodeid}: {len(parametersets)} parameter sets specified, with different number of ids: {num_ids}", + pytrace=False, + ) return list(itertools.islice(ids, num_ids)) @@ -1486,6 +1481,7 @@ def _resolve_args_directness( :returns A dict mapping each arg name to either "indirect" or "direct". """ + nodeid = self.definition.nodeid arg_directness: dict[str, Literal["indirect", "direct"]] if isinstance(indirect, bool): arg_directness = dict.fromkeys( @@ -1496,14 +1492,13 @@ def _resolve_args_directness( for arg in indirect: if arg not in argnames: fail( - f"In {self.function.__name__}: indirect fixture '{arg}' doesn't exist", + f"In {nodeid}: indirect fixture '{arg}' doesn't exist", pytrace=False, ) arg_directness[arg] = "indirect" else: fail( - f"In {self.function.__name__}: expected Sequence or boolean" - f" for indirect, got {type(indirect).__name__}", + f"In {nodeid}: expected Sequence or boolean for indirect, got {type(indirect).__name__}", pytrace=False, ) return arg_directness @@ -1520,12 +1515,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 +1529,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, ) diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 82a5509c49a..7217c80c03d 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -119,7 +119,7 @@ def gen() -> Iterator[int | None | Exc]: with pytest.raises( fail.Exception, match=( - r"In func: ids contains unsupported value Exc\(from_gen\) \(type: \) at index 2. " + r"In mock::nodeid: ids contains unsupported value Exc\(from_gen\) \(type: \) at index 2. " r"Supported types are: .*" ), ): @@ -300,7 +300,7 @@ class A: 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 = IdMaker([], [], None, None, None, None, None)._idval(value, "a", 6) + escaped = IdMaker([], [], None, None, None, None)._idval(value, "a", 6) assert isinstance(escaped, str) escaped.encode("ascii") @@ -323,8 +323,7 @@ def test_unicode_idval(self) -> None: ] for val, expected in values: assert ( - IdMaker([], [], None, None, None, None, None)._idval(val, "a", 6) - == expected + IdMaker([], [], None, None, None, None)._idval(val, "a", 6) == expected ) def test_unicode_idval_with_config(self) -> None: @@ -353,7 +352,7 @@ def getini(self, name): ("ação", MockConfig({option: False}), "a\\xe7\\xe3o"), ] for val, config, expected in values: - actual = IdMaker([], [], None, None, config, None, None)._idval(val, "a", 6) + actual = IdMaker([], [], None, None, config, None)._idval(val, "a", 6) assert actual == expected def test_bytes_idval(self) -> None: @@ -367,8 +366,7 @@ def test_bytes_idval(self) -> None: ] for val, expected in values: assert ( - IdMaker([], [], None, None, None, None, None)._idval(val, "a", 6) - == expected + IdMaker([], [], None, None, None, None)._idval(val, "a", 6) == expected ) def test_class_or_function_idval(self) -> None: @@ -384,8 +382,7 @@ def test_function(): values = [(TestClass, "TestClass"), (test_function, "test_function")] for val, expected in values: assert ( - IdMaker([], [], None, None, None, None, None)._idval(val, "a", 6) - == expected + IdMaker([], [], None, None, None, None)._idval(val, "a", 6) == expected ) def test_notset_idval(self) -> None: @@ -394,9 +391,7 @@ def test_notset_idval(self) -> None: Regression test for #7686. """ - assert ( - IdMaker([], [], None, None, None, None, None)._idval(NOTSET, "a", 0) == "a0" - ) + assert IdMaker([], [], None, None, None, None)._idval(NOTSET, "a", 0) == "a0" def test_idmaker_autoname(self) -> None: """#250""" @@ -407,7 +402,6 @@ def test_idmaker_autoname(self) -> None: None, None, None, - None, ).make_unique_parameterset_ids() assert result == ["string-1.0", "st-ring-2.0"] @@ -418,18 +412,17 @@ def test_idmaker_autoname(self) -> None: 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")], None, None, None, None, None + ("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"))], None, None, None, None, None + ("a"), [pytest.param(re.compile(b"foo"))], None, None, None, None ).make_unique_parameterset_ids() assert result == ["foo"] @@ -455,7 +448,6 @@ def test_idmaker_native_strings(self) -> None: None, None, None, - None, ).make_unique_parameterset_ids() assert result == [ "1.0--1.1", @@ -488,7 +480,6 @@ def test_idmaker_non_printable_characters(self) -> None: None, None, None, - None, ).make_unique_parameterset_ids() assert result == ["\\x00-1", "\\x05-2", "\\x00-3", "\\x05-4", "\\t-5", "\\t-6"] @@ -503,7 +494,6 @@ def test_idmaker_manual_ids_must_be_printable(self) -> None: None, None, None, - None, ).make_unique_parameterset_ids() assert result == ["hello \\x00", "hello \\x05"] @@ -511,7 +501,7 @@ 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)], None, None, None, None, None + ("a", "b"), [pytest.param(e.one, e.two)], None, None, None, None ).make_unique_parameterset_ids() assert result == ["Foo.one-Foo.two"] @@ -534,7 +524,6 @@ def ids(val: object) -> str | None: None, None, None, - None, ).make_unique_parameterset_ids() assert result == ["10.0-IndexError()", "20-KeyError()", "three-b2"] @@ -555,7 +544,6 @@ def ids(val: object) -> str: None, None, None, - None, ).make_unique_parameterset_ids() assert result == ["a-a0", "a-a1", "a-a2"] @@ -593,7 +581,6 @@ def getini(self, name): None, config, None, - None, ).make_unique_parameterset_ids() assert result == [expected] @@ -625,7 +612,7 @@ def getini(self, name): ] for config, expected in values: result = IdMaker( - ("a",), [pytest.param("string")], None, ["ação"], config, None, None + ("a",), [pytest.param("string")], None, ["ação"], config, None ).make_unique_parameterset_ids() assert result == [expected] @@ -656,14 +643,13 @@ def getini(self, name): None, config, None, - None, ).make_unique_parameterset_ids() assert result == [expected] def test_idmaker_duplicated_empty_str(self) -> None: """Regression test for empty strings parametrized more than once (#11563).""" result = IdMaker( - ("a",), [pytest.param(""), pytest.param("")], None, None, None, None, None + ("a",), [pytest.param(""), pytest.param("")], None, None, None, None ).make_unique_parameterset_ids() assert result == ["0", "1"] @@ -728,7 +714,6 @@ def test_idmaker_with_ids(self) -> None: ["a", None], None, None, - None, ).make_unique_parameterset_ids() assert result == ["a", "3-4"] @@ -740,7 +725,6 @@ def test_idmaker_with_paramset_id(self) -> None: ["a", None], None, None, - None, ).make_unique_parameterset_ids() assert result == ["me", "you"] @@ -752,7 +736,6 @@ def test_idmaker_with_ids_unique_names(self) -> None: ["a", "a", "b", "c", "b"], None, None, - None, ).make_unique_parameterset_ids() assert result == ["a0", "a1", "b0", "c", "b1"] @@ -811,7 +794,7 @@ def func(x, y): metafunc = self.Metafunc(func) with pytest.raises( fail.Exception, - match="In func: expected Sequence or boolean for indirect, got dict", + match="In mock::nodeid: expected Sequence or boolean for indirect, got dict", ): metafunc.parametrize("x, y", [("a", "b")], indirect={}) # type: ignore[arg-type] @@ -1431,7 +1414,8 @@ def test_ids_numbers(x,expected): result = pytester.runpytest() result.stdout.fnmatch_lines( [ - "In test_ids_numbers: ids contains unsupported value OSError() (type: ) at index 2. " + "In test_parametrized_ids_invalid_type.py::test_ids_numbers: ids contains unsupported value " + "OSError() (type: ) at index 2. " "Supported types are: str, bytes, int, float, complex, bool, enum, regex or anything with a __name__." ] ) @@ -2303,8 +2287,8 @@ def test_func(foo, bar): "", "*= ERRORS =*", "*_ ERROR collecting test_multiple_hidden_param_is_forbidden.py _*", - "E Failed: In test_func: multiple instances of HIDDEN_PARAM cannot be used " - "in the same parametrize call, because the tests names need to be unique.", + "E Failed: In test_multiple_hidden_param_is_forbidden.py::test_func: multiple instances of " + "HIDDEN_PARAM cannot be used in the same parametrize call, because the tests names need to be unique.", "*! Interrupted: 1 error during collection !*", "*= no tests collected, 1 error in *", ] @@ -2318,12 +2302,16 @@ def test_multiple_hidden_param_is_forbidden_idmaker(self) -> None: [pytest.HIDDEN_PARAM, pytest.HIDDEN_PARAM], None, "some_node_id", - None, ) expected = "In some_node_id: multiple instances of HIDDEN_PARAM" with pytest.raises(Failed, match=expected): id_maker.make_unique_parameterset_ids() + def test_idmaker_error_without_nodeid(self) -> None: + id_maker = IdMaker(["a"], [pytest.param("a")], None, [object()], None, None) + with pytest.raises(Failed, match="ids contains unsupported value"): + id_maker.make_unique_parameterset_ids() + def test_multiple_parametrize(self, pytester: Pytester) -> None: items = pytester.getitems( """ From 3aabdca3f301dca9c3574bb4e15226a9387dee61 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Mon, 17 Nov 2025 23:18:28 +0200 Subject: [PATCH 2/3] python: move `_resolve_args_directness` function Needed for the next commit. --- src/_pytest/fixtures.py | 40 +++++++++++++++++++++++++++++++++++++ src/_pytest/python.py | 44 ++++------------------------------------- 2 files changed, 44 insertions(+), 40 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 701be0283a1..9f3bb877a5d 100644 --- a/src/_pytest/fixtures.py +++ b/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. diff --git a/src/_pytest/python.py b/src/_pytest/python.py index e9d7fab0413..7374fa3cee0 100644 --- a/src/_pytest/python.py +++ b/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 @@ -1327,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 @@ -1464,45 +1467,6 @@ def _validate_ids( 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". - """ - nodeid = self.definition.nodeid - 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 _validate_if_using_arg_names( self, argnames: Sequence[str], From adcdeb9236d40a0b3a7f25e03ae94e683501cf56 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Mon, 17 Nov 2025 23:19:13 +0200 Subject: [PATCH 3/3] fixtures: fix incorrect "duplicate parametrization" when using `indirect=[...]` The code in `_get_direct_parametrize_args` only handled `indirect=True/False`, it didn't account for `indirect=[...]`. This caused the added test to start failing after d56b1af5252ceb8578d6751a6b2006e79defbb08. Fix by using the common `_resolve_args_directness` function. It's a bit heavy for the task but correctness first... Fix #13974. --- src/_pytest/fixtures.py | 15 ++++++++++----- testing/python/collect.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 9f3bb877a5d..5fc2abc3991 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1523,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 diff --git a/testing/python/collect.py b/testing/python/collect.py index b26931007d9..d1901684527 100644 --- a/testing/python/collect.py +++ b/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: