From c14a9adba35ac675ce3e825d34d01d5bb51748c3 Mon Sep 17 00:00:00 2001
From: Florian Bruhin <me@the-compiler.org>
Date: Thu, 4 Mar 2021 11:56:21 +0100
Subject: [PATCH] Fix skip signature (#8392)

* Fix test_strict_and_skip

The `--strict` argument was removed in #2552, but the removal wasn't
actually correct - see #1472.

* Fix argument handling in pytest.mark.skip

See #8384

* Raise from None

* Fix test name
---
 changelog/8384.bugfix.rst |  1 +
 doc/en/reference.rst      |  2 +-
 src/_pytest/skipping.py   | 13 +++++--------
 testing/test_skipping.py  | 18 +++++++++++++++++-
 4 files changed, 24 insertions(+), 10 deletions(-)
 create mode 100644 changelog/8384.bugfix.rst

diff --git a/changelog/8384.bugfix.rst b/changelog/8384.bugfix.rst
new file mode 100644
index 00000000000..3b70987490e
--- /dev/null
+++ b/changelog/8384.bugfix.rst
@@ -0,0 +1 @@
+The ``@pytest.mark.skip`` decorator now correctly handles its arguments. When the ``reason`` argument is accidentally given both positional and as a keyword (e.g. because it was confused with ``skipif``), a ``TypeError`` now occurs. Before, such tests were silently skipped, and the positional argument ignored. Additionally, ``reason`` is now documented correctly as positional or keyword (rather than keyword-only).
diff --git a/doc/en/reference.rst b/doc/en/reference.rst
index bc6c5670a5c..9ad82b3e4b9 100644
--- a/doc/en/reference.rst
+++ b/doc/en/reference.rst
@@ -150,7 +150,7 @@ pytest.mark.skip
 
 Unconditionally skip a test function.
 
-.. py:function:: pytest.mark.skip(*, reason=None)
+.. py:function:: pytest.mark.skip(reason=None)
 
     :keyword str reason: Reason why the test function is being skipped.
 
diff --git a/src/_pytest/skipping.py b/src/_pytest/skipping.py
index 1ad312919ca..7fe9783a4fa 100644
--- a/src/_pytest/skipping.py
+++ b/src/_pytest/skipping.py
@@ -161,7 +161,7 @@ def evaluate_condition(item: Item, mark: Mark, condition: object) -> Tuple[bool,
 class Skip:
     """The result of evaluate_skip_marks()."""
 
-    reason = attr.ib(type=str)
+    reason = attr.ib(type=str, default="unconditional skip")
 
 
 def evaluate_skip_marks(item: Item) -> Optional[Skip]:
@@ -184,13 +184,10 @@ def evaluate_skip_marks(item: Item) -> Optional[Skip]:
                 return Skip(reason)
 
     for mark in item.iter_markers(name="skip"):
-        if "reason" in mark.kwargs:
-            reason = mark.kwargs["reason"]
-        elif mark.args:
-            reason = mark.args[0]
-        else:
-            reason = "unconditional skip"
-        return Skip(reason)
+        try:
+            return Skip(*mark.args, **mark.kwargs)
+        except TypeError as e:
+            raise TypeError(str(e) + " - maybe you meant pytest.mark.skipif?") from None
 
     return None
 
diff --git a/testing/test_skipping.py b/testing/test_skipping.py
index fc66eb18e64..349de6e080f 100644
--- a/testing/test_skipping.py
+++ b/testing/test_skipping.py
@@ -861,9 +861,25 @@ def test_hello():
                 pass
         """
         )
-        result = pytester.runpytest("-rs")
+        result = pytester.runpytest("-rs", "--strict-markers")
         result.stdout.fnmatch_lines(["*unconditional skip*", "*1 skipped*"])
 
+    def test_wrong_skip_usage(self, pytester: Pytester) -> None:
+        pytester.makepyfile(
+            """
+            import pytest
+            @pytest.mark.skip(False, reason="I thought this was skipif")
+            def test_hello():
+                pass
+        """
+        )
+        result = pytester.runpytest()
+        result.stdout.fnmatch_lines(
+            [
+                "*TypeError: __init__() got multiple values for argument 'reason' - maybe you meant pytest.mark.skipif?"
+            ]
+        )
+
 
 class TestSkipif:
     def test_skipif_conditional(self, pytester: Pytester) -> None: