Skip to content
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

Document that mark.usefixtures does not work on fixtures #1014

Closed
ulope opened this issue Sep 16, 2015 · 10 comments
Closed

Document that mark.usefixtures does not work on fixtures #1014

ulope opened this issue Sep 16, 2015 · 10 comments
Labels
type: enhancement new feature or API change, should be merged into features branch

Comments

@ulope
Copy link
Contributor

ulope commented Sep 16, 2015

I just ran into #378 in a test suite with complex fixture dependencies and it took me quite a while to figure out that usefixtures had no effect when used on fixtures themselves. It would be great if this fact was mentioned in the docs.

@The-Compiler
Copy link
Member

This was already documented in #977 - where would you have expected the documentation?

@ulope
Copy link
Contributor Author

ulope commented Sep 16, 2015

Ah, the problem I see with that is that people probably don't search for mark when they want information on a specific one.

The first hit on google when searching for "pytest usefixtures" leads to https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects. Maybe adding a link to the mark documentation (e.g. "more information on marks and where they're applicable" or some such) would be enough.

rcorre added a commit to rcorre/qutebrowser that referenced this issue Jul 10, 2016
Running test_standarddir would pollute the user's home with
`~/.cache/qute_test`.

The `no_cachedir_tag` fixture was supposed to prevent this, but was not
working because [usefixtures does not work on fixtures]
(pytest-dev/pytest#1014).

This fixes the fixture to actually prevent cachedir creation, but
applies it to tests individually (or by class) rather than with autouse
because the cachedir tests cannot pass if it is working.
rcorre added a commit to rcorre/qutebrowser that referenced this issue Jul 12, 2016
Running test_standarddir would pollute the user's home with
`~/.cache/qute_test`.

The `no_cachedir_tag` fixture was supposed to prevent this, but was not
working because [usefixtures does not work on fixtures]
(pytest-dev/pytest#1014).

This fixes the fixture to actually prevent cachedir creation, but
applies it to tests individually (or by class) rather than with autouse
because the cachedir tests cannot pass if it is working.
rcorre added a commit to rcorre/qutebrowser that referenced this issue Jul 15, 2016
Running test_standarddir would pollute the user's home with
`~/.cache/qute_test`.

The `no_cachedir_tag` fixture was supposed to prevent this, but was not
working because [usefixtures does not work on fixtures]
(pytest-dev/pytest#1014).

This fixes the fixture to actually prevent cachedir creation, but
applies it to tests individually (or by class) rather than with autouse
because the cachedir tests cannot pass if it is working.
@tlandschoff-scale
Copy link

I just spent hours on this while refactoring our test suite to reduce duplication between our fixtures (and especially overriding fixtures in different packages). Not knowing that this is a known bug missing feature, I wrote an example to illustrate this:

import pytest

@pytest.fixture
def foo():
    return {"foo": 42}

@pytest.fixture
def bar(foo):
    bar = range(3)
    foo["bar"] = bar
    return bar

@pytest.fixture
@pytest.mark.usefixtures("bar")
def fixture_with_usefixtures(foo):
    return foo

@pytest.fixture
def fixture_using_fixture(foo, bar):
    # bar not used, therefore I'd rather use usefixtures
    return foo

@pytest.mark.usefixtures("bar")
def test_with_usefixtures(foo):
    assert foo["bar"]

def test_indirect_usefixtures(fixture_with_usefixtures):
    assert fixture_with_usefixtures["bar"]

def test_indirect_argument(fixture_using_fixture):
    assert fixture_using_fixture["bar"]

The setup I am testing is a bit complicated. I have a fixture for an IPC broker and another one that creates a client of this broker and attaches it to the broker. Most of the time I actually need the client object but for some tests I only need to have it attached.

Maybe that's not the best way to structure it, but I was very surprised that a fixture can use another fixture in a named parameter but not using the marker.

So my main point is that this is an inconsistency that's easy to fall into.

@nicoddemus
Copy link
Member

@tlandschoff-scale agree completely, we should raise some error when that happens, if possible.

@nicoddemus nicoddemus added the type: enhancement new feature or API change, should be merged into features branch label Feb 16, 2017
@nicoddemus
Copy link
Member

nicoddemus commented Feb 16, 2017

And not for usefixtures only, but any mark.

@tlandschoff-scale
Copy link

I agree in so far as that is much better than just ignoring the request.
But I don't see any reason why consistency should be avoided here (apart from implementation complexity of course).

You might say that there is no use case of marking a fixture. We already tried that (apart from usefixtures) as well. Basically, we have a large test suite which currently runs an hour but does a lot of integration stuff. Some tests set up an application server or even a network of them (for testing synchronization and caching features).

I actually tried to use @pytest.mark.integration on the fixture that brings up a server as this is generally an expensive operation. I bet this would be useful to other users of pytest as well.

So disallowing this for now is fine with me (as it is not implemented anyway), but I think getting it implemented later should not be definitely ruled out.

@RonnyPfannschmidt
Copy link
Member

transfering markers from fixtures should be fine, but quite franckly, with marks as they are now, there is no way

we pretty much have to break some apis to fix it

@nicoddemus
Copy link
Member

transfering markers from fixtures should be fine, but quite franckly, with marks as they are now, there is no way

I agree that transferring it is probably possible but there are quirks with marks in general, but my concern is make the rest of the code understand that now fixtures can be marked, including the case mentioned for general marks (@pytest.mark.integration) and parametrization via @pytest.mark.parametrize.

@RonnyPfannschmidt
Copy link
Member

@nicoddemus yes, this involves even more transferring markers, and that us currently on a technical base that is at best horrendous and latent broken in many places

@suntzu86
Copy link

suntzu86 commented Apr 1, 2017

I just puzzled over this for a while too. Searching docs did not come up with the above warning until I googled for "pytest mark usefixtures not working" which brought me here.

+1 for @nicoddemus 's suggestion at least in the short term. If this had failed loudly, that would have been a lot more clear than silently doing nothing, leaving me wondering how come my code is never called.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement new feature or API change, should be merged into features branch
Projects
None yet
Development

No branches or pull requests

6 participants