Skip to content

Conversation

@RonnyPfannschmidt
Copy link
Member

Fixes #2781

When --strict-markers is enabled, marker names used in -m expressions are now validated against registered markers.

Copilot AI review requested due to automatic review settings January 18, 2026 11:05
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Jan 18, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request implements validation of marker names in -m (marker expression) arguments when --strict-markers is enabled, addressing issue #2781. Previously, --strict-markers only validated markers used in test decorations (e.g., @pytest.mark.foo), but not markers referenced in command-line -m expressions.

Changes:

  • Added tracking of identifier names during expression parsing to capture marker names used in expressions
  • Implemented validation of marker names in -m expressions against registered markers when strict mode is enabled
  • Added comprehensive test coverage for the new validation behavior

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/_pytest/mark/expression.py Modified Scanner, expression parser, and Expression class to track and expose identifier names used in expressions
src/_pytest/mark/init.py Added _validate_marker_names() function to validate markers in expressions when strict_markers is enabled
testing/test_mark_expression.py Added unit tests for the new Expression.idents() method
testing/test_mark.py Added integration test verifying that unregistered markers in -m expressions trigger errors with --strict-markers
changelog/2781.feature.rst Documented the new feature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 232 to 237
pytester.makeini(
"""
[pytest]
markers = registered: a registered marker
"""
)
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The makeini call here is redundant because it's immediately overwritten by another makeini call on lines 239-245 when option_name is "strict_markers" or "strict". This first ini file is never used. Consider removing these lines or restructuring the test to avoid creating the ini file twice.

Copilot uses AI. Check for mistakes.
@RonnyPfannschmidt RonnyPfannschmidt force-pushed the fix-2781-strict-markers-m-expression branch from ec4ad16 to 20e1063 Compare January 18, 2026 11:23
Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I debated with myself whether this is a good feature. It can help with bad typos, but maybe there's a use case for trying non-registered markers? But I couldn't think of any such use cases. So LGTM. Let's see if anyone complains...

Consider also mentioning this briefly in the strict_markers documentation.

@@ -0,0 +1 @@
When ``--strict-markers`` is enabled, marker names used in ``-m`` expressions are now validated against registered markers.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
When ``--strict-markers`` is enabled, marker names used in ``-m`` expressions are now validated against registered markers.
When :confval:`strict_markers` is enabled, marker names used in :option:`-m` expressions are now validated against registered markers.


# Get registered markers from the ini configuration.
registered_markers: set[str] = set()
for line in config.getini("markers"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently have this in 2 places (in _pytest.mark.pytest_cmdline_main and in MarkGenerator. If we are to follow the "rule of three", maybe can add a method to Config like

def _iter_registered_markers(self) -> Iterator[tuple[str, str]]

which yields (name, description) (I don't remember if the description is optional, if it is, would be tuple[str, str | None]).

if not strict_markers:
return

# Get registered markers from the ini configuration.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I don't find this comment adds much, but OK if it helps.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the commments are sometihng i need to retrain myself to note and remove

marker = line.split(":")[0].split("(")[0].strip()
registered_markers.add(marker)

# Check each identifier in the expression.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I don't find this comment adds much, but OK if it helps.

return ret
ident = s.accept(TokenType.IDENT)
if ident:
# This is a marker/keyword name - track it.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I don't find this comment adds much, but OK if it helps.

#: The original input line, as a string.
self.input: Final = input
self._code: Final = code
#: The identifiers (marker/keyword names) used in the expression.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expression is written to be "independent" of its uses, i.e. it doesn't "know" about markers or keywords. I would therefore remove the parenthetical here.

Comment on lines 348 to 352
"""Return the set of identifier names used in the expression.
For marker expressions (-m), these are marker names.
For keyword expressions (-k), these are keyword names.
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, I suggest dropping the mention of marker/keyword.

Suggested change
"""Return the set of identifier names used in the expression.
For marker expressions (-m), these are marker names.
For keyword expressions (-k), these are keyword names.
"""
"""Return the set of all identifiers which appear in the expression."""

@RonnyPfannschmidt RonnyPfannschmidt force-pushed the fix-2781-strict-markers-m-expression branch from 20e1063 to 56688fb Compare January 19, 2026 10:17
Closes pytest-dev#2781

Co-authored-by: Cursor AI <ai@cursor.sh>
Co-authored-by: Anthropic Claude Opus 4.5 <claude@anthropic.com>
@RonnyPfannschmidt RonnyPfannschmidt force-pushed the fix-2781-strict-markers-m-expression branch from 56688fb to 2192ac2 Compare January 19, 2026 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided (automation) changelog entry is part of PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--strict does not complain when non-existent markers are specified as args to pytest

2 participants