pytest.skip option to allow to work at module level

[nicoddemus]

As discussed in pytest-dev/pytest#2338

@The-Compiler wrote

---

The Skipped exception already has allow_module_level=True as argument which is used by pytest.importorskip internally - so you should already be able to do raise pytest.skip.Exception("...", allow_module_level=True).

I think we should just expose that via pytest.skip(...) too.

[georgeyk]

@nicoddemus Hi, can I try to fix this ?

I'm not familiar with the codebase, but it looks like the following:

• update https://github.com/pytest-dev/pytest/blob/0a15edd5732578f6cc40e28770f0fc87dfa5ced9/_pytest/outcomes.py#L65-L72 to add allow_module_level
• and here too: https://github.com/pytest-dev/pytest/blob/0a15edd5732578f6cc40e28770f0fc87dfa5ced9/_pytest/skipping.py#L156-L177
• tests and docs

Is that correct ?

[nicoddemus]

Hi @georgeyk actually it is simpler. 😁

pytest.skip raises Skipped, and that class already supports the flag we want:

https://github.com/pytest-dev/pytest/blob/6690b8a44420aba860e6f2c79ca296e61e249f3c/_pytest/outcomes.py#L32-L39

So it is just a matter of changing pytest.skip to accept a keyword argument allow_module_level and forward that to Skipped.

I would like for allow_module_level to be keyword-argument only, but given that we don't have support for that in Python 2 I would code pytest.skip like this:

def skip(msg="", **kwargs):
    """ ...
    :kwarg bool allow_module_level: allows this function to be called at the module level, skipping the 
        rest of the module. Default to False.
    """
    __tracebackhide__ = True
    allow_module_level = kwargs.pop('allow_module_level', False)
    if kwargs:
        raise TypeError('unexpected keyword arguments: {}'.format(kwargs.keys())
    raise Skipped(msg=msg, allow_module_level=allow_module_level)

And then a test and docs. 👍

[bilderbuchi]

@nicoddemus the related PR for this issue is merged.
I could not find the policy regarding when to close issues, i.e. close when the fix is merged into the repo or when it is released. It probably would make sense to document that in the contributor documents.
IMHO, the former makes more sense, as closed issues are nearly as discoverable as open ones on Github, and it keeps your issues list (i.e. the list of things that still need work) a bit cleaner.

[RonnyPfannschmidt]

thanks for the note - we try to close automatically with merging, but sometimes a reference is forgotten

[bilderbuchi]

OK, thanks.

[nicoddemus]

Our policy is to close as soon as the fix is merged either on master or features as you suggest.

What happened is that it was merged to the features branch, and GH will only handle commit commands when they are merged to master. Also sometimes people forget/don't know to add "Fix #X" to the commit message as well and we end up forgetting to close manually.

[bilderbuchi]

OK. I've used the GH API to automatically identify a handful of issues (back until ~ issue 2400) that should be closed, I'll @mention you to close them.