function-scoped fixture run before session-scoped fixture

[brianmaissy]

This is a possible regression of #2405, or possibly a previously unconsidered edge case.

I have a case where a conftest in a subdirectory overrides a fixture with the same name from the enclosing directory's conftest, and this fact causes the fixtures to be set up in the wrong order (a function-scoped fixture is run before a session-scoped fixture).

Here's the example:

conftest.py
subdirectory/
    conftest.py
    test_it.py

The root conftest:

@pytest.fixture(scope='session')
def fixture_one():
    print('setting up fixture one (session-scoped)')


@pytest.fixture(autouse=True)
def fixture_two():
    print('setting up fixture two (function-scoped autouse)')
    yield
    print('cleaning up fixture two (function-scoped autouse)')


@pytest.fixture()
def fixture_three(fixture_one):
    print('setting up fixture three (function-scoped)')

The subdirectory conftest:

# overrides the original fixture_three, and is also dependent on it
@pytest.fixture()
def fixture_three(fixture_three):
    print('setting up overridden fixture three (function-scoped)')

test_it.py:

def test_it(fixture_three):
    pass

The output:

$ py.test -s
============================= test session starts ==============================
platform linux -- Python 3.6.7, pytest-4.5.0, py-1.7.0, pluggy-0.11.0
rootdir: /mnt/hgfs/test
collected 1 item                                                               

subdirectory/test_it.py
setting up fixture two (function-scoped autouse)
setting up fixture one (session-scoped)
setting up fixture three (function-scoped)
setting up overridden fixture three (function-scoped)
.cleaning up fixture two (function-scoped autouse)

As you can see, fixture_two (the autouse function-scoped fixture) is run before fixture_one, which is the session-scoped dependency of the original fixture_three (and an indirect dependency of the overridden fixture_three).

Renaming the second fixture_three solves the problem.

[brianmaissy]

From what I was able to understand from the code changed in #3306, the problem is that in getfixtureclosure the fixtures are referred to only by name, not by baseid, therefore we don't have any way to differentiate between the two fixtures named fixture_three

[brianmaissy]

It looks like in order to solve it, a significant amount of code would need to be modified to refer to fixtures with a more fully-qualified name. @RonnyPfannschmidt @nicoddemus based on the git blame you seem to be the ones with the most intimate knowledge of this code. Any insight?

[RonnyPfannschmidt]

nope, sorry

[samueljsb]

I would always want a redefined fixture in a subdirectory to overwrite the one from the parent directory. If, for example, a test in subdirectory/test_it.py wanted to use a fixture called fixture_three, which fixture would it get?
If I want to call a fixture and modify its output I should create a fixture with a new name (otherwise the order of my fixture definitions in subdirectory/conftest.py would determine which fixtures were available as I defined new ones? (e.g. a fixture_zero defined at the top of the file would have access to the session-scoped fixture_three from the root directory, but a fixture_four defined at the bottom wouldn't?)

[bluetech]

Noticed this too while reviewing #11243. As @brianmaissy said, the problem is here:

pytest/src/_pytest/fixtures.py

Line 1558 in 9c8937b

merge(fixturedefs[-1].argnames)

Just using -1 means it doesn't work right for fixture overrides.

A simpler reproduction is:

import pytest

@pytest.fixture(scope="module")
def whoops(): print("WHOOPS")

@pytest.fixture(scope="module")
def fix(whoops): print("MOD")

class Test:
    @pytest.fixture(scope="function")
    def fix(self, fix): print("FUNC")

    def test(self, fix): pass

The test runs all fixtures, so the expected fixture closure is ["fix", "whoops"], but the actual is ["fix"]. This is because getfixtureclosure only looks at fixturedefs[-1], i.e. fixturedefs[1], which is the function-scoped fix, and ignores the class-scoped fix which is at fixturedefs[0].

I guess getfixtureclosure needs to handle this properly like _getnextfixturedef does. Specifically, if fixturedefs[i] (transitively) requests its own name, we should then include fixturedefs[i - 1] in the closure. But I haven't looked too closely yet.

[jgersti]

I reported a bug in #11350 where this results in a hard error instead of just a wrong ordering