Skip to content

fixtures: fix closure computation with indirect override chains#13789

Merged
bluetech merged 2 commits intopytest-dev:mainfrom
bluetech:fixtures-closure-visibility-3
Oct 10, 2025
Merged

fixtures: fix closure computation with indirect override chains#13789
bluetech merged 2 commits intopytest-dev:mainfrom
bluetech:fixtures-closure-visibility-3

Conversation

@bluetech
Copy link
Copy Markdown
Member

@bluetech bluetech commented Oct 6, 2025

Continuation of #13774.

The previous fix was a minimal change to make the existing code at least consider fixtures overrides, and handles the common case (direct override chains), but still wasn't correct, as it didn't handle override chains involving an intermediary (not the overridden) fixture.

To make this work, the algorithm needs to change. Now instead of a simple breadth-first search, we do a depth-first search, which more closely simulates the runtime behavior, and allows us the to check the "stack" of fixtures so we can use the correct fixture index (depth in the override chain).

This is more expensive but should be OK.

Refs #13773.

Explicitly test a couple more scenarios. One of the tests doesn't
currently pass so marked xfail.
Continuation of 72ae3db.

The previous fix was a minimal change to make the existing code at least
consider fixtures overrides, and handles the common case (direct
override chains), but still wasn't correct, as it didn't handle
override chains involving an intermediary (not the overridden) fixture.

To make this work, the algorithm needs to change. Now instead of a
simple breadth-first search, we do a depth-first search, which more
closely simulates the runtime behavior, and allows us the to check the
"stack" of fixtures so we can use the correct fixture index (depth in
the override chain).

This is more expensive but should be OK.

Refs pytest-dev#13773.
@bluetech
Copy link
Copy Markdown
Member Author

Going to merge this, because it finishes the work in #13774 for pytest 9.0, and I have some work planned which depends on it.

I also noticed that in #11234 @sadra-barikbin had the same idea (changing fixturemanager::getfixtureclosure algo from BFS to DFS), though didn't get to implement it. So must have some merit :)

@bluetech bluetech merged commit 37acc4f into pytest-dev:main Oct 10, 2025
33 checks passed
@bluetech bluetech deleted the fixtures-closure-visibility-3 branch October 10, 2025 10:50
bluetech added a commit to bluetech/pytest that referenced this pull request Apr 8, 2026
… from overridden fixtures

As the xfail'ed test
`test_fixture_closure_with_overrides_and_parametrization` demonstrates,
adding any direct parametrization causes the statically computed fixture
closure to omit transitive dependencies from overridden (but still
requested) fixtures. (The dynamic algorithm does get it right).

The `prune_dependency_tree()` function did not account for overridden
fixtures properly (only checked `-1`). Make it share the core recursive
DFS traversal used by `getfixtureclosure()`, which already fixed a
similar problem (pytest-dev#13789).

An alternative solution is to replace `prune_dependency_tree` wholesale
with another call to `getfixtureclosure`, as proposed in PR pytest-dev#11243,
however it's somewhat harder to get working, so let's go with this
solution for now.

Fix pytest-dev#14248
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip news used on prs to opt out of the changelog requirement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant