pytest-dev / pytest

The pytest framework makes it easy to write small tests, yet scales to support complex functional testing
https://pytest.org
MIT License
11.93k stars 2.65k forks source link

pytest_generate_tests cannot modify fixtures of parametrized tests #4017

Open QuLogic opened 6 years ago

QuLogic commented 6 years ago

In Matplotlib, we have a general autouse fixture that may or may not load a backend (i.e., UI toolkit) that causes a fatal error (if it doesn't exist), depending on test markers. In an effort to avoid that, I tried to add a fixture that skips tests that might use an unavailable backend. Because file-specific fixtures run after those in conftest.py, I used pytest_generate_tests to implement it, but this no longer seems to work on parametrized tests.

For the sake of an example, start with an autouse fixture that always fails in conftest.py:

# conftest.py
import pytest

@pytest.fixture(autouse=True)
def settings(request):
    print('settings', request.node.fixturenames)
    raise RuntimeError("This should not be called.")

then in test_foo.py, a fixture that should skip the test, inserted first via pytest_generate_tests so that it occurs before the above exception:

# test_foo.py
import pytest

def pytest_generate_tests(metafunc):
    print('pytest_generate_tests')
    metafunc.fixturenames.insert(0, 'should_skip')

@pytest.fixture
def should_skip():
    print('should_skip')
    pytest.skip('Should be skipped')

def test_foo():
    print('test_foo')
    assert True

This works for test_foo; should_skip is called first and the remaining fixtures are skipped.

If I then add a parametrized test:

@pytest.mark.parametrize('bar', [1, 2, 3])
def test_bar(bar):
    print('test_bar', bar)
    assert True

all of these call settings first instead of should_skip. The printout shows that should_skip is no longer in the list of fixtures either.

This used to work in pytest 3.6.3, but fails in 3.7.0. I bisected the issue back to #3629, but I'm not sure exactly if that intended for this behaviour to change.

Hopefully all the relevant info: ``` $ python --version Python 3.6.6 $ pip list Package Version Location ------------------------------- ----------------------------- --------------------------------- execnet 1.5.0 py 1.6.0 pytest 3.6.3.dev34+g1dc5e97a pytest-cov 2.6.0 pytest-faulthandler 1.5.0 pytest-flake8 1.0.2 pytest-forked 0.2 pytest-rerunfailures 4.1 pytest-xdist 1.23.0 ```
QuLogic commented 6 years ago

I also tried specifying should_skip as a parameter of the tests; then it doesn't get removed, but it still isn't run first.

RonnyPfannschmidt commented 6 years ago

this is a sife-effect of introducing the definition note to get access to markers in metafunc -now you suddenly no longer get sway with mutating random state

from my pov this is a unplanned api change that we might want to document, but certainly not revert

since you use-case isnt clear

for whole modules you can just do pytestmark = pytest.skip(...)

QuLogic commented 6 years ago

I'm aware of pytestmark, but it is not suitable in this case. In fact, that is essentially what we do right now (except using importorskip and/or skip based on some conditions, plus setup some other things, which is why I preferred a fixture).

The problem is that importing Qt4 stops importing from Qt5 and vice versa. So if there's a module level importorskip, then depending on the order the files are loaded, one or the other will always be skipped. And the user cannot choose with -k because the choice was made during collection. You can see what I am attempting at matplotlib/matplotlib#12163.

RonnyPfannschmidt commented 6 years ago

in that case i would propose using a opt-in skip at the module level (you can do a full module skip) as a starting point

to see to a good backend structure things might take a bit longer

if you do have backend abstraction, then ideally you use it for tests instead of having one file per backend (that way backend selection cna be done at the test strart)

however such an "pure" concept might simply not fit practical application in the code as it is right now

QuLogic commented 6 years ago

Looking at this example, I think adding something like this to test_foo.py:

@pytest.fixture(autouse=True)
def settings(should_skip, settings):
    pass

works to tweak the order. Is that something that might be stable?

RonnyPfannschmidt commented 6 years ago

its not clear to me what fits well as i currently don't have the time to gather all the context needed to read the mathplotlib in a understanding manner

nicoddemus commented 6 years ago

Is that something that might be stable?

Yes, should_skip will be instantiated before the other settings in that example. I believe this is stable and we would not change this lightly.

RonnyPfannschmidt commented 5 years ago

This one is related tto making ffunctiondefinition part of the collection ttree

Zac-HD commented 5 years ago

Follow-up to #2522 then?

RonnyPfannschmidt commented 5 years ago

Yup

nicoddemus commented 5 years ago

(I added this to the "detangle node structure" project)