diff --git a/changelog/6626.improvement.rst b/changelog/6626.improvement.rst new file mode 100644 index 00000000000..387e0ec9778 --- /dev/null +++ b/changelog/6626.improvement.rst @@ -0,0 +1,2 @@ +pytest now warns when custom parametrization IDs contain characters that prevent +selecting the generated test case directly with ``-k``. diff --git a/doc/en/example/parametrize.rst b/doc/en/example/parametrize.rst index 8e7e58f9829..d8baf4fd562 100644 --- a/doc/en/example/parametrize.rst +++ b/doc/en/example/parametrize.rst @@ -87,6 +87,12 @@ parametrized test. These IDs can be used with :option:`-k` to select specific ca to run, and they will also identify the specific case when one is failing. Running pytest with :option:`--collect-only` will show the generated IDs. +When providing custom test IDs, prefer IDs that can be used as part of a +:option:`-k` identifier. Characters with special meaning in ``-k`` expressions, +such as parentheses, commas, whitespace, and ``=``, can prevent the generated +test ID from being selected directly with ``-k``. pytest emits a warning for +custom IDs that contain such characters. + Numbers, strings, booleans and None will have their usual string representation used in the test ID. For other objects, pytest will make a string based on the argument name: diff --git a/src/_pytest/mark/expression.py b/src/_pytest/mark/expression.py index 65a77620226..0fc50f1a23e 100644 --- a/src/_pytest/mark/expression.py +++ b/src/_pytest/mark/expression.py @@ -158,6 +158,22 @@ def reject(self, expected: Sequence[TokenType]) -> NoReturn: ) +def is_safe_identifier_part(input: str) -> bool: + """Return whether input can safely be embedded in an identifier.""" + wrapped = f"x[{input}]" + try: + scanner = Scanner(wrapped) + except SyntaxError: + return False + + token = scanner.accept(TokenType.IDENT) + return ( + token is not None + and token.value == wrapped + and scanner.accept(TokenType.EOF) is not None + ) + + # True, False and None are legal match expression identifiers, # but illegal as Python identifiers. To fix this, this prefix # is added to identifiers in the conversion to Python AST. diff --git a/src/_pytest/python.py b/src/_pytest/python.py index be0ea5b4d05..3f5b1a9a9a3 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -65,6 +65,7 @@ from _pytest.fixtures import get_scope_node from _pytest.main import Session from _pytest.mark import ParameterSet +from _pytest.mark.expression import is_safe_identifier_part from _pytest.mark.structures import _HiddenParam from _pytest.mark.structures import get_unpacked_marks from _pytest.mark.structures import HIDDEN_PARAM @@ -82,6 +83,7 @@ from _pytest.stash import StashKey from _pytest.warning_types import PytestCollectionWarning from _pytest.warning_types import PytestReturnNotNoneWarning +from _pytest.warning_types import PytestWarning if TYPE_CHECKING: @@ -991,6 +993,16 @@ def _strict_parametrization_ids_enabled(self) -> bool: strict_parametrization_ids = self.config.getini("strict") return cast(bool, strict_parametrization_ids) + def _warn_if_id_sabotages_selection(self, id: str) -> None: + if not is_safe_identifier_part(id): + warnings.warn( + PytestWarning( + f"{self._make_error_prefix()}parametrization ID {id!r} " + "contains characters that prevent selecting the generated " + "test case directly with '-k'" + ) + ) + def _resolve_ids(self) -> Iterable[str | _HiddenParam]: """Resolve IDs for all ParameterSets (may contain duplicates).""" for idx, parameterset in enumerate(self.parametersets): @@ -999,13 +1011,17 @@ def _resolve_ids(self) -> Iterable[str | _HiddenParam]: if parameterset.id is HIDDEN_PARAM: yield HIDDEN_PARAM else: - yield _ascii_escaped_by_config(parameterset.id, self.config) + id = _ascii_escaped_by_config(parameterset.id, self.config) + self._warn_if_id_sabotages_selection(id) + yield id elif self.ids and idx < len(self.ids) and self.ids[idx] is not None: # ID provided in the IDs list - parametrize(..., ids=[...]). if self.ids[idx] is HIDDEN_PARAM: yield HIDDEN_PARAM else: - yield self._idval_from_value_required(self.ids[idx], idx) + id = self._idval_from_value_required(self.ids[idx], idx) + self._warn_if_id_sabotages_selection(id) + yield id else: # ID not provided - generate it. yield "-".join( @@ -1086,7 +1102,11 @@ def _idval_from_function(self, val: object, argname: str, idx: int) -> str | Non raise ValueError(msg) from e if id is None: return None - return self._idval_from_value(id) + + resolved_id = self._idval_from_value(id) + if resolved_id is not None: + self._warn_if_id_sabotages_selection(resolved_id) + return resolved_id def _idval_from_hook(self, val: object, argname: str) -> str | None: """Try to make an ID for a parameter in a ParameterSet by calling the @@ -1095,6 +1115,8 @@ def _idval_from_hook(self, val: object, argname: str) -> str | None: id: str | None = self.config.hook.pytest_make_parametrize_id( config=self.config, val=val, argname=argname ) + if id is not None: + self._warn_if_id_sabotages_selection(id) return id return None diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 566071d47a1..10662115189 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -11,6 +11,7 @@ from typing import Any from typing import cast from typing import ClassVar +import warnings import hypothesis from hypothesis import strategies @@ -25,6 +26,7 @@ from _pytest.python import Function from _pytest.python import IdMaker from _pytest.scope import Scope +from _pytest.warning_types import PytestWarning import pytest @@ -450,6 +452,21 @@ def test_notset_idval(self) -> None: """ assert IdMaker([], [], None, None, None, None)._idval(NOTSET, "a", 0) == "a0" + def test_idmaker_does_not_warn_for_unselectable_builtin_id(self) -> None: + with warnings.catch_warnings(): + warnings.simplefilter("error", PytestWarning) + + result = IdMaker( + ("arg",), + [pytest.param(complex(-0.0, -2))], + None, + None, + None, + None, + ).make_unique_parameterset_ids() + + assert result == ["(-0-2j)"] + def test_idmaker_autoname(self) -> None: """#250""" result = IdMaker( @@ -541,17 +558,20 @@ def test_idmaker_non_printable_characters(self) -> None: 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( - ("s",), - [ - pytest.param("x00", id="hello \x00"), - pytest.param("x05", id="hello \x05"), - ], - None, - None, - None, - None, - ).make_unique_parameterset_ids() + with pytest.warns(PytestWarning) as warnings_record: + result = IdMaker( + ("s",), + [ + pytest.param("x00", id="hello \x00"), + pytest.param("x05", id="hello \x05"), + ], + None, + None, + None, + None, + ).make_unique_parameterset_ids() + + assert len(warnings_record) == 2 assert result == ["hello \\x00", "hello \\x05"] def test_idmaker_enum(self) -> None: @@ -570,18 +590,21 @@ def ids(val: object) -> str | None: return repr(val) return None - result = IdMaker( - ("a", "b"), - [ - pytest.param(10.0, IndexError()), - pytest.param(20, KeyError()), - pytest.param("three", [1, 2, 3]), - ], - ids, - None, - None, - None, - ).make_unique_parameterset_ids() + with pytest.warns(PytestWarning) as warnings_record: + result = IdMaker( + ("a", "b"), + [ + pytest.param(10.0, IndexError()), + pytest.param(20, KeyError()), + pytest.param("three", [1, 2, 3]), + ], + ids, + None, + None, + None, + ).make_unique_parameterset_ids() + + assert len(warnings_record) == 2 assert result == ["10.0-IndexError()", "20-KeyError()", "three-b2"] def test_idmaker_idfn_unique_names(self) -> None: @@ -604,6 +627,74 @@ def ids(val: object) -> str: ).make_unique_parameterset_ids() assert result == ["a-a0", "a-a1", "a-a2"] + def test_idmaker_warns_for_unselectable_callable_id(self) -> None: + with pytest.warns( + PytestWarning, + match=r"parametrization ID '\(1, 1\)' contains characters that prevent selecting", + ): + result = IdMaker( + ("arg",), + [pytest.param((1, 1))], + repr, + None, + None, + None, + ).make_unique_parameterset_ids() + + assert result == ["(1, 1)"] + + def test_idmaker_does_not_warn_for_selectable_custom_ids(self) -> None: + with warnings.catch_warnings(): + warnings.simplefilter("error", PytestWarning) + + result = IdMaker( + ("arg",), + [ + pytest.param(1, id="and"), + pytest.param(2, id="or"), + pytest.param(3, id="not"), + pytest.param(4, id=""), + ], + None, + None, + None, + None, + ).make_unique_parameterset_ids() + + assert result == ["and", "or", "not", ""] + + def test_idmaker_warns_for_unselectable_explicit_param_id(self) -> None: + with pytest.warns( + PytestWarning, + match=r"parametrization ID 'foo\(bar\)' contains characters that prevent selecting", + ): + result = IdMaker( + ("arg",), + [pytest.param(1, id="foo(bar)")], + None, + None, + None, + None, + ).make_unique_parameterset_ids() + + assert result == ["foo(bar)"] + + def test_idmaker_warns_for_unselectable_ids_list(self) -> None: + with pytest.warns( + PytestWarning, + match=r"parametrization ID 'foo,bar' contains characters that prevent selecting", + ): + result = IdMaker( + ("arg",), + [pytest.param(1)], + None, + ["foo,bar"], + None, + None, + ).make_unique_parameterset_ids() + + assert result == ["foo,bar"] + def test_idmaker_with_idfn_and_config(self) -> None: """Unit test for expected behavior to create ids with idfn and disable_test_id_escaping_and_forfeit_all_rights_to_community_support @@ -2318,6 +2409,34 @@ def test_func(x): result = pytester.runpytest("-v") result.stdout.fnmatch_lines(["*test_func*0*PASS*", "*test_func*2*PASS*"]) + def test_pytest_make_parametrize_id_warns_for_unselectable_id( + self, pytester: Pytester + ) -> None: + pytester.makeconftest( + """ + def pytest_make_parametrize_id(config, val): + return "foo(bar)" + """ + ) + pytester.makepyfile( + """ + import pytest + + @pytest.mark.parametrize("x", [1]) + def test_func(x): + pass + """ + ) + + result = pytester.runpytest("-W", "default") + + result.stdout.fnmatch_lines( + [ + "*PytestWarning: *parametrization ID 'foo(bar)' contains characters that prevent selecting*" + ] + ) + result.assert_outcomes(passed=1, warnings=1) + def test_pytest_make_parametrize_id_with_argname(self, pytester: Pytester) -> None: pytester.makeconftest( """ diff --git a/testing/test_mark_expression.py b/testing/test_mark_expression.py index ac9c882e61e..715ead3f925 100644 --- a/testing/test_mark_expression.py +++ b/testing/test_mark_expression.py @@ -3,6 +3,7 @@ from _pytest.mark import MarkMatcher from _pytest.mark.expression import Expression from _pytest.mark.expression import ExpressionMatcher +from _pytest.mark.expression import is_safe_identifier_part import pytest @@ -193,6 +194,31 @@ def matcher(name: str, /, **kwargs: str | int | bool | None) -> bool: assert evaluate(ident, matcher) +@pytest.mark.parametrize( + ("ident", "expected"), + ( + ("foo", True), + ("foo-bar", True), + ("foo[bar]", True), + ("foo/bar", True), + ("foo+bar", True), + ("foo:bar", True), + ("foo.bar", True), + ("foo(bar)", False), + ("(foo)", False), + ("foo,bar", False), + ("foo bar", False), + ("foo=bar", False), + ("and", True), + ("or", True), + ("not", True), + ("", True), + ), +) +def test_is_safe_identifier_part(ident: str, expected: bool) -> None: + assert is_safe_identifier_part(ident) is expected + + @pytest.mark.parametrize( "ident", (