Account for changes in fixture dependencies properly

[Anton3]

Move fixture teardown to SetupState

A fixture's teardown should first perform teardown of the dependent fixtures - this is the only known way that maintains the correct teardown order in all cases. Before this PR, this code resided in FixtureDef. The disadvantage is that the responsibilities and implementation of SetupState (including exception handling) was spread across the codebase. This PR moves the execution of fixture teardown into SetupState, which facilitates code reuse and enforces Single-Responsibility Principle.

A more important issue was that fixture teardown for parametrized fixtures was performed during the setup of the next test. This lead to incorrect attribution of fixture teardown errors, time spent and logs to the next test. This PR makes sure to precompute param changes for parametrized fixtures in advance using nextitem and always runs fixture teardown at test teardown stage.

Relevant tests:

• test_teardown_stale_fixtures_single_exception - shows correct attribution of teardown errors
• test_teardown_stale_fixtures_multiple_exceptions
• test_parametrized_fixture_carries_over_unaware_item
• test_fixture_rebuilt_when_param_appears
• test_fixture_not_rebuilt_when_not_requested
• test_cache_mismatch_error_on_sudden_getfixturevalue - as was insisted in the review comments, if we discover at setup or call stage that a fixture needs to be recomputed, we prefer to error out instead
• test_cache_key_mismatch_error_on_unexpected_param_change
• test_fixture_teardown_when_not_requested_but_param_changes
• test_fixture_teardown_with_reordered_tests
• test_parametrized_fixtures_teardown_in_reverse_setup_order
• test_parametrized_fixture_teardown_order_with_mixed_scopes
• test_override_fixture_with_new_parametrized_fixture - this is a peculiar test that works by sheer luck; we should track fixture overrides more precisely in the future

Closes: #9287

Add FinalizerHandle to avoid accumulating stale finalizers

To reiterate, adding teardown of the dependent fixture as a "finalizer" to the dependency-fixture is the only known way of achieving the fully correct teardown order. But this meant that stale finalizers from function-scoped fixtures accumulated at higher-scoped fixtures and Nodes. This manifested as a "memory leak" in very large test suites.

Besides performance concerns, with getfixturevalue dependency tracking added, a stale finalizer could result in tearing down a fixture randomly.

To solve those issues, this PR introduces FinalizerHandle that allows to remove stale finalizers, and plucks all remaining self-finalizers at fixture's teardown.

Relevant tests:

• test_stale_finalizer_not_invoked

Closes: #4871

Account for changes in getfixturevalue dependencies

Previously, for a given fixture, only dependencies from argnames were checked to see if they need to be recomputed, recomputing the current fixture. Meanwhile, if a fixture depends on a parametrized fixture via getfixturevalue, then the current fixture's value could erroneously be carried over, even though the dependency was recomputed due to its parameter change.

The fix involves tracking fixture dependencies (_own_fixture_defs) in FixtureRequest, then adding finalizers to all those fixtures in FixtureDef.execute.

This precise dependency information was also used in setuponly.py.

Relevant tests:

• test_getfixturevalue_parametrized_dependency_tracked
• test_getfixturevalue_parametrized_dependency_order
• test_request_stealing_then_getfixturevalue_on_parametrized

Closes: #14103

[Anton3]

@RonnyPfannschmidt please review 😃

[RonnyPfannschmidt]

I'm currently not able to do a deep review

This has a few details that give me headaches

• it sidesteps setup/teardown minimizing and im not sure if its feasible to integrate in a comprehensible manner
• its unclear to me what happens if the involved fixtures are indirect to a test
• Its unclear to me what happens if someone creates a cycle via the new dependency tracking

[Anton3]

@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.

[RonnyPfannschmidt]

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()

[Anton3]

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.

[RonnyPfannschmidt]

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

[Anton3]

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

[Anton3]

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.

[Anton3]

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)

[Anton3]

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

[Anton3]

@RonnyPfannschmidt what do you think is the lesser evil in this case?

import pytest

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

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

def test_b(request):
    hmmm(request)

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

1. Teardown the fixture at the end of test_a, because nextitem does not request the fixture
2. Carry the fixture over, but if test_b requests foo via getfixturevalue, then we fail, because we refuse to perform teardown at call phase

* Assume that parametrized tests autoreordering is turned off, or that the user forced a custom order, or that there are multiple parameters, and autoreordering had to pick an argname to not be consecutive, and our fixture was picked.

UPD: I will go with option 1 for now, because in most cases extra setup-teardown won't happen due to autoreordering, and worst-case scenario is tests slowdown, while option 2 risks breaking some tests in-the-wild (I'm almost certain that I'll find out at my worksite that the new version of pytest broke some of the millions of pytest tests.)

[RonnyPfannschmidt]

teardown when the next item doesnt use is worse than failure at first glance

the failure is a acceptable first step as it turns a silent error into a visible one, but the real issue is that teardown_towards should tear it down in the teardown of test_b as test_c has indeed different parameters

[Anton3]

but the real issue is that teardown_towards should tear it down in the teardown of test_b as test_c has indeed different parameters

There is no issue there:

• With approach 1, teardown of test_b will do nothing (the fixture is already torn down), and test_c will setup it.
• With approach 2, test_b will fail without finishing the fixture, while for test_c no extra teardown-setup work will happen.

The issue is only in test_a -> test_b transition.

[Anton3]

But I got the idea, you think failing explicitly is better than the current state of teardown in setup phase. Will go on with this approach.

[RonnyPfannschmidt]

the goal should be to eventually e able to do it in teardown_towards

but bascially if a fixture has to be removed in the setup phase something went wrong

i do fear that i am missing some edge cases here where the teardown is correct

but my current understanding is that the teardown ought to happen in the teardown phase of the prior test item

[Anton3]

@RonnyPfannschmidt ready for review

[Anton3]

Could anyone review this please?

[RonnyPfannschmidt]

i just did a preliminary skim of the change

the finalizer handles are a interesting way to sort the dependencies out, unfortunately it creates a very global way to sort it

i haven’t done my own die diligence yet, i hope that there’s potentially a sensible way to integrate the detail handling of this with setupstate,

after my first iteration im getting the impression it should be integrated into finalizers in a more direct manner

this is quite the tricky problem, in particular wit the current architecture
i am of the impression that a missing underlying refactoring/feature addition is making the task you set out to solve very hard

we have to be very careful about the effect of the tech debt incurred by this

[Anton3]

@RonnyPfannschmidt fixed + replied

Why removal handles are needed:

• They solve the memory leak of potentially thousands of stale finalizers in higher-scoped fixtures and nodes, as per Refactor fixture finalization #4871
• They prevent the issue I described in test_stale_finalizer_not_invoked

In #4871 they also suggest that ideally we should avoid fixture finalizers and instead plan fixture setup-teardown at collection stage. This is impossible, because, as explained here, custom implementations of pytest_runtestloop can reorder, remove and duplicate tests last second before running them.

---

An alternative implementation without finalizers would be the following:

1. For each FixtureDef, collect a set of dependents
2. At teardown stage, first compute a set of what FixtureDefs need to be recomputed on their own
3. Then take a transitive closure of the set using (1)

---

Still, I can't see a non-ad-hoc way of avoiding the pile-up of finalizers of parametrized fixtures in nodes. Removal handles as currently in the PR are the most straightforward solution that I see.

A solution for nodes without handles would be to remember the node for which we have a finalizer currently registered and not readd it on fixture recomputation. The drawback would be incorrect finalizer ordering:

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

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

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

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

The order of fixture teardown on module exit will be: bar, foo - as if the order of registration was foo, bar.

But the instance of foo that was first is gone, and the current instance of foo was setup after bar. So the order of fixture teardown should be foo, bar.

AFAICS the only way the correct order can be achieved is by removing the finalizers of foo at its teardown.

[RonnyPfannschmidt]

ok, i need to reiterate the handles idea a bit more fir my own understaniding

i think the fact that its currently a global variable is part of my confusion

ideally the complete details on caching/recreation and current fixture closure turn into the responsibility of the setup state
currently they are all over the place

but that is sooo out of scope for this change

i do believe having the handles as one variable may create possibly neglectable issues wit in process pytest runs for testing pytest/pytest plugins - off hand it looks to me as if we could leave handles in place by accident if the pytest under test gets really broken - i suspect that this could be resolved by having a weakref.WeakKeyDictionary which would automatically clean up

[Anton3]

Alternatively, there is the following compromise:

• For inter-fixturedef dependencies, use the solution from the previous comment without finalizers and handles
• For fixturedef-node finalizers, allow them to pile up. As noted in Refactor fixture finalization #4871, the most troubling is that currently all those finalizers refer to separate SubRequest objects. In this PR, I move _current_request to FixtureDef, so there may be no more than 1 instance of cached SubRequest per FixtureDef

What do you think?

Edit: After thinking for a while, I still believe handles are a cleaner solution, because this approach

1. Leaves no garbage in nodes
2. Always performs teardown in reverse-setup order, as described here
3. Keeps the correct order of fixture teardown w.r.t. user finalizers

[Anton3]

I don't think weakref can be used with handles. If we drop a handle, that does not mean that we no longer want the finalizer to run, unless we want to break the current finalizers API for users. What dropping a handle currently means in the PR is that we don't want the ability to remove the finalizer.

Edit: actually, a handle could keep a weakref to FinalizerId, but that's pinching pennies.

If handles are kept past their intended scope by mistake, then finalizers are eventually invoked and cleared, and handles are left owning a bunch of FinalizerId which own nothing.

[Anton3]

@RonnyPfannschmidt I've moved fixture finalizers management to SetupState. Looks a bit cleaner now. Ready for review

[RonnyPfannschmidt]

Thanks for moving it into setupstate

That removes my biggest concern

I did another skim and now it looks solid at first glance to me

As this necessary is something big ill have to allocate some time for a deeper read

Thank you

[RonnyPfannschmidt]

this looks correct to me now

I’m tempted to suggest figuring how to make the finalizer handles a bit less opaque - to ease debugging the data (should be a followup)

i'd like a second pair of eyes on it and i think i have to run a local test

based on the current code and integration in setupstate i'm confident this is good to go

my due diligence needs some more work

thanks for getting this started and powering through the follow-ups
this was hard and important work and I’m grateful someone actually went down that rabbit hole -

[Anton3]

@RonnyPfannschmidt I've reworked the Callable into a public FinalizerHandle class. Please resolve review comments that have been fixed

[RonnyPfannschmidt]

@nicoddemus @bluetech ping on this one - i'll also try to take another looks tis week but im stretched very thin amt

[Anton3]

I've skimmed through fixture-related issues updated in the last 2 years:

• runner: re-raise pytest.exit inside ExceptionGroups (fixes #13650) #13736

  This PR fixes ignoring pytest.exit via wrapping in ExceptionGroup. Probably need to add some tests for that

  This PR only ensures that its changes don't block the other PR (current situation is not worsened). The root cause of that issue is not touched in this PR.

• Numerous issues regarding controlling fixture cache state dynamically:

  • Pytest session-scoped fixture is re-initialized unexpectedly #13755
  • Add new fixture cache_result param (default True) to ignore fixture cache and re-execute on each usage of the fixture #12814
  • Allow to dynamically create fixture in plugin #12376
  • RFC. Support for resetting function-scoped fixtures  #12596

  As long as we agree that finalizers for custom caches should be executed by SetupState, this PR should not block anything

[Anton3]

@nicoddemus @bluetech Could you please take a look at the PR?

[RonnyPfannschmidt]

i did another round of walk-trough and approve this - i'd still like another pair of critical eyes as this one is a important enhancement that introduces a new mechanism

[Anton3]

@The-Compiler Could you perhaps take a look at the PR?

[Anton3]

@nicoddemus @bluetech gentle ping?

[nicoddemus]

Sorry I won't be able to review this properly.

@RonnyPfannschmidt if you have looked at it thoroughly and are confident with the result, feel free to go ahead and merge it.

[bluetech]

I will try to review this.

If you can split the changes to smaller atomic commits (if possible) that would help a lot.

Also, if you can update the pull request summary to explain what is the problem being fixed here that would also help. Currently it lists the code changes, which don't explain the rationale, and links to issues, which I'm sure explain the problems but are a bit hard to sift through. So a clear description would help a lot.

[Anton3]

@bluetech I'm afraid the main change in fixtures.py and runner.py is more-or-less monolithic, apart from publishing FinalizerHandle. I don't believe inventing new intermediate implementations, intentionally partially incorrect, is a good idea.

I've written a detailed description. Walking through the tests and linked issues again was actually useful, I've found that #2043 is actually irrelevant to this PR.

For now, please try reading through the PR as-is and let me know if anything is unclear. One more thing I could try is adding comments throughout the PR, describing what problems are being solved. This may or may not prove to be necessary.

[bluetech]

@Anton3 I understand if it can't be broken down. Thanks for adding the description. I will try to take a look this weekend.

[Anton3]

@bluetech friendly reminder 👀

[Anton3]

@bluetech weekly ping 🤔

[bluetech]

Hey @Anton3,

Thanks for the impressive work!

I am still going through this PR.

First, I split the pytest_fixture_post_finalizer changes to #14275, please have a look at my comments there. If the changes there are merged, I will ask you to rebase here.

In the meantime please see the small comments below (still not a review of the main changes).

[bluetech]

Please add a small docstring to the non-private methods of SetupState. SetupState is not a public API, but I mean as documentation for the internal users of it.

It would also be good to discuss the fixture stuff in SetupState's docstring, it currently only discusses the setup/teardown for nodes.

[bluetech]

@Anton3 Now that #14275 is merged, can you rebase on main? Thanks.

[Anton3]

@bluetech done

[Anton3]

This rule is actually broken for parametrized fixtures. The actual order (as implemented in the PR) is:

• function-scoped fixtures
• class, module, package, session-scoped parametrized fixtures, as well as their dependencies - first in the reverse order of parametrized fixture setup, then (for dependencies of a given parametrized fixture) in the reverse order of fixture setup
• class-scoped fixtures
• module-scoped fixtures
• package-scoped fixtures
• session-scoped fixtures

That's still better than the status quo (where parametrized fixture teardown runs at the test setup stage), but needs further refinement (perhaps in follow-up PRs).

[Anton3]

Thinking about it, we can eventually come up with the perfect algorithm for SetupState, if it knows about all fixture dependencies (not wrapped in opaque finalizers).

Instead of FinalizerHandle, I'll add deeper fixture dependency info to SetupState.

[Anton3]

This sacrifices the current ordering of user finalizers w.r.t. teardown of fixtures that depend on current fixture / node.

[bluetech]

I'm still reviewing this, but just a couple of comments.

[bluetech]

I made some small comments inline, here are some high level comments.

---

First, just stating the problem in my own words a bit.

The basic problem we need to handle is the fact that a high-scope fixture can arbitrarily change its param between items (because pytest allows plugins to choose any arbitrary order of items), which necessities tearing down that fixture and its dependents "early".

By "early" I mean the following. Suppose that this param change scenario wasn't a thing, then it would have been sufficient(?) to rely solely on the node stack for fixture teardown, i.e. attach each fixture to a node in the stack as a dependent, and have the node tear its dependents down when it's normally popped from the stack.

Alas, param changes are a thing so we need to handle this "early" or "sudden" changes which break the stack ordering.

The current code tries to handle this by having fixtures register finalizers on other fixtures and nodes, which is somewhat inefficient, too dynamic and buggy in some edge cases.

In #4871 (comment) I discussed a couple of other solutions, one not really viable and the other probably not either.

The solution proposed here is to stop relying on finalizers and instead "take the hit" and build a full bidirectional dependency graph of fixtures and nodes in SetupState (bidirectional = both dependencies and dependents).

---

About _own_fixture_defs, I want to see if we can avoid adding it. With it we now have:

• arg2fixturedefs -- static argname -> FixtureDefs for the item (after #14290)
• _fixture_defs -- active argname -> FixtureDef for the item (both static and dynamic)
• _active_fixtures -- active FixtureDef -> _FixtureInfo for the item (both static and dynamic)
• _own_fixture_defs -- active argname -> FixtureDef for the FixtureRequest (both static and dynamic)
• and we also have SubRequest._parent_request.

I feel like some of this must be redundant and can be consolidated.

---

Regarding the full bidrectionality of the graph, are we sure we need that?

Ignoring the param-value-change problem for a moment, let's suppose _active_fixtures as a "fixture stack", i.e. setup -> push, teardown -> pop. Before teardown, the stack is effectively the topological ordering in which we chose to execute the fixture DAG. In this case, we could avoid keeping dependent_fixtures as follows: when a Node is torn down, pop and teardown from the _active_fixtures stack as long as the top-of-stack fixture scope is the node being torn down.

But in sudden param-value-change teardown situation, we only want to teardown that fixture and its dependents, not the entire fixture stack up to it (this is the design decision by pytest, which is maybe debatable...). OK, so let's say that for this case we need dependent_fixtures on fixtures. But maybe not for nodes?

The code also keeps dependencies for fixtures. This is needed to unregister from dependent_fixtures of dependency fixtures on teardown, to avoid double teardowns and garbage accumulation on high-scope fixtures. Hmm, it would be nice to avoid dependencies but (without spending too much thought on it) I don't see how.

---

Another thing this PR fixes is to tear down "stale" fixtures during item teardown rather than nextitem setup. This is achieved in _finish_stale_fixtures by going over all _active_fixtures and seeing if nextitem is going to change their param and if so do an early teardown.

The "go over all active fixtures" part I am a bit worried about, seems to me like it could be quite slow in some scenarios. What can we do to avoid this "full graph scan"?

[bluetech]

• Let's move the check logic (i.e. the actual if) back here. Can leave the error raise out of line as it would be too distracting here.
• Let's inline _get_cached_value back.

The reason is that I feel it's easier to grok the code logic when it's just reading top to bottom, without jumping to other functions.

[bluetech]

Should remove this paragraph now.

[bluetech]

Suggested change

	Fixtures are now rebuilt when param changes for a fixture they depend on, if the dependency is via :func:`request.getfixturevalue() <pytest.FixtureRequest.getfixturevalue>`.
	Fixtures are now rebuilt when param changes for a fixture they depend on, even if the dependency is via :func:`request.getfixturevalue() <pytest.FixtureRequest.getfixturevalue>`.

[bluetech]

Suggested change

	# fixture loading happens inside another fixture or inside an utility function.
	# fixture loading happens inside another fixture or inside a utility function.

[bluetech]

Suggested change

	"""Register the fixture as active. Should be called before fixture setup."""
	"""Register the fixture as active. Should be called before fixture setup.
	node:
	The node in the stack matching the fixture's scope (e.g. for a
	module-scoped fixture, the Module node).
	"""

[bluetech]

That's one wicked test 😀 I kinda wish we didn't allow a fixture to be both parametrized and not parametrized, though maybe there's a realistic use case for it.

[bluetech]

I think this might need to involve callspec.indices somehow, I don't remember so marking a note to check.

[bluetech]

Can we explain that the ordering of _active_fixtures is meaningful and some details about it?