Skip to content

ENH: allow suspicious-looking reasons in skip when passed as keyword without warning #15367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
neutrinoceros wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ENH: allow suspicious-looking reasons in skip when passed as keyword without warning #15367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/_pytest/skipping.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,20 +204,23 @@ def evaluate_skip_marks(item: Item) -> Skip | None:
return Skip(reason)

for mark in item.iter_markers(name="skip"):
reason = mark.args[0] if mark.args else mark.kwargs.get("reason")
# Marks applied to a module or class are seen by each of their items,
# so only warn once per reason.
warned_reasons = item.config.stash.setdefault(warned_skip_reasons_key, set())
if (
isinstance(reason, str)
"reason" not in mark.kwargs
and mark.args
and isinstance(reason := mark.args[0], str)
and _looks_like_condition(reason)
and reason not in warned_reasons
):
warned_reasons.add(reason)
item.warn(
PytestWarning(
f"The reason {reason!r} of pytest.mark.skip looks like a condition. "
"Did you mean pytest.mark.skipif?"
"Did you mean pytest.mark.skipif ?\n"
"If intentional, pass reason as a keyword argument to disable "
"this warning."
)
)
try:
Expand Down
42 changes: 32 additions & 10 deletions testing/test_skipping.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1569,16 +1569,17 @@ def test_exit_reason_only():
result.stdout.fnmatch_lines("*_pytest.outcomes.Exit: foo*")


@pytest.mark.parametrize(
"reason",
[
"sys.version_info < (3, 6)",
"sys.platform.startswith('win')",
"os.name == 'nt'",
],
)
SUSPECT_REASONS = [
"sys.version_info < (3, 6)",
"sys.platform.startswith('win')",
"os.name == 'nt'",
]


@pytest.mark.parametrize("reason", SUSPECT_REASONS)
def test_skip_reason_looks_like_condition_warns(
pytester: Pytester, reason: str
pytester: Pytester,
reason: str,
) -> None:
pytester.makepyfile(
f"""
Expand All @@ -1591,11 +1592,32 @@ def test_func():
)
result = pytester.runpytest("-W", "default::pytest.PytestWarning")
result.stdout.fnmatch_lines(
["*looks like a condition*Did you mean pytest.mark.skipif?"]
[
"*looks like a condition*Did you mean pytest.mark.skipif ?",
"*If intentional, pass reason as a keyword argument to disable this warning.",
]
)
result.assert_outcomes(skipped=1, warnings=1)


@pytest.mark.parametrize("reason", SUSPECT_REASONS)
def test_skip_reason_looks_like_condition_kwarg(
pytester: Pytester,
reason: str,
) -> None:
pytester.makepyfile(
f"""
import pytest

@pytest.mark.skip({reason=!r})

@RonnyPfannschmidt RonnyPfannschmidt Nov 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

at first glance that format string looks wrong, the equals form is something one has to get used to 🦺

@neutrinoceros neutrinoceros Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Would you prefer the following, equivalent form ?

Suggested change
@pytest.mark.skip({reason=!r})
@pytest.mark.skip(reason={reason!r})

(for reference, the syntax I used was introduced in Python 3.8)

@RonnyPfannschmidt RonnyPfannschmidt Nov 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's fine as is im just bemused that the "new" syntax looks "wrong " to me

def test_func():
pass
"""
)
result = pytester.runpytest("-W", "default::pytest.PytestWarning")
result.assert_outcomes(skipped=1, warnings=0)


@pytest.mark.parametrize(
"reason",
[
Expand Down
Loading