Skip to content

pytest.xfail should work like pytest.mark.xfail #7071

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

Closed
lucianmarin opened this issue Apr 10, 2020 · 16 comments
Closed

pytest.xfail should work like pytest.mark.xfail #7071

lucianmarin opened this issue Apr 10, 2020 · 16 comments
Labels

Comments

@lucianmarin
Copy link

lucianmarin commented Apr 10, 2020

pytest.xfail should do exactly what pytest.mark.xfail does. pytest.xfail acts like pytest.skip for no good reason. pytest.xfail should let the test run then mark it as XFAIL or XPASS.

The current pytest.xfail already exists as pytest.fail and new delayed pytest.xfail should developed in place.

#7060
#810

@lucianmarin lucianmarin changed the title pytest.xfail fails pytest.xfail should work like pytest.mark.xfail Apr 13, 2020
@seshendras
Copy link

This is valid issue impeding my efforts as well. With a call to pytest.xfail function, I do not see the test being executed at all. The test is just skipped, which is not the intended behavior after a pytest.xfail call.

@rhshadrach
Copy link

This can be accomplished via the request fixture. I don't know why pytest.xfail doesn't do the same, but I also don't understand pytest internals. Minimal example:

@pytest.mark.parametrize("arg", [1, 2, 3, 4])
def test_foo(request, arg):
    if arg == 2 or arg == 3:
        request.applymarker(pytest.mark.xfail)
    assert arg in (1, 2)

1 passes, 2 fails (xpass strict), 3 xfails, and 4 fails.

@RonnyPfannschmidt
Copy link
Member

imperative xfail is intended for any place in a test that discovers i can't continue and its okish
, lets indicate a expected failure

it intentionally mirrors the imperative skip

there is no plan to change its meaning

so far i haven't seen anyone demonstrate a example use-case for a correct delayed one

@rhshadrach
Copy link

Thanks for the remarks @RonnyPfannschmidt

it intentionally mirrors the imperative skip

Understood, but imperative skip also mirrors mark.skip, which is not the case for xfail. I think that's what users might find surprising. Just to be clear, while I personally think the pytest.xfail behavior is somewhat unexpected, I am more than happy that the functionality exists via request.

so far i haven't seen anyone demonstrate a example use-case for a correct delayed one

In pandas we have a parameterized fixture that produces e.g. all of our transformation kernels for groupby. This fixture is used by a variety of tests for which some kernels may not work, depending on the test. We would like our CI to fail if these tests start working in such cases. While we were using pytest.xfail, I am working on switching this over to request.applymarker. If there is a better option, I would certainly welcome it.

@RonnyPfannschmidt
Copy link
Member

Pytest.params can be used to bind marks to specific parameters, for more details please link the code implementing your particular use case

@rhshadrach
Copy link

Thanks @RonnyPfannschmidt! Here is an example from pandas:

https://github.com/pandas-dev/pandas/blob/1db3aa2b8ae8bd3b20dea14b92e1e7355bfa266a/pandas/tests/groupby/transform/test_transform.py#L336-L369

But if it's easier to grok, here is an abstracted minimal example:

from pandas.core.groupby.base import transformation_kernels

@pytest.fixture(params=transformation_kernels)  # e.g. ["cummax", "cummin", "cumsum"]
def transformation_kernel(request):
    return request.param


def test_foo(request, transformation_kernel):
    if transformation_kernel == "cummax":
        request.applymarker(pytest.mark.xfail(reason="cummax has a bug"))
    assert transformation_kernel != "cummax"


def test_bar(request, transformation_kernel):
    if transformation_kernel == "cummin":
        request.applymarker(pytest.mark.xfail(reason="cummin has a bug"))
    assert transformation_kernel != "cummin"

We have tests to ensure that the list transformation_kernels is kept up to date. When this list is updated, all of our tests using the transformation_kernel fixture thus automatically include the new kernel.

@RonnyPfannschmidt
Copy link
Member

@rhshadrach so if we had
@pytest.mark.xfail(lambda: transformation_kernel: transformation_kernel == "cumax", reason = "cumax bug #abc") your use case would resolve?

We have a plan for that, but im not going to search the issue for that tonight

@rhshadrach
Copy link

rhshadrach commented Jan 2, 2021

Issue is #7395; assuming that can handle multiple fixtures and a combination of fixtures and @pytest.mark.parametrize then yes, absolutely.

Would I be correct in saying that your view is that marking using a decorator is preferred because then the expectations are set during test collection rather than test execution?

@RonnyPfannschmidt
Copy link
Member

Thanks for looking it up

I Strongly prefer the declarative way as it implies the Metadata is available after collection

@seshendras
Copy link

imperative xfail is intended for any place in a test that discovers i can't continue and its okish
, lets indicate a expected failure

it intentionally mirrors the imperative skip

there is no plan to change its meaning

so far i haven't seen anyone demonstrate a example use-case for a correct delayed one

I am unable to use @pytest.mark.xfail decorator in two scenarios:

  • stacked parameters: when the I have stacked/multiple levels of @pytest.mark.parametrize( ), that generate combinations running over a hundred. In such situations, one must be able to selectively mark tests as failed
  • many sources: when I have to glean into output from a fixture(s) and also test parameters

I am using pytest.skip function in obvious situations that are not supported, and pytest.xfail function where test is expected to fail due to a transient issue(buggy feature, partially built feature, ...) or a situation where one must ensure that test fails for the right reason and not cause any adverse impact. The essence is to be able to use pytest.xfail function in similar situations as one would apply @pytest.mark.xfail decorator.

@pytest.mark.parametrize("pool_size, pool_stride", [(2, 1), (3, 1), (3, 2), (3, 3),
        (2, 2), (2, 3)], ids=["2P-1st", "3P-1st", "3P-2st", "3P-3st", "2P-2st", "2P-3st",])
@pytest.mark.parametrize("in_dim_w", [32, 80, 128, 210, 300],
        ids=["32W", "80W", "128W", "210W", "300W"])
def test_sepconv_xxx(available_backend, rnd_input, rnd_weights, in_dim_w, pool_size, pool_stride):

    if available_backend ==  BackendType.Software:
        if version == 1 and (in_dim_w == 300 and not (pool_size and pool_stride == 3)):
            pytest.xfail('Open bug xyz: in software version 1')
        elif version == 2 and (pool_size == 2 and pool_stride == 3):
            pytest.skip('Not supported in software version 2')
        elif rnd_input < 0 and pool_size == 3:
            pytest.xfail('this combo is yet to be supported')

It has been a real problem for us by not having the capability to run the tests fully and mark it as expected fail. This has caused certain real issues go unnoticed that would have been caught sooner.

PS: Tried my best to illustrate the use case :-)

@seshendras
Copy link

This can be accomplished via the request fixture. I don't know why pytest.xfail doesn't do the same, but I also don't understand pytest internals. Minimal example:

@pytest.mark.parametrize("arg", [1, 2, 3, 4])
def test_foo(request, arg):
    if arg == 2 or arg == 3:
        request.applymarker(pytest.mark.xfail)
    assert arg in (1, 2)

1 passes, 2 fails (xpass strict), 3 xfails, and 4 fails.

This certainly helps. Thanks a lot.

@nicoddemus
Copy link
Member

This certainly helps. Thanks a lot.

pytest.skip and pytest.xfail both work by raising an exception, and changing them at this point I believe is a no-go, as I'm sure they also have their use cases which would break.

Adding the request.applymarker example to the docs could help then?

@seshendras
Copy link

This certainly helps. Thanks a lot.

pytest.skip and pytest.xfail both work by raising an exception, and changing them at this point I believe is a no-go, as I'm sure they also have their use cases which would break.

Adding the request.applymarker example to the docs could help then?

Certainly! That should help, anyone looking for this functionality.

My observation, 'pytest.skip' and 'pytest.xfail' functions both seem to do the same thing(wrt functionality) at the moment, unlike their respective decorator versions.

Thanks

@nicoddemus
Copy link
Member

My observation, 'pytest.skip' and 'pytest.xfail' functions both seem to do the same thing(wrt functionality) at the moment, unlike their respective decorator versions.

Not sure I follow, can you clarify that? The decorated versions produce different outcomes in the terminal (skipped vs failed tests).

@RonnyPfannschmidt
Copy link
Member

@nicoddemus i believe the issue is taken with xfail having run=True as default which means code runs, and xpass is possible

@nicoddemus
Copy link
Member

Ahh right, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants