Allow module level skip#2808
Conversation
nicoddemus
left a comment
nicoddemus
left a comment
There was a problem hiding this comment.
Great work!
Aside from my comments, we also need a changelog file.
_pytest/outcomes.py
Outdated
| dependencies. See the pytest_skipping plugin for details. | ||
|
|
||
| :kwarg bool allow_module_level: allows this function to be called at | ||
| module level, skipping the rest of the module. Default to False. |
There was a problem hiding this comment.
I think this needs to be indented otherwise tox -e docs fails
_pytest/outcomes.py
Outdated
| allow_module_level = kwargs.pop('allow_module_level', False) | ||
| if kwargs: | ||
| keys = [k for k in kwargs.keys()] | ||
| raise TypeError('unexpected keyworkd arguments: {}'.format(keys)) |
There was a problem hiding this comment.
Needs to use {0} to work on Python 2.6:
raise TypeError('unexpected keyworkd arguments: {0}'.format(keys))
|
This probably should go to the |
|
|
||
|
|
||
| .. code-block:: python | ||
|
|
There was a problem hiding this comment.
i do wonder if there is a better example for this (thats not looking like a skipif marker or a importorskip would already solve
There was a problem hiding this comment.
Maybe using the use case from previous discussion ? Something like:
if not pytest.config.getoption('--custom-flag'):
pytest.skip('--custom-flag is missing, skipping tests', allow_module_level=True)
|
Should I close this and re-open targeting the features branch ? |
|
You can rebase on the |
georgeyk
force-pushed
the
allow-module-level-skip
branch
from
0fb31f8 to
9824499
Compare
|
Is this |
|
Hmm |
|
nope |
|
actually i do, whole module skips should be ignored, collectrports have no when so at collect time we should never fold skips |
I see, thanks. Could you tell @georgeyk what should be changed then? I'm not sure what should be done with collectreports. And the error is happening at the end of the session, when we are about to show the terminal summary, not during collection: |
|
correct - a skip in a collect-report is added to the skip stats, and such a stat shouldn't be folded in the display |
|
one solution could be to add a when property to collect reports another solution could be to just ignore them its not clear what has the best effect |
That was my first thought too, but after thinking some more I think it might make sense in some cases. For example, if (say) 3 whole modules are skipped for the same reason, it makes sense to fold it. In light of that it might make sense to just make a special case for it in |
|
Something like (untested): when = getattr(event, 'when', None) # might be a collection report
if when == 'setup' and 'skip' in keywords and 'pytestmark' not in keywords: |
nicoddemus
left a comment
nicoddemus
left a comment
There was a problem hiding this comment.
Thanks @georgeyk, well done!
|
@nicoddemus Thank you all to support this great project. I hope to do more in the future! |
We really appreciate it, thanks! |
|
@RonnyPfannschmidt you think we can merge this? |
5 participants
Work-in-progress fix for #2805
I need to update the PR with all contributing guideline recommendations, but a initial feedback would be good.
--//--
Thanks for submitting a PR, your contribution is really appreciated!
Here's a quick checklist that should be present in PRs:
$issue_id.$typefor example (588.bug)removal,feature,bugfix,vendor,docortrivialbugfix,vendor,docortrivialfixes, targetmaster; for removals or features targetfeatures;Unless your change is a trivial or a documentation fix (e.g., a typo or reword of a small section) please:
AUTHORS;