Warn when pytest.mark.skip reason looks like a condition

changelog/12582.improvement.rst

@@ -0,0 +1 @@
+pytest now warns when the reason given to ``pytest.mark.skip`` looks like a condition (for example ``"sys.platform == 'win32'"``), since ``pytest.mark.skipif`` was probably intended.

src/_pytest/skipping.py

@@ -3,6 +3,7 @@
 
 from __future__ import annotations
 
+import ast
 from collections.abc import Generator
 from collections.abc import Mapping
 import dataclasses
@@ -24,6 +25,7 @@
 from _pytest.reports import TestReport
 from _pytest.runner import CallInfo
 from _pytest.stash import StashKey
+from _pytest.warning_types import PytestWarning
 
 
 def pytest_addoption(parser: Parser) -> None:
@@ -166,6 +168,32 @@ class Skip:
     reason: str = "unconditional skip"
 
 
+warned_skip_reasons_key = StashKey[set[str]]()
+
+
+def _looks_like_condition(reason: object) -> bool:
+    """Return whether a skip reason looks like a skipif condition, e.g.
+    ``"sys.platform == 'win32'"`` (#12582)."""
+    if not isinstance(reason, str):
+        return False
+    try:
+        tree = ast.parse(reason.strip(), mode="eval")
+    except (SyntaxError, ValueError):
+        return False
+    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
+
+
 def evaluate_skip_marks(item: Item) -> Skip | None:
     """Evaluate skip and skipif marks on item, returning Skip if triggered."""
     for mark in item.iter_markers(name="skipif"):
@@ -186,6 +214,22 @@ 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)
+            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?"
+                )
+            )
         try:
             return Skip(*mark.args, **mark.kwargs)
         except TypeError as e:

testing/test_skipping.py

@@ -1567,3 +1567,76 @@ def test_exit_reason_only():
     )
     result = pytester.runpytest(p)
     result.stdout.fnmatch_lines("*_pytest.outcomes.Exit: foo*")
+
+
+@pytest.mark.parametrize(
+    "reason",
+    [
+        "sys.version_info < (3, 6)",
+        "sys.platform.startswith('win')",
+        "os.name == 'nt'",
+    ],
+)
+def test_skip_reason_looks_like_condition_warns(
+    pytester: Pytester, reason: str
+) -> None:
+    pytester.makepyfile(
+        f"""
+        import pytest
+
+        @pytest.mark.skip({reason!r})
+        def test_func():
+            pass
+        """
+    )
+    result = pytester.runpytest("-W", "default::pytest.PytestWarning")
+    result.stdout.fnmatch_lines(
+        ["*looks like a condition*Did you mean pytest.mark.skipif?"]
+    )
+    result.assert_outcomes(skipped=1, warnings=1)
+
+
+@pytest.mark.parametrize(
+    "reason",
+    [
+        "flaky",
+        "not supported on windows",
+        "nothing in particular",
+        "Test is not isolated",
+    ],
+)
+def test_skip_reason_plain_text_does_not_warn(pytester: Pytester, reason: str) -> None:
+    pytester.makepyfile(
+        f"""
+        import pytest
+
+        @pytest.mark.skip({reason!r})
+        def test_func():
+            pass
+        """
+    )
+    result = pytester.runpytest("-W", "default::pytest.PytestWarning")
+    result.assert_outcomes(skipped=1, warnings=0)
+
+
+def test_skip_reason_looks_like_condition_warns_once_per_reason(
+    pytester: Pytester,
+) -> None:
+    """A module- or class-level mark applies to every item, but warns only once."""
+    pytester.makepyfile(
+        """
+        import pytest
+
+        pytestmark = pytest.mark.skip("sys.platform == 'win32'")
+
+        def test_a(): pass
+        def test_b(): pass
+
+        @pytest.mark.skip("os.name == 'nt'")
+        class TestClass:
+            def test_c(self): pass
+            def test_d(self): pass
+        """
+    )
+    result = pytester.runpytest("-W", "default::pytest.PytestWarning")
+    result.assert_outcomes(skipped=4, warnings=2)