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

Question regarding fixture visibility #2246

Closed
TristenHayfield opened this issue Feb 14, 2017 · 10 comments
Closed

Question regarding fixture visibility #2246

TristenHayfield opened this issue Feb 14, 2017 · 10 comments
Labels
type: question general question, might be closed after 2 weeks of inactivity

Comments

@TristenHayfield
Copy link

Let's say you you have a test in package A which requires a fixture from package B. Currently one can import the fixture into the test or conftest.py and that works just fine.

However if the fixture is a modular fixture (i.e. one which depends on other fixtures), it will only work:

  • If the test imports that fixture's dependencies
  • Or, if the fixture's dependencies are plug-ins

Why doesn't pytest doesn't let modular fixtures use the import mechanism to gain access to their dependencies? My main gripes with having to use the plugin mechanism are:

  • It's not consistent with simpler use cases where imports are sufficient
  • It doesn't promote writing modular fixtures
  • It encourages one to use poor style when writing modular fixtures (i.e. implicitly relying on plugins to provide certain fixtures instead of using direct imports to explicitly state your intent)

Any thoughts about this? Or is there another way to go about this issue?

@RonnyPfannschmidt RonnyPfannschmidt added the type: question general question, might be closed after 2 weeks of inactivity label Feb 14, 2017
@RonnyPfannschmidt
Copy link
Member

fixtures can be overridden by plugin order, by making them imported dependencies a major feature would be lost (namely being able to override/switch dependencies

fixtures are a "dependency injection" system

@TristenHayfield
Copy link
Author

What I'm suggesting here is really not at odds with the current system of dependency injection. What is currently in place is a hierarchy of scopes used to resolve fixture requests. From the pytest documentation, the current hierarchy is:

The discovery of fixtures functions starts at test classes, then test modules, then conftest.py files and finally builtin and third party plugins.

I'm suggesting that for a given fixture declared in some module, the search should extend beyond the final third-party scope and into the module itself.

It wouldn't break anything. One could always override it at any of the many scopes preceding it in the current hierarchy.

@nicoddemus
Copy link
Member

Hi @TristenHayfield,

Could you elaborate a bit? In your first message, you mention "package A" and "package B", and in your follow up you mention "the search should extend beyond the final third-party scope and into the module itself".

I'm not sure I understand fully what you are proposing (one example or two would be helpful).

@TristenHayfield
Copy link
Author

TristenHayfield commented Feb 14, 2017

Oops I meant the third-party plugin scope.

Assume you have some test which is trying to make use of a fixture provided by another package :

test.py:

from important_package.fixtures import important_fixture

def test_thing(important_fixture):
    pass

important_package.fixtures:

import pytest

@pytest.fixture
def another_important_fixture():
    pass

@pytest.fixture
def important_fixture(another_important_fixture):
    pass

test_thing currently errors because I didn't import another_important_fixture into test.py.
What I'm suggesting is that the final, outermost scope for resolving another_important_fixture should be important_package.fixtures since that's the module from which important_fixture, which depends on it, originates. Currently the search for fixtures stops at the plugin scope.

Does that make sense?

@RonnyPfannschmidt
Copy link
Member

nope, the error you see is intentional,

the main problem is actual importing of fixtures (that makes them seen as a new fixture at a different place)

you either have to do that for all of them or none

@TristenHayfield
Copy link
Author

I understand that the error might be intentional, but I'm suggesting that extending the search to important_package.fixtures in the above example would encourage modularity and fixture reuse (outlined in my original comment) and wouldn't conflict with the existing uses of pytest.

As it stands, the only way to use modular fixtures at scale involves using plug-ins, since it doesn't make sense for individual tests to import higher-order dependencies (i.e. the dependencies of dependencies).

@RonnyPfannschmidt
Copy link
Member

for a fixture a python import is a definition of an entirely new and different fixture so to begin with, it makes no sense to import a single fixture, as its not an import of a fixtur its a definition of a fixture

@nicoddemus
Copy link
Member

FWIW, it is easy to turn any module into a plugin. Using the example you posted all you need is:

test.py:

pytest_plugins = ['important_package.fixtures']

def test_thing(important_fixture):
    pass

Also I really like how one can define a fixture in a test module, and when it becomes useful for other test modules the fixture can just be moved to a conftest.py file and no test code need to change. If fixtures were imported explicitly, that would mean that moving a fixture I would need to change imports around when moving a fixture to higher-level in the module hierarchy for reuse. I also understand that this would still be possible under your proposal, just thought I would mention it.

Also I'm not sure it would be possible to implement this in backward compatible way. As @RonnyPfannschmidt mentioned, in the current design when you import a fixture into a module, it is exactly as if you were defining a new fixture in that scope with the same name. This is actually a common source of errors for users, so much we have tried to figure out ways to warn about it (#1511).

@TristenHayfield
Copy link
Author

TristenHayfield commented Feb 15, 2017

Thanks @RonnyPfannschmidt for the explanation and thanks @nicoddemus for the helpful example. I didn't realize that one could, in essence, baptize another module as a plugin (this is documented, but I guess I misunderstood it as only applying to plugins (i.e. modules with declared entry points)). It seems that this also recursively processes pytest_plugins in the newly baptized plugin so that higher-order dependencies can be handled that way (that is, if important_package.fixtures in your example also contains a pytest_plugins variable, that gets processed as well). This is not obvious from the documentation either - the documentation only mentions this effect for conftest.py

Imports acting as redefinitions of fixtures is most likely a common source of errors because it's not documented (at least in the fixtures documentation - the one thing that people might want to reuse between tests) and it runs contrary to the normal functioning of Python imports. I'm guessing it's being done for cleanup and isolation between tests. Anyhow, I can see a sane way forward now when it comes to sharing fixtures from other packages, so thank you.

nicoddemus added a commit to nicoddemus/pytest that referenced this issue Feb 15, 2017
@nicoddemus
Copy link
Member

Sure, glad we could help!

Opened #2252 attempting to improve the docs a little.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: question general question, might be closed after 2 weeks of inactivity
Projects
None yet
Development

No branches or pull requests

3 participants