@RonnyPfannschmidt please review 😃

@RonnyPfannschmidt

it sidesteps setup/teardown minimizing and im not sure if its feasible to integrate in a comprehensible manner

Are you wondering if fixing #14095 is an overall positive change? If so, I can remove that part from the PR for now, and it can be investigated later separately.

There is also a minor issue with my implementation. When I do

request._execute_fixturedef(fixturedef, only_maintain_cache=True)

it may finish that fixture, but when we setup the current fixture, we may discover that the control flow in the fixture body changed, and we no longer need the fixture this time.

I can change it so that the other fixtures are not finish-ed unnecessarily. We basically just check recursively whether params are OK for dependencies.

its unclear to me what happens if the involved fixtures are indirect to a test

What do you mean? Parametrized / non-parametrized case? In the parametrized case, we cannot get the parametrized fixture using getfixturevalue (pytest will not be able to resolve the test name), but I can add a test that requests the dependent fixture via getfixturevalue, it should be fine.

Its unclear to me what happens if someone creates a cycle via the new dependency tracking

The initial cache handling step uses only_maintain_cache=True to avoid bogus dependencies. I believe that _get_active_fixturedef (the part that got moved into _find_fixturedef) will detect the cycle before the dependency is cached anywhere.

There is test_detect_recursive_dependency_error for a recursive dependency between fixtures. I will write a simple test for recursive dependency using getfixturevalue.

trigger a rebuild is more correct than not
however there should be different test reordering as just correctly triggering teardown makes the testrun a bit more expensive

with indirect i mean what happens if a replaced fixture is not a dependency of the directly used fixtures, but rather down in the dependency tree

# untested quickly written example
        import pytest

        @pytest.fixture(scope="session")
        def foo():
            return "outer"

        @pytest.fixture(scope="session")
        def bar(foo):
            return f"dependent_{foo}"

        @fixture(scope="session):
        def baz(bar): 
            return bar


        def test_before_class(baz):
            assert bar == "dependent_outer"

        class TestOverride:
            @pytest.fixture(scope="session")
            def foo(self):
                return "inner"

            def test_in_class(self, baz):
                assert bar == "dependent_inner"

        def test_after_class(baz):
            assert bar == "dependent_outer"

with loops i mean what actually happens when one fixture goads another ones closure into calling getfixturevalue for itself

# untested quickly written example
@fixture
def trickster(request):
   return request.getfixturevalue
   
 @fixture
 def evil_recurse(trickster):
     return partial(trickster, "evil_recurse")
     
@fixture 
def trigger(evil_recurse):
    evil_recurse()

I've found another issue in my implementation. Suppose we depend on some fixture dynamically. After a recomputation we stop depending on it. Everything is fine, but at some point that fixture gets finalized, and because we did addfinalizer(self.finish) on it, we get finalized out of blue, perhaps in the middle of a test.

One solution is to add "evaluation epoch" to the finalizers to avoid finalizing a more recent evaluation of the fixture than expected.

teardown should idieally only happen in teardown phases - thats why the teardown_towards heleprs got added

we should fail rather than teardown in setup/call phases

I've moved parametrized fixture finalization to teardown phase. In FixtureDef.execute: find the next item that uses this fixture; if the fixture has a different param there, finalize now.

We need to be cautious with what to teardown when. Consider these tests:

@pytest.fixture(scope="session")
def foo(request):
    return request.param

@pytest.mark.parametrize("foo", [1], indirect=True)
def test_a(foo):
    pass

@pytest.mark.parametrize("foo", [1], indirect=True)
def test_b(foo):
    pass

def test_c():
    pass

@pytest.mark.parametrize("foo", [2], indirect=True)
def test_d(foo):
    pass

Finalizing foo[1] in test_c's teardown phase is not appropriate.

For each parametrized fixture in the current item we should find the next item that uses the fixture with that parameter and if parameters differ then finalize the fixture in the current item's teardown.

This requires some code restructuring, because we now require arbitrary lookahead instead of just nextitem. I've swept that under the rug with session._current_item_index for now. Tell me if you got better ideas.

Overall, setup-plan did get prettier. A hefty chunk of FixtureDef.execute evaporated.

I need to write more tests, for issues you pointed out and some more:

• Reproducer for this issue, explaining the need for _cache_epoch
• Demo for the new error in _check_cache_hit
• Check that request.param is correct in pytest_fixture_post_finalizer, explaining the need for _request_cache
• Throwing without setting cached_result in pytest_fixture_setup, possibly adding finalizers before that
• What happens to cached fixtures when a test fails with --maxfail. That's why we should not call addfinalizers on arbitrary test items ahead, they will get ignored with --maxfail
• Some edge cases in _should_finalize_in_current_item

I've reworked _current_item_index into a slightly less hacky solution. This relies on session.items containing the tests, seems reasonable, pytest_collection doc seems to require it. Perhaps there may be a better place for injecting the next items info.

More importantly, this disallows pytest_runtestloop overrides that reorder tests and insert custom items on the fly. Here are the implementations I've seen:

1. Running items in parallel in subprocesses (chunked or strided split), while keeping order - OK
2. Selecting a subset of tests to run by config options (BTW hack: should be done in collection hooks) - OK
3. Rerunning flaky tests - QUESTIONABLE
   • Running tests until the end (nextitem=None), then rerunning flaky tests while keeping the order - OK
   • Rerunning flaky tests immediately - OK (might teardown-setup more fixtures than necessary)
   • Running tests in parallel in subprocesses (until nextitem=None), collecting flaky tests per-subprocess, then running flaky tests while keeping the order - OK
   • Running tests in parallel in subprocesses, collecting flaky tests in the order they failed, then running them together - NOT OK
4. pytest-xdist - looks OK

The implementations of pytest_runtestloop I've seen will work. But some plugin somewhere may be broken by those changes. We should probably document them.

Alternatively, we could avoid all of those troubles by dropping support for carrying fixtures along tests that do not depend on them:

@pytest.fixture(scope="session")
def foo(request):
    return request.param

@pytest.parametrize("foo", [1], indirect=True)
def test_a(foo):
    pass

def test_b():
    assert 2 + 2 == 4

@pytest.parametrize("foo", [1], indirect=True)
def test_c(foo):
    pass

In this case, pytest auto-reorders fixtures based on param values (if reordering is not turned off by the user), but in a more complex scenario additional teardown-setup work will surface:

@pytest.fixture(scope="session")
def foo():
    pass

@pytest.fixture(scope="session")
def bar():
    pass

@pytest.fixture(scope="session")
def baz():
    pass

@pytest.parametrize(("foo", "bar"), [1, 1], indirect=True)
def test_a(foo, bar):
    pass

@pytest.parametrize(("bar, "baz"), [1, 1], indirect=True)
def test_b(bar, baz):
    pass

@pytest.parametrize(("baz, "foo"), [1, 1], indirect=True)
def test_c(bar, baz):
    pass

Currently all those fixtures are carried over. They will not be carried over.

And so I come back to the "disgusting" idea of tearing down parametrized fixtures in the test before the parameter changes. So in this test setup, foo will be torn down in test_c, even if foo has nothing to do with test_c. I will go on with this route.

Done!

I should add the following tests (besides the ones mentioned earlier in one, two):

• When exactly parametrized fixture teardown happens (the "ugly" situation described in this comment) - more of a golden test
• Reordering tests in pytest_runtestloop such that the index caching I suggested previously would lead to stale values of parametrized fixtures (and undetectable using nextitem)
• What happens if pytest_fixture_post_finalizer throws
• pytest_fixture_post_finalizer is no longer called multiple times for the same cache teardown
• Golden test for the order of fixture teardown (in the PR, non-function-scoped parametrized fixtures are finalized between function and class scope)

By the way, this PR also relates to:

• Refactor fixture finalization #4871 - pytest_fixture_post_finalizer are no longer called extra times
• Pytest does not support indirect parameters for overriden fixtures #2043 - no longer trying to execute fixturedefs that are hidden in the current item
• run class scoped parametrized fixture's dependency fixture's teardown after each test class iteration #9427 - is a duplicate of Fixtures are not rebuilt when param changes for a fixture they depend on, if the dependency is via getfixturevalue #14103, and vice versa