Allow module level skip #2808

georgeyk · 2017-10-01T21:45:40Z

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:

Add a new news fragment into the changelog folder
- name it $issue_id.$type for example (588.bug)
- if you don't have an issue_id change it to the pr id after creating the pr
- ensure type is one of removal, feature, bugfix, vendor, doc or trivial
- Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."
Target: for bugfix, vendor, doc or trivial fixes, target master; for removals or features target features;
Make sure to include reasonable tests for your change if necessary

Unless your change is a trivial or a documentation fix (e.g., a typo or reword of a small section) please:

Add yourself to AUTHORS;

coveralls · 2017-10-01T22:08:36Z

Coverage increased (+0.003%) to 92.235% when pulling 0fb31f8 on georgeyk:allow-module-level-skip into 6690b8a on pytest-dev:master.

nicoddemus

Great work!

Aside from my comments, we also need a changelog file.

nicoddemus · 2017-10-01T23:38:48Z

_pytest/outcomes.py

    """ skip an executing test with the given message.  Note: it's usually
    better to use the pytest.mark.skipif marker to declare a test to be
    skipped under certain conditions like mismatching platforms or
    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.


I think this needs to be indented otherwise tox -e docs fails

nicoddemus · 2017-10-01T23:39:23Z

_pytest/outcomes.py

+    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))


Needs to use {0} to work on Python 2.6:

raise TypeError('unexpected keyworkd arguments: {0}'.format(keys))

keyworkd -> keyword

The-Compiler · 2017-10-02T04:58:33Z

This probably should go to the features branch.

RonnyPfannschmidt · 2017-10-02T05:51:26Z

doc/en/skipping.rst

+
+
+.. code-block:: python
+


i do wonder if there is a better example for this (thats not looking like a skipif marker or a importorskip would already solve

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)

georgeyk · 2017-10-02T23:36:52Z

Should I close this and re-open targeting the features branch ?

nicoddemus · 2017-10-02T23:39:15Z

You can rebase on the features branch, push force to this branch and then edit the target branch of the PR here on GH to features.

…kwarg

georgeyk · 2017-10-03T01:58:10Z

Is this event.when supposed to exist in CollectReport instance ? https://github.com/pytest-dev/pytest/blob/features/_pytest/skipping.py#L352

nicoddemus · 2017-10-03T23:03:01Z

Hmm folded_skips is relatively new I think. @RonnyPfannschmidt do you have a suggestion on how to proceed here?

RonnyPfannschmidt · 2017-10-04T06:42:30Z

nope

RonnyPfannschmidt · 2017-10-04T06:43:10Z

actually i do, whole module skips should be ignored, collectrports have no when

so at collect time we should never fold skips

nicoddemus · 2017-10-04T10:56:41Z

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:

Traceback (most recent call last):
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/_pytest/pytester.py", line 744, in runpytest_inprocess
    reprec = self.inline_run(*args, **kwargs)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/_pytest/pytester.py", line 717, in inline_run
    ret = pytest.main(list(args), plugins=plugins)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/_pytest/config.py", line 59, in main
    return config.hook.pytest_cmdline_main(config=config)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 745, in __call__
    return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 339, in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 334, in <lambda>
    _MultiCall(methods, kwargs, hook.spec_opts).execute()
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 614, in execute
    res = hook_impl.function(*args)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/_pytest/main.py", line 131, in pytest_cmdline_main
    return wrap_session(config, _main)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/_pytest/main.py", line 125, in wrap_session
    exitstatus=session.exitstatus)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 745, in __call__
    return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 339, in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 302, in __call__
    return outcome.get_result()
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 279, in get_result
    raise ex[1].with_traceback(ex[2])
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 265, in __init__
    self.result = func()
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 300, in <lambda>
    outcome = _CallOutcome(lambda: self.oldcall(hook, hook_impls, kwargs))
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 334, in <lambda>
    _MultiCall(methods, kwargs, hook.spec_opts).execute()
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 613, in execute
    return _wrapped_call(hook_impl.function(*args), self.execute)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 250, in _wrapped_call
    wrap_controller.send(call_outcome)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/_pytest/terminal.py", line 427, in pytest_sessionfinish
    exitstatus=exitstatus)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 745, in __call__
    return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 339, in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 302, in __call__
    return outcome.get_result()
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 279, in get_result
    raise ex[1].with_traceback(ex[2])
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 265, in __init__
    self.result = func()
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 300, in <lambda>
    outcome = _CallOutcome(lambda: self.oldcall(hook, hook_impls, kwargs))
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 334, in <lambda>
    _MultiCall(methods, kwargs, hook.spec_opts).execute()
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/pluggy.py", line 614, in execute
    res = hook_impl.function(*args)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/_pytest/skipping.py", line 291, in pytest_terminal_summary
    show_skipped(terminalreporter, lines)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/_pytest/skipping.py", line 370, in show_skipped
    fskips = folded_skips(skipped)
  File "/home/travis/build/pytest-dev/pytest/.tox/py36/lib/python3.6/site-packages/_pytest/skipping.py", line 352, in folded_skips
    if event.when == 'setup' and 'skip' in keywords and 'pytestmark' not in keywords:
AttributeError: 'CollectReport' object has no attribute 'when'

RonnyPfannschmidt · 2017-10-04T12:09:56Z

correct - a skip in a collect-report is added to the skip stats, and such a stat shouldn't be folded in the display

RonnyPfannschmidt · 2017-10-04T12:11:43Z

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

nicoddemus · 2017-10-04T13:43:31Z

correct - a skip in a collect-report is added to the skip stats, and such a stat shouldn't be folded in the display

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 folded_skips.

nicoddemus · 2017-10-04T14:25:57Z

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:

coveralls · 2017-10-04T22:45:36Z

Coverage increased (+0.006%) to 92.506% when pulling 03ce0ad on georgeyk:allow-module-level-skip into d132c50 on pytest-dev:features.

nicoddemus

Thanks @georgeyk, well done!

georgeyk · 2017-10-04T23:06:49Z

@nicoddemus Thank you all to support this great project. I hope to do more in the future!

nicoddemus · 2017-10-04T23:11:12Z

I hope to do more in the future!

We really appreciate it, thanks!

coveralls · 2017-10-05T02:04:58Z

Coverage increased (+0.006%) to 92.506% when pulling 0668a6c on georgeyk:allow-module-level-skip into d132c50 on pytest-dev:features.

nicoddemus · 2017-10-11T23:00:10Z

@RonnyPfannschmidt you think we can merge this?

nicoddemus reviewed Oct 1, 2017

View reviewed changes

RonnyPfannschmidt reviewed Oct 2, 2017

View reviewed changes

georgeyk added 6 commits October 2, 2017 21:07

Add allow_module_level kwarg to skip helper

79d3353

Add initial tests using skip with allow_module_level kwarg

06307be

Update skipping documentation to include usage of allow_module_level …

e4a6e52

…kwarg

Fix docstring alignment and typos

c1aa63c

Update documentation example of pytest.skip(allow_module_level=True)

59f6693

Add 2808.feature changelog entry

9824499

georgeyk force-pushed the allow-module-level-skip branch from 0fb31f8 to 9824499 Compare October 3, 2017 00:27

georgeyk changed the base branch from master to features October 3, 2017 00:27

Fix: handle CollectReport in folded_skips function

03ce0ad

nicoddemus approved these changes Oct 4, 2017

View reviewed changes

Add myself to authors file

0668a6c

RonnyPfannschmidt merged commit df37cdf into pytest-dev:features Oct 12, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow module level skip #2808

Allow module level skip #2808

georgeyk commented Oct 1, 2017 •

edited

Loading

coveralls commented Oct 1, 2017

nicoddemus left a comment

nicoddemus Oct 1, 2017

nicoddemus Oct 1, 2017

The-Compiler Oct 2, 2017

The-Compiler commented Oct 2, 2017

RonnyPfannschmidt Oct 2, 2017

georgeyk Oct 2, 2017

georgeyk commented Oct 2, 2017

nicoddemus commented Oct 2, 2017

georgeyk commented Oct 3, 2017

nicoddemus commented Oct 3, 2017

RonnyPfannschmidt commented Oct 4, 2017

RonnyPfannschmidt commented Oct 4, 2017

nicoddemus commented Oct 4, 2017

RonnyPfannschmidt commented Oct 4, 2017

RonnyPfannschmidt commented Oct 4, 2017

nicoddemus commented Oct 4, 2017

nicoddemus commented Oct 4, 2017

coveralls commented Oct 4, 2017

nicoddemus left a comment

georgeyk commented Oct 4, 2017

nicoddemus commented Oct 4, 2017

coveralls commented Oct 5, 2017

nicoddemus commented Oct 11, 2017

Allow module level skip #2808

Allow module level skip #2808

Conversation

georgeyk commented Oct 1, 2017 • edited Loading

coveralls commented Oct 1, 2017

nicoddemus left a comment

Choose a reason for hiding this comment

nicoddemus Oct 1, 2017

Choose a reason for hiding this comment

nicoddemus Oct 1, 2017

Choose a reason for hiding this comment

The-Compiler Oct 2, 2017

Choose a reason for hiding this comment

The-Compiler commented Oct 2, 2017

RonnyPfannschmidt Oct 2, 2017

Choose a reason for hiding this comment

georgeyk Oct 2, 2017

Choose a reason for hiding this comment

georgeyk commented Oct 2, 2017

nicoddemus commented Oct 2, 2017

georgeyk commented Oct 3, 2017

nicoddemus commented Oct 3, 2017

RonnyPfannschmidt commented Oct 4, 2017

RonnyPfannschmidt commented Oct 4, 2017

nicoddemus commented Oct 4, 2017

RonnyPfannschmidt commented Oct 4, 2017

RonnyPfannschmidt commented Oct 4, 2017

nicoddemus commented Oct 4, 2017

nicoddemus commented Oct 4, 2017

coveralls commented Oct 4, 2017

nicoddemus left a comment

Choose a reason for hiding this comment

georgeyk commented Oct 4, 2017

nicoddemus commented Oct 4, 2017

coveralls commented Oct 5, 2017

nicoddemus commented Oct 11, 2017

georgeyk commented Oct 1, 2017 •

edited

Loading