From 2192ac23b0baede62777cfab4c548d5d5b780eba Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Sun, 18 Jan 2026 12:03:24 +0100 Subject: [PATCH 1/2] fix: validate marker names in -m expression with --strict-markers Closes #2781 Co-authored-by: Cursor AI Co-authored-by: Anthropic Claude Opus 4.5 --- changelog/2781.feature.rst | 1 + src/_pytest/mark/__init__.py | 32 ++++++++++++++++++ src/_pytest/mark/expression.py | 24 +++++++++---- testing/test_mark.py | 60 +++++++++++++++++++++++++++++++++ testing/test_mark_expression.py | 18 ++++++++++ 5 files changed, 128 insertions(+), 7 deletions(-) create mode 100644 changelog/2781.feature.rst diff --git a/changelog/2781.feature.rst b/changelog/2781.feature.rst new file mode 100644 index 00000000000..d3a30efc20d --- /dev/null +++ b/changelog/2781.feature.rst @@ -0,0 +1 @@ +When :confval:`strict_markers` is enabled, marker names used in :option:`-m` expressions are now validated against registered markers. diff --git a/src/_pytest/mark/__init__.py b/src/_pytest/mark/__init__.py index 56c407ab371..400c32f79c7 100644 --- a/src/_pytest/mark/__init__.py +++ b/src/_pytest/mark/__init__.py @@ -258,6 +258,10 @@ def deselect_by_mark(items: list[Item], config: Config) -> None: return expr = _parse_expression(matchexpr, "Wrong expression passed to '-m'") + + # Validate marker names in the expression if strict_markers is enabled. + _validate_marker_names(expr, config) + remaining: list[Item] = [] deselected: list[Item] = [] for item in items: @@ -270,6 +274,34 @@ def deselect_by_mark(items: list[Item], config: Config) -> None: items[:] = remaining +def _validate_marker_names(expr: Expression, config: Config) -> None: + """Validate that all marker names in the expression are registered. + + Only validates when strict_markers is enabled. + """ + strict_markers = config.getini("strict_markers") + if strict_markers is None: + strict_markers = config.getini("strict") + if not strict_markers: + return + + registered_markers: set[str] = set() + for line in config.getini("markers"): + # example lines: "skipif(condition): skip the given test if..." + # or "hypothesis: tests which use Hypothesis", so to get the + # marker name we split on both `:` and `(`. + marker = line.split(":")[0].split("(")[0].strip() + registered_markers.add(marker) + + unknown_markers = expr.idents() - registered_markers + if unknown_markers: + unknown_str = ", ".join(sorted(unknown_markers)) + raise UsageError( + f"Unknown marker(s) in '-m' expression: {unknown_str}. " + "Use 'pytest --markers' to see available markers." + ) + + def _parse_expression(expr: str, exc_message: str) -> Expression: try: return Expression.compile(expr) diff --git a/src/_pytest/mark/expression.py b/src/_pytest/mark/expression.py index 3bdbd03c2b5..16262e0eeff 100644 --- a/src/_pytest/mark/expression.py +++ b/src/_pytest/mark/expression.py @@ -70,10 +70,11 @@ class Token: class Scanner: - __slots__ = ("current", "input", "tokens") + __slots__ = ("current", "idents", "input", "tokens") def __init__(self, input: str) -> None: self.input = input + self.idents: set[str] = set() self.tokens = self.lex(input) self.current = next(self.tokens) @@ -163,13 +164,13 @@ def reject(self, expected: Sequence[TokenType]) -> NoReturn: IDENT_PREFIX = "$" -def expression(s: Scanner) -> ast.Expression: +def expression(s: Scanner) -> tuple[ast.Expression, frozenset[str]]: if s.accept(TokenType.EOF): ret: ast.expr = ast.Constant(False) else: ret = expr(s) s.accept(TokenType.EOF, reject=True) - return ast.fix_missing_locations(ast.Expression(ret)) + return ast.fix_missing_locations(ast.Expression(ret)), frozenset(s.idents) def expr(s: Scanner) -> ast.expr: @@ -197,6 +198,7 @@ def not_expr(s: Scanner) -> ast.expr: return ret ident = s.accept(TokenType.IDENT) if ident: + s.idents.add(ident.value) name = ast.Name(IDENT_PREFIX + ident.value, ast.Load()) if s.accept(TokenType.LPAREN): ret = ast.Call(func=name, args=[], keywords=all_kwargs(s)) @@ -314,12 +316,16 @@ class Expression: The expression can be evaluated against different matchers. """ - __slots__ = ("_code", "input") + __slots__ = ("_code", "_idents", "input") - def __init__(self, input: str, code: types.CodeType) -> None: + def __init__( + self, input: str, code: types.CodeType, idents: frozenset[str] + ) -> None: #: The original input line, as a string. self.input: Final = input self._code: Final = code + #: All identifiers which appear in the expression. + self._idents: Final = idents @classmethod def compile(cls, input: str) -> Expression: @@ -329,13 +335,17 @@ def compile(cls, input: str) -> Expression: :raises SyntaxError: If the expression is malformed. """ - astexpr = expression(Scanner(input)) + astexpr, idents = expression(Scanner(input)) code = compile( astexpr, filename="", mode="eval", ) - return Expression(input, code) + return Expression(input, code, idents) + + def idents(self) -> frozenset[str]: + """Return the set of all identifiers which appear in the expression.""" + return self._idents def evaluate(self, matcher: ExpressionMatcher) -> bool: """Evaluate the match expression. diff --git a/testing/test_mark.py b/testing/test_mark.py index 67219313183..2b2bc43a4ff 100644 --- a/testing/test_mark.py +++ b/testing/test_mark.py @@ -3,10 +3,14 @@ import os import sys +from typing import cast from unittest import mock from _pytest.config import ExitCode +from _pytest.config import UsageError +from _pytest.mark import _validate_marker_names from _pytest.mark import MarkGenerator +from _pytest.mark.expression import Expression from _pytest.mark.structures import EMPTY_PARAMETERSET_OPTION from _pytest.nodes import Collector from _pytest.nodes import Node @@ -213,6 +217,62 @@ def test_hello(): ) +class TestValidateMarkerNames: + """Tests for _validate_marker_names (issue #2781).""" + + class FakeConfig: + def __init__( + self, + markers: list[str], + strict_markers: bool | None = None, + strict: bool = False, + ) -> None: + self._ini: dict[str, list[str] | bool | None] = { + "markers": markers, + "strict_markers": strict_markers, + "strict": strict, + } + + def getini(self, name: str) -> list[str] | bool | None: + return self._ini[name] + + def _make_config( + self, + strict_markers: bool | None = None, + strict: bool = False, + ) -> pytest.Config: + return cast( + pytest.Config, + self.FakeConfig( + markers=["registered: a registered marker"], + strict_markers=strict_markers, + strict=strict, + ), + ) + + def test_unknown_marker_with_strict_markers(self) -> None: + expr = Expression.compile("unknown_marker") + + with pytest.raises(UsageError, match=r"Unknown marker.*unknown_marker"): + _validate_marker_names(expr, self._make_config(strict_markers=True)) + + def test_unknown_marker_with_strict(self) -> None: + expr = Expression.compile("unknown_marker") + + with pytest.raises(UsageError, match=r"Unknown marker.*unknown_marker"): + _validate_marker_names(expr, self._make_config(strict=True)) + + def test_registered_marker_passes(self) -> None: + expr = Expression.compile("registered") + + _validate_marker_names(expr, self._make_config(strict_markers=True)) + + def test_no_validation_without_strict(self) -> None: + expr = Expression.compile("any_marker") + + _validate_marker_names(expr, self._make_config()) + + @pytest.mark.parametrize( ("expr", "expected_passed"), [ diff --git a/testing/test_mark_expression.py b/testing/test_mark_expression.py index 1e3c769347c..0f8ea3c7e0a 100644 --- a/testing/test_mark_expression.py +++ b/testing/test_mark_expression.py @@ -322,3 +322,21 @@ def test_str_keyword_expressions( expr: str, expected: bool, mark_matcher: MarkMatcher ) -> None: assert evaluate(expr, mark_matcher) is expected + + +@pytest.mark.parametrize( + ("expr", "expected_idents"), + ( + ("", frozenset()), + ("foo", frozenset(["foo"])), + ("foo and bar", frozenset(["foo", "bar"])), + ("foo or bar", frozenset(["foo", "bar"])), + ("not foo", frozenset(["foo"])), + ("(foo and bar) or baz", frozenset(["foo", "bar", "baz"])), + ("foo and foo", frozenset(["foo"])), # Duplicates are deduplicated. + ("mark(a=1)", frozenset(["mark"])), # Only marker name, not kwargs. + ), +) +def test_expression_idents(expr: str, expected_idents: frozenset[str]) -> None: + """Test that Expression.idents() returns the identifiers in the expression.""" + assert Expression.compile(expr).idents() == expected_idents From fbfc464b0a542295b65e80e83be9602078f9457a Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Wed, 21 Jan 2026 17:24:51 +0100 Subject: [PATCH 2/2] refactor: centralize marker registration parsing in Config Extract marker line parsing into a single _iter_registered_markers() method on Config class, replacing duplicate parsing logic in three locations: - MarkGenerator._markers cache population - _validate_marker_names() for strict marker checking - pytest_cmdline_main() for --markers display Introduces RegisteredMarker NamedTuple with name, signature, and description fields for cleaner API. Co-authored-by: Cursor AI Co-authored-by: Anthropic Claude Opus 4.5 --- src/_pytest/config/__init__.py | 30 ++++++++++++++++++++++++++++++ src/_pytest/mark/__init__.py | 18 ++++-------------- src/_pytest/mark/structures.py | 9 +++------ 3 files changed, 37 insertions(+), 20 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 21dc35219d8..dde68a5ac6e 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -35,6 +35,7 @@ from typing import Final from typing import final from typing import IO +from typing import NamedTuple from typing import TextIO from typing import TYPE_CHECKING import warnings @@ -1012,6 +1013,19 @@ def __len__(self) -> int: return len(self._config._inicfg) +class RegisteredMarker(NamedTuple): + """A marker registered in the configuration. + + :param name: The marker name (e.g., ``skipif``). + :param signature: The full marker signature (e.g., ``skipif(condition)``). + :param description: The marker description. + """ + + name: str + signature: str + description: str + + @final class Config: """Access to configuration values, pluginmanager and plugin hooks. @@ -1671,6 +1685,22 @@ def getini(self, name: str) -> Any: self._inicache[canonical_name] = val = self._getini(canonical_name) return val + def _iter_registered_markers(self) -> Iterator[RegisteredMarker]: + """Iterate over all markers registered in the configuration. + + Yields :class:`RegisteredMarker` named tuples with ``name``, + ``signature``, and ``description`` fields. + """ + for line in self.getini("markers"): + # Example lines: "skipif(condition): skip the given test if..." + # or "hypothesis: tests which use Hypothesis", so to get the + # marker name we split on both `:` and `(`. + parts = line.split(":", 1) + signature = parts[0] + description = parts[1].strip() if len(parts) == 2 else "" + name = signature.split("(")[0].strip() + yield RegisteredMarker(name, signature, description) + # Meant for easy monkeypatching by legacypath plugin. # Can be inlined back (with no cover removed) once legacypath is gone. def _getini_unknown_type(self, name: str, type: str, value: object): diff --git a/src/_pytest/mark/__init__.py b/src/_pytest/mark/__init__.py index 400c32f79c7..b29b6adb37f 100644 --- a/src/_pytest/mark/__init__.py +++ b/src/_pytest/mark/__init__.py @@ -134,12 +134,9 @@ def pytest_cmdline_main(config: Config) -> int | ExitCode | None: if config.option.markers: config._do_configure() tw = _pytest.config.create_terminal_writer(config) - for line in config.getini("markers"): - parts = line.split(":", 1) - name = parts[0] - rest = parts[1] if len(parts) == 2 else "" - tw.write(f"@pytest.mark.{name}:", bold=True) - tw.line(rest) + for marker in config._iter_registered_markers(): + tw.write(f"@pytest.mark.{marker.signature}:", bold=True) + tw.line(f" {marker.description}" if marker.description else "") tw.line() config._ensure_unconfigure() return 0 @@ -285,14 +282,7 @@ def _validate_marker_names(expr: Expression, config: Config) -> None: if not strict_markers: return - registered_markers: set[str] = set() - for line in config.getini("markers"): - # example lines: "skipif(condition): skip the given test if..." - # or "hypothesis: tests which use Hypothesis", so to get the - # marker name we split on both `:` and `(`. - marker = line.split(":")[0].split("(")[0].strip() - registered_markers.add(marker) - + registered_markers = {marker.name for marker in config._iter_registered_markers()} unknown_markers = expr.idents() - registered_markers if unknown_markers: unknown_str = ", ".join(sorted(unknown_markers)) diff --git a/src/_pytest/mark/structures.py b/src/_pytest/mark/structures.py index 9743f673adc..598f128ca7c 100644 --- a/src/_pytest/mark/structures.py +++ b/src/_pytest/mark/structures.py @@ -601,12 +601,9 @@ def __getattr__(self, name: str) -> MarkDecorator: # name is in the set we definitely know it, but a mark may be known and # not in the set. We therefore start by updating the set! if name not in self._markers: - for line in self._config.getini("markers"): - # example lines: "skipif(condition): skip the given test if..." - # or "hypothesis: tests which use Hypothesis", so to get the - # marker name we split on both `:` and `(`. - marker = line.split(":")[0].split("(")[0].strip() - self._markers.add(marker) + self._markers.update( + m.name for m in self._config._iter_registered_markers() + ) # If the name is not in the set of known marks after updating, # then it really is time to issue a warning or an error.