[LuanaPBueno]

This adds a warning when the reason passed to pytest.mark.skip looks like a condition, like pytest.mark.skip("sys.platform == 'win32'"). That test is always skipped, and it's easy to mix up with skipif without noticing.

While testing this, the rule suggested in the issue turned out to also catch normal reasons like "nothing in particular" or "Test is not isolated" (since in and is not are valid comparisons in Python), and two existing tests broke because of it. So I made it a bit stricter: it only warns when there's an attribute access, a call, or a comparison with a value, like os.name == 'nt'.

I used Claude to help, but I reviewed all the changes, ran the tests locally and checked that the new tests fail without the fix. Happy to change anything!

Closes #12582

• Include new tests or update existing tests when applicable.
• Allow maintainers to push and squash when merging my commits.
• Add text like closes #XYZW to the PR description and/or commits.
• If AI agents were used, they are credited in Co-authored-by commit trailers.
• Create a new changelog file in the changelog directory.
• Add yourself to AUTHORS in alphabetical order (I did this in pytest-dev/pytest#15202 (already merged), so my name is already in the list).

[Zac-HD]

Thanks for taking this on Luana!

I'm excited to merge this soon; what do you think of my suggestion below?

[Zac-HD]

I think I favor a simpler approach: rather than checking the contents of the expression, just reject those which contain only letters/space/dot:

Suggested change

	for node in ast.walk(tree):
	# Attribute access or calls, for example ``sys.platform.startswith('win')``.
	if isinstance(node, ast.Attribute | ast.Call):
	return True
	# Comparisons against values, for example ``os.name == 'nt'``; comparisons
	# of plain names are ignored as they are also valid English, for example
	# ``"nothing in particular"``.
	if isinstance(node, ast.Compare) and not all(
	isinstance(operand, ast.Name) for operand in [node.left, *node.comparators]
	):
	return True
	return False
	return re.fullmatch(r"[A-z .]+", reason) is None

it seems to me that this better captures the core "looks like valid english" issue, and is very simple.

[LuanaPBueno]

Thanks for the review, Zac! I like the idea of making it simpler. I have two small concerns about the regex, though:

'[A-z]' also matches a few non-letter characters ('[', '', ']', '^', '_'), since they come between 'Z' and 'a' in ASCII. I think '[A-Za-z .]' is what we want here.
Checking only for letters, spaces and dots would also warn on many common reasons that are just normal sentences, for example:

• "doesn't work on Windows" (apostrophe)
• "flaky, see seting the traceback type doesnt work anymore #123" (comma, '#', digits)
• "broken on Python 3.13" (digits)

I worry this could add a lot of noise to existing test suites, which is the same problem I ran into with the original rule from the issue.

One middle ground could be to keep the regex approach but allow more characters that are common in normal English, like '[\w\s.,'#:!?-]+'. That way digits, commas and apostrophes are fine, while things like '==', '<' or parentheses still warn. But I'm happy to go with whichever you prefer, either that or keeping the 'ast' version. What do you think?

[Zac-HD]

Checking only for letters, spaces and dots would also warn on many common reasons that are just normal sentences, for example:

These examples all raise an error in ast.parse(...) - my proposal is that we should warn on anything which parses as Python, unless it consists only of letters, space, and dot.

[ege-arhan]

Tested the branch head locally. A module- or class-level pytest.mark.skip now warns once per collected item, not once per mark.

src/_pytest/skipping.py:213-221: the warning is emitted inside the per-item loop over item.iter_markers(name="skip"), and iter_markers yields inherited marks for every item, so a mark that decorates 3 tests produces 3 identical warnings.

Repro:

import pytest
pytestmark = pytest.mark.skip("sys.platform == 'win32'")
def test_a(): pass
def test_b(): pass
def test_c(): pass

python -m pytest test_mod.py -W "default::pytest.PytestWarning" prints the same "looks like a condition" warning 3 times; same with a @pytest.mark.skip(...) class decorator (3 warnings for 3 methods). The added test only has a single test function, so it cannot catch this.

Suggestion: warn once per unique reason, e.g. keep a set() of already-warned reason strings on the session/config in evaluate_skip_marks, or move the check to collection time where the mark owner is known. Otherwise every module-level skip turns into noise proportional to the number of tests.

[LuanaPBueno]

@ege-arhan thanks for testing it! You’re right, module and class level marks were warning once per item. I now keep a set of already-warned reasons in the config stash, so each reason only warns once, and added a test with a module-level and a class-level skip (a213e48).