fix overridden/extended fixtures

[andresliszt]

This PR aims to solve this issue #12091. In the issue example shown below, fixturedefs defined here was returning a tuple of the two versions of the overridden main fixture. The dependency on param was in the first element of the tuple, so doing fixture_defs[-1] would never find that dependency due to name duplication. The change is quite simple, checks in the tuple all fixturedefs whose argname is contained in fixture_defs[-1].argnames

import pytest

@pytest.fixture
def param() -> int:
    return 1

@pytest.fixture
def main(param: int) -> int:
    return sum(range(param + 1))


class TestFoo:
    @pytest.fixture
    def main(self, main: int) -> int:
        return main

    @pytest.mark.parametrize("param", [2])
    def test_foo(self, main: int) -> None:
        assert main == 3

[bluetech]

Thanks for the PR @andresliszt.

As currently implemented, this will potentially add too many names to the closure (where previously it would potentially miss some). This can happen if we have e.g. 3 levels of overrides (e.g. conftest, module, class), where -1 uses -2 but -2 doesn't use -3, and -3 has some extra dependencies. The current code adds -3's dependencies, but it shouldn't.

[andresliszt]

Thanks for the PR @andresliszt.

As currently implemented, this will potentially add too many names to the closure (where previously it would potentially miss some). This can happen if we have e.g. 3 levels of overrides (e.g. conftest, module, class), where -1 uses -2 but -2 doesn't use -3, and -3 has some extra dependencies. The current code adds -3's dependencies, but it shouldn't.

Hey hello!, i think is handled, all additions are made with respect to -1. If -2 doesn't use -3, it will never be considered. I updated the tests with your example.

[bluetech]

Hey hello!, i think is handled, all additions are made with respect to -1. If -2 doesn't use -3, it will never be considered. I updated the tests with your example.

In the code as written it is added -- it is iterating over all fixturedefs and adding all of their argnames. You can verify by putting a breakpoint and seeing that the closure includes foo.

The reason foo doesn't get executed in the end is that it's pruned by the prune_dependency_tree function, which is also buggy in that it only considers -1. Sometimes two wrongs make a right :)

[andresliszt]

Hey hello!, i think is handled, all additions are made with respect to -1. If -2 doesn't use -3, it will never be considered. I updated the tests with your example.

In the code as written it is added -- it is iterating over all fixturedefs and adding all of their argnames. You can verify by putting a breakpoint and seeing that the closure includes foo.

The reason foo doesn't get executed in the end is that it's pruned by the prune_dependency_tree function, which is also buggy in that it only considers -1. Sometimes two wrongs make a right :)

Thanks for your answer i really like to know more about this amazing project!. The last question I have is this, the inner function i created is called with the result of this

fixturedefs = self.getfixturedefs(argname, parentnode)
>>> fixturedefs
(<FixtureDef argname='main' scope='function' baseid='example/test.py'>, <FixtureDef argname='main' scope='function' baseid='example/test.py::TestFoo'>)

As i understand the function self.getfixturedefs gives us the fixtures that the node depends on, in this case the two main
defined at class and module level (the one defined in the conftest is not there). At the end of the function getfixtureclosure i get the clousure ['main', 'param']. Where do references to conftest main/foo happen?

it is iterating over all fixturedefs and adding all of their argnames.

is it iterarting over all fixturedefs or iteraring over all fixturedefs of the current node?

[bluetech]

As i understand the function self.getfixturedefs gives us the fixtures that the node depends on, in this case the two main
defined at class and module level (the one defined in the conftest is not there).

It gives the fixtures that the node may depend on, so it returns the 3 mains. The reason for this is that a test may use request.getfixturevalue to dynamically request any applicable (visible) fixture.

At the end of the function getfixtureclosure i get the clousure ['main', 'param']. Where do references to conftest main/foo happen?

I checked it myself now (with your revised test), and I see foo is also returned. How are you checking it?

is it iterarting over all fixturedefs or iteraring over all fixturedefs of the current node?

The function is this:

        def dependent_fixtures_argnames(
            fixture_defs: Sequence[FixtureDef[Any]],
        ) -> List[str]:
            last_fixture = fixture_defs[-1]
            # Initialize with the argnames of the last fixture
            dependent_argnames = list(last_fixture.argnames)
            for arg in fixture_defs:
                if arg.argname in last_fixture.argnames:
                    # Add new argument names maintaining order and avoiding duplicates
                    for argname in arg.argnames:
                        if argname not in dependent_argnames:
                            dependent_argnames.append(argname)
            return dependent_argnames

Where fixture_defs is [main in conftest, main in module, main in class].
last_fixture is main in class.
You start with list(last_fixture.argnames) which is param and main.
You then iterate over all 3 fixture defs.
For all fixture defs the arg.argname is main so the arg.argname in last_fixture.argnames condition passes for all of them.
You then add all of arg.argnames to the results. For main in conftest, this includes foo.

[andresliszt]

You are absolutely right @bluetech ! In my example i forgot to add foo dependency on conftest.main, now I fixed my example and foo is being added!. Thanks

[bluetech]

Thanks for the update. Now there is a different problem. The code handles the case of test -> foo -> foo -> bar (correctly including bar), but doesn't handle test -> foo -> baz -> foo -> bar (incorrectly doesn't include bar).