From 19fc04f06755434a2c6d3e002993dff17bd2d8c0 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 3 Mar 2021 14:01:02 +0100 Subject: [PATCH 1/4] Fix test_strict_and_skip The `--strict` argument was removed in #2552, but the removal wasn't actually correct - see #1472. --- testing/test_skipping.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/test_skipping.py b/testing/test_skipping.py index fc66eb18e64..87d4cd3b279 100644 --- a/testing/test_skipping.py +++ b/testing/test_skipping.py @@ -861,7 +861,7 @@ def test_hello(): pass """ ) - result = pytester.runpytest("-rs") + result = pytester.runpytest("-rs", "--strict-markers") result.stdout.fnmatch_lines(["*unconditional skip*", "*1 skipped*"]) From 70e967f51a23289f8ab4d0865a55bb1ce3ba8e09 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 3 Mar 2021 14:38:01 +0100 Subject: [PATCH 2/4] Fix argument handling in pytest.mark.skip See #8384 --- changelog/8384.bugfix.rst | 1 + doc/en/reference.rst | 2 +- src/_pytest/skipping.py | 13 +++++-------- testing/test_skipping.py | 16 ++++++++++++++++ 4 files changed, 23 insertions(+), 9 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..66ee6def7c8 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?") return None diff --git a/testing/test_skipping.py b/testing/test_skipping.py index 87d4cd3b279..7fc87c63efe 100644 --- a/testing/test_skipping.py +++ b/testing/test_skipping.py @@ -864,6 +864,22 @@ def test_hello(): result = pytester.runpytest("-rs", "--strict-markers") result.stdout.fnmatch_lines(["*unconditional skip*", "*1 skipped*"]) + def test_wrong_strict_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: From 02d9dfe65019c69a8262a05d203ac5e7dc40a9d8 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 3 Mar 2021 17:32:58 +0100 Subject: [PATCH 3/4] Raise from None --- src/_pytest/skipping.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/skipping.py b/src/_pytest/skipping.py index 66ee6def7c8..7fe9783a4fa 100644 --- a/src/_pytest/skipping.py +++ b/src/_pytest/skipping.py @@ -187,7 +187,7 @@ def evaluate_skip_marks(item: Item) -> Optional[Skip]: try: return Skip(*mark.args, **mark.kwargs) except TypeError as e: - raise TypeError(str(e) + " - maybe you meant pytest.mark.skipif?") + raise TypeError(str(e) + " - maybe you meant pytest.mark.skipif?") from None return None From 8eff9a17200839fd26227b76db5747bbbdfb1c29 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 4 Mar 2021 10:58:51 +0100 Subject: [PATCH 4/4] Fix test name --- testing/test_skipping.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/test_skipping.py b/testing/test_skipping.py index 7fc87c63efe..349de6e080f 100644 --- a/testing/test_skipping.py +++ b/testing/test_skipping.py @@ -864,7 +864,7 @@ def test_hello(): result = pytester.runpytest("-rs", "--strict-markers") result.stdout.fnmatch_lines(["*unconditional skip*", "*1 skipped*"]) - def test_wrong_strict_usage(self, pytester: Pytester) -> None: + def test_wrong_skip_usage(self, pytester: Pytester) -> None: pytester.makepyfile( """ import pytest