-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Fix skip signature #8392
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.
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
Fix skip signature #8392
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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). |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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?") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not doing "from e" here because it's not really relevant to the user where the exception comes from. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would from None Make sense to completely suppress it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems reasonable - added! |
||
|
||
return None | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -861,9 +861,25 @@ def test_hello(): | |
pass | ||
""" | ||
) | ||
result = pytester.runpytest("-rs") | ||
result = pytester.runpytest("-rs", "--strict-markers") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not directly related, see #2552 (comment) |
||
result.stdout.fnmatch_lines(["*unconditional skip*", "*1 skipped*"]) | ||
|
||
def test_wrong_strict_usage(self, pytester: Pytester) -> None: | ||
The-Compiler marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the existing
__init__
rather than hand-rolling argument parsing will also catch other issues, like giving too many arguments to the mark. All those were silently ignored before.