pytest icon indicating copy to clipboard operation
pytest copied to clipboard

silently skip a test

Open notestaff opened this issue 7 years ago • 45 comments

Sometimes a test should always be skipped, e.g. because we generate tests by first generating all possible combinations of parameters and then calling pytest.skip inside the test function for combinations that don't make sense. That's different from tests that are skipped in a particular test run, but that might be run in another test run (e.g. on different hardware or when a particular feature is added). Could you add a way to mark tests that should always be skipped, and have them reported separately from tests that are sometimes skipped? When a report says "400 tests skipped" it looks like something is wrong, but maybe 390 of these should always be skipped, and just 10 should be tested later when conditions permit. Also, the "% of tests done" status message becomes distorted when always-skipped tests are included.

notestaff avatar Jul 29 '18 19:07 notestaff

GitMate.io thinks possibly related issues are https://github.com/pytest-dev/pytest/issues/1563 (All pytest tests skipped), https://github.com/pytest-dev/pytest/issues/251 (dont ignore test classes with a own constructor silently), https://github.com/pytest-dev/pytest/issues/1364 (disallow test skipping), https://github.com/pytest-dev/pytest/issues/149 (Distributed testing silently ignores collection errors), and https://github.com/pytest-dev/pytest/issues/153 (test intents).

pytestbot avatar Jul 29 '18 19:07 pytestbot

See also #3844.

blueyed avatar Aug 25 '18 20:08 blueyed

[this is copied from a comment in #3844 that is now closed for unrelated reasons; I would suggest the syntax @pytest.mark.ignore() and pytest.ignore, in line with how skip currently works]

This would be extremely useful to me in several scenarios.

Pytest makes it easy (esp. through parametrization and parametrized fixtures) to test a cartesian product of parameter combinations. It's slightly less easy (not least because fixtures can't be reused as parameters) to reduce that cartesian product where necessary.

Often, certain combination simply do not make sense (for what's being tested), and currently, one can only really skip / xfail them (unless one starts complicated plumbing and splitting up of the parameters/fixtures).

But skips and xfails get output to the log (and for good reason - they should command attention and be eventually fixed), and so it's quite a simple consideration that one does not want to pollute the result with skipping invalid parameter combinations.

For this task, pytest.ignore would be the perfect tool. That this would be very intuitive is underlined by the fact that I wanted to open just such an issue before I found the exact same request here already.

h-vetinari avatar Nov 14 '18 08:11 h-vetinari

pytest.ignore is inconsistent and shouldn't exist it because the time it can happen the test is already running - by that time "ignore" would be a lie

at a work project we have the concept of "uncollect" for tests, which can take a look at fixture values to conditionally remove tests from the collection at modifyitems time

RonnyPfannschmidt avatar Nov 14 '18 08:11 RonnyPfannschmidt

@RonnyPfannschmidt

pytest.ignore is inconsistent and shouldn't exist it because the time it can happen the test is already running - by that time "ignore" would be a lie

What's so bad a bout "lying" if it's in the interest of reporting/logging, and directly requested by the user?

at a work project we have the concept of "uncollect" for tests, which can take a look at fixture values to conditionally remove tests from the collection at modifyitems time

"At work" sounds like "not in pytest (yet)". Can you elaborate how that works?

h-vetinari avatar Nov 14 '18 22:11 h-vetinari

@h-vetinari the lies have complexity cost - a collect time "ignore" doesnt have to mess around with the other reporting, it can jsut take the test out cleanly - a outcome level ignore needs a new special case for all report outcome handling and in some cases cant correctly handle it to begin with

for example whats the correct way to report an ignore triggered in the teardown of a failed test - its simply insane

as for the work project, it pretty much just goes off at pytest-modifyitems time and partitions based on a marker and conditionals that take the params

RonnyPfannschmidt avatar Nov 15 '18 05:11 RonnyPfannschmidt

@h-vetinari As for handling this during collection time, see https://github.com/pytest-dev/pytest/issues/4377#issuecomment-438173521 for an example, and for docs: https://docs.pytest.org/en/latest/reference.html?highlight=pytest_collection_modifyitems#_pytest.hookspec.pytest_collection_modifyitems. I think it should work to remove the items that "do not make sense" there.

blueyed avatar Nov 15 '18 08:11 blueyed

@blueyed Thanks for the response! That seems like it would work, but is not very extensible, as I'd have to pollute my pytest_collection_modifyitems hook with the individual "ignores" for the relevant tests (which differ from test to test).

Is there a way to add a hook that modifies the collection directly at the test itself, without changing global behaviour?

h-vetinari avatar Nov 18 '18 14:11 h-vetinari

@RonnyPfannschmidt Thanks for the response. Obviously, I don't have anywhere near as good of an overview as you, I'm just a simple user. ;-)

for example whats the correct way to report an ignore triggered in the teardown of a failed test - its simply insane

You're right, that skips only make sense at the beginning of a test - I would not have used it anywhere else in a test (e.g. after something that can fail), but I can see the problem from an API design perspective.

This is then getting closer again to the question I just asked to @blueyed, of having a per-test post-collection (or rather pre-execution) hook, to uncollect some tests. The essential part is that I need to be able to inspect the actual value of the parametrized fixtures per test, to be able to decide whether to ignore or not.

as for the work project, it pretty much just goes off at pytest-modifyitems time and partitions based on a marker and conditionals that take the params

This sounds great (if the params are the fixtures), but I'd need this on a per-test basis (maybe as a decorator that takes a function of the same signature as the test?).

h-vetinari avatar Nov 18 '18 14:11 h-vetinari

@h-vetinari an extracted solution of what i did at work would have 2 components

a) a hook to determine the namespace/kwargs for maker conditionals b) a marker to control the deselection (most likely @pytest.mark.deselect(*conditions, reason=...)

CC @psav

RonnyPfannschmidt avatar Nov 18 '18 22:11 RonnyPfannschmidt

@RonnyPfannschmidt @h-vetinari This has been stale for a while, but I've just come across the same issue - how do you 'silently unselect' a test? Are there any new solutions or propositions? If you have a large highly-dimensional parametrize-grid, this is needed quite often so you don't run (or even collect) the tests whose parameters don't make sense.

aldanor avatar Dec 12 '19 13:12 aldanor

@aldanor I haven't followed this further, but would still love to have this feature! :)

h-vetinari avatar Dec 12 '19 14:12 h-vetinari

the only way to completely "unselect" is not to generate

the next best thing is to deselect at collect time

RonnyPfannschmidt avatar Dec 12 '19 17:12 RonnyPfannschmidt

As @h-vetinari pointed out, sometimes "not generate" is not really an option, e.g. you have a highly-dimensional grid of parameters and you need to unselect just a few that don't make sense, and you don't want for them to show up as 'skipped', because they weren't really skipped.

So, as noted above, perhaps @pytest.mark.deselect(lambda x: ...) or something similar would work then?

aldanor avatar Dec 13 '19 14:12 aldanor

@aldanor Why not then do something along the lines of

@pytest.mark.parametrize('test_name',[parameters for parameters in 
						existing_parameter_generation() if make_sense(parameters)]

and that way not generate them

Tadaboody avatar Dec 16 '19 18:12 Tadaboody

@Tadaboody's suggestion is on point I believe. You can always preprocess the parameter list yourself and deselect the parameters as appropriate. @aldanor @h-vetinari @notestaff does that solve your issue?

nicoddemus avatar Dec 17 '19 22:12 nicoddemus

@Tadaboody @nicoddemus

Not always, as I've mentioned above:

If you have a large highly-dimensional parametrize-grid

For instance,

@pytest.mark.parametrize('x1', list(range(10)), ids='x1={}'.format)
@pytest.mark.parametrize('x2', list(range(10)), ids='x2={}'.format)
@pytest.mark.parametrize('x3', list(range(10)), ids='x3={}'.format)
@pytest.mark.parametrize('x4', list(range(10)), ids='x4={}'.format)
def test_foo(x1, x2, x3, x4):
    # we want to silently ignore some of these 10k points

Yes, you could argue that you could rewrite the above using a single list comprehensions, then having to rewrite formatting, the whole thing becoming more ugly, less flexible to extend, and your parameter generation now being mixed up with deselection logic.

You can always preprocess the parameter list yourself and deselect the parameters as appropriate

If in the example above it would have sort of worked as a hack, but it fails completely once you have reusable parametrization:

@pytest.fixture(params=list(range(10)), ids='x1={}'.format)
def x1(request):
    return request.param

@pytest.fixture(params=list(range(10)), ids='x2={}'.format)
def x2(request):
    return request.param

@pytest.fixture(params=list(range(10)), ids='x3={}'.format)
def x3(request):
    return request.param

@pytest.fixture(params=list(range(10)), ids='x4={}'.format)
def x4(request):
    return request.param

def test_123(x1, x2, x3):
    # we want to to deselect some of these 1k points

def test_234(x2, x3, x4):
    # we want to ignore some of these 1k points too

aldanor avatar Dec 18 '19 00:12 aldanor

@aldanor thanks for the example.

What do you suggest the API for skipping tests be?

nicoddemus avatar Dec 18 '19 16:12 nicoddemus

@nicoddemus @aldanor

How about something like

@pytest.fixture.unselectif(function_of_x1_to_x3)
def test_123(x1, x2, x3):
    # we want to to deselect some of these 1k points

resp.

@pytest.fixture.unselectif(function_of_x1_to_x4)
@pytest.mark.parametrize('x1', list(range(10)), ids='x1={}'.format)
@pytest.mark.parametrize('x2', list(range(10)), ids='x2={}'.format)
@pytest.mark.parametrize('x3', list(range(10)), ids='x3={}'.format)
@pytest.mark.parametrize('x4', list(range(10)), ids='x4={}'.format)
def test_foo(x1, x2, x3, x4):
    # we want to silently ignore some of these 10k points

This fixture.unselectif should take a function lambda *args: something with exactly the same signature as the test (or at least a subset thereof). It could quite freely error if it doesn't like what it's seeing (e.g. requesting something that's not a known fixture), but - assuming everything is set up correctly - would be able to unselect the corresponding parameters already at selection-time.

Note: the name is just an example, and obviously completely up for bikeshedding. After pressing "comment" I immediately thought it should rather be fixture.uncollect.

h-vetinari avatar Dec 18 '19 16:12 h-vetinari

IIUC how pytest works, once you've entered the test function body, it's already too late. So there's not a whole lot of choices really, it probably has to be something like

@pytest.mark.select(lambda x: x[0] > x[1])
def test_123(x1, x2, x3):
    ...
  • You could also have @pytest.mark.deselect for convenience. But if only one existed, I'd prefer the former.
  • You could also have reason=... optional kwarg, either a string or a lambda.

aldanor avatar Dec 18 '19 16:12 aldanor

@h-vetinari @aldanor it seems what you want is already possible with a custom hook and mark:

# contents of conftest.py
def pytest_configure(config):
    config.addinivalue_line(
        "markers", "uncollect_if(*, func): function to unselect tests from parametrization"
    )

def pytest_collection_modifyitems(config, items):
    removed = []
    kept = []
    for item in items:
        m = item.get_closest_marker('uncollect_if')
        if m:
            func = m.kwargs['func']
            if func(**item.callspec.params):
                removed.append(item)
                continue
        kept.append(item)
    if removed:
        config.hook.pytest_deselected(items=removed)
        items[:] = kept


# test_foo.py
import pytest

def uncollect_if(x, y, z):
    if x > 5:
        return True

@pytest.mark.uncollect_if(func=uncollect_if)
@pytest.mark.parametrize('x', range(10))
@pytest.mark.parametrize('y', range(10, 100, 10))
@pytest.mark.parametrize('z', range(1000, 1500, 100))
def test_foo(x, y, z, tmpdir):
    pass

This produces:

 λ pytest .tmp\uncollect\ -q
.............................................................. [ 22%]
.............................................................. [ 45%]
.............................................................. [ 68%]
.............................................................. [ 91%]
......................                                         [100%]
270 passed, 180 deselected in 1.12s

This can also easily be turned into a plugin.

nicoddemus avatar Dec 18 '19 17:12 nicoddemus

@nicoddemus Thanks for the demo, that looks cool! I think a plugin would be good, or even better: a built-in feature of fixtures. I'm saying this because otherwise, it would be much harder to get this into other projects (like numpy/pandas etc.), where the appetite for more plugins etc. is very low.

h-vetinari avatar Dec 19 '19 06:12 h-vetinari

A built-in feature is not out of question, but I'm personally not very keen on adding this to the core myself, as it is a very uncommon use case and a plugin does the job nicely.

Note also that if a project for some reason doesn't want to add a plugin as dependency (and a test dependency at that, so I think it should not have as much friction as adding a new dependency to the project itself), it can always copy that code into their conftest.py file.

We can definitely thought add the example above to the official documentation as an example of customization. I would be happy to review/merge a PR to that effect. 😁

nicoddemus avatar Dec 19 '19 12:12 nicoddemus

@nicoddemus : It would be convenient if the metafunc.parametrize function would cause the test not to be generated if the argvalues parameter is an empty list, because logically if your parametrization is empty there should be no test run.

Currently though if metafunc.parametrize(...,argvalues=[],...) is called in the pytest_generate_tests(metafunc) hook it will mark the test as ignored.

SaturnIC avatar Jun 11 '20 12:06 SaturnIC

Hi @SaturnIC,

Not sure, users might generate an empty parameter set without realizing it (a bug in a function which produces the parameters for example), which would then make pytest simply not report anything regarding that test, as if it didn't exist; this will probably generate some confusion until the user can figure out the problem.

I think adding a mark like we do today is a good trade-off: at least the user will have some feedback about the skipped test, and see the reason in the summary.

nicoddemus avatar Jun 11 '20 12:06 nicoddemus

we dont mark a test ignored, we mark it skip or xfail with a given reason

RonnyPfannschmidt avatar Jun 11 '20 20:06 RonnyPfannschmidt

Hi @nicoddemus,

thanks for the fast reply. I think it can not be the responsibility of pytest to prevent users from misusing functions against their specification, this would surely be an endless task.

Is there another good reason why an empty argvalues list should mark the test as skip (thanks @RonnyPfannschmidt) instead of not running it at all ?

SaturnIC avatar Jun 12 '20 06:06 SaturnIC

Off hand I am not aware of any good reason to ignore instead of skip /xfail

RonnyPfannschmidt avatar Jun 12 '20 07:06 RonnyPfannschmidt

Off hand I am not aware of any good reason to ignore instead of skip /xfail

Some good reasons (I'm biased, I'll admit) have come up in this very thread.

In short: Having lots of parametrisation, but not polluting (skip-)log with tests that represent impossible parameter combinations.

h-vetinari avatar Jun 12 '20 09:06 h-vetinari

I apologise, I should have been more specific.

there are good reasons to deselect impossible combinations, this should be done as deselect at modifyitems time

The skip/xfail for a actually empty matrix seems still absolutely necessary for parameterization.

The missing capability of fixtures at modifyitems time gives this unnecessary hardship

RonnyPfannschmidt avatar Jun 12 '20 10:06 RonnyPfannschmidt