Skip to content

fix: deduplicate Directory nodes on re-collection to preserve fixture identity (#14635) - #14645

Merged
RonnyPfannschmidt merged 2 commits into
pytest-dev:mainfrom
RonnyPfannschmidt:fix/14635-deduplicate-directory-nodes-on-recollection
Jul 22, 2026
Merged

RonnyPfannschmidt merged 2 commits into
pytest-dev:mainfrom
RonnyPfannschmidt:fix/14635-deduplicate-directory-nodes-on-recollection

Conversation

@RonnyPfannschmidt

Copy link
Copy Markdown
Member

Summary

Fixes #14635.

When Session re-collects a parent Directory (due to handle_dupes=False for file CLI args), fresh child nodes were created. Since fixture registration uses node identity for matching (_matchfactories checks fixturedef.node in parent_nodes), the new Directory children didn't match fixtures registered with the original instances.

This caused fixture closure computation to fail for tests collected after unrelated paths, because the conftest fixtures were registered with a now-orphaned Directory node instance.

Root cause: _collect_one_node with handle_dupes=False calls collect() on the parent Directory, which creates brand-new child Directory/Package instances. The _collection_cache is then updated with these new children. Later collection paths that look up cached children find different node objects than the ones conftest fixtures were registered with.

Fix: After re-collection, replace any freshly-created Directory children with the previously-seen instances for the same path. Module/File nodes are still recreated to preserve --keep-duplicates semantics (which needs fresh test items).

Test plan

Made with Cursor

@RonnyPfannschmidt
RonnyPfannschmidt force-pushed the fix/14635-deduplicate-directory-nodes-on-recollection branch from 441d49f to ecdc8d5 Compare July 14, 2026 07:58
@pyba-chronographer pyba-chronographer Bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Jul 14, 2026
@RonnyPfannschmidt
RonnyPfannschmidt force-pushed the fix/14635-deduplicate-directory-nodes-on-recollection branch from 2724383 to db00165 Compare July 14, 2026 08:13
@RonnyPfannschmidt
RonnyPfannschmidt marked this pull request as ready for review July 14, 2026 08:13
Copilot AI review requested due to automatic review settings July 14, 2026 08:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a regression where collecting multiple CLI paths (mixing directories and files) could cause a parent directory to be re-collected and its child Directory nodes to be replaced with fresh instances, breaking fixture visibility because fixture registration relies on collector node identity.

Changes:

  • Reconciles re-collected Directory children with previously-seen instances when handle_dupes=False, preserving fixture identity while still recreating file/module nodes for --keep-duplicates semantics.
  • Adds a regression test covering order-dependent fixture-closure/parametrize validation failures triggered by mixed dir/file collection arguments.
  • Adds a changelog entry describing the fixed behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/_pytest/main.py Reuses cached Directory children after re-collection to keep fixture registration tied to stable node identities.
testing/test_conftest.py Adds a regression test that fails on order-dependent collection when directory nodes are recreated.
changelog/14635.bugfix.rst Documents the fixture-resolution regression and its order-dependent symptom.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

RonnyPfannschmidt and others added 2 commits July 21, 2026 10:52
… identity (pytest-dev#14635)

When Session re-collects a parent Directory (handle_dupes=False for file
CLI args), fresh child nodes were created. Since fixture registration
uses node identity for matching, the new Directory children didn't match
fixtures registered with the original instances.

Reuse previously-seen Directory children (by path) when re-collecting a
parent. Module/File nodes are still recreated to preserve
--keep-duplicates semantics.

Co-authored-by: Cursor AI <ai@cursor.sh>
Co-authored-by: Anthropic Claude Opus 4 <claude@anthropic.com>
…#14635

Add a changelog entry explaining the fix for fixture resolution failures
when multiple CLI paths share ancestor directories, and a comment on
the dedup code explaining why post-processing was chosen over a
session-global deduplication mechanism.

Co-authored-by: Cursor AI <ai@cursor.sh>
Co-authored-by: Anthropic Claude Opus 4 <claude@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@RonnyPfannschmidt
RonnyPfannschmidt force-pushed the fix/14635-deduplicate-directory-nodes-on-recollection branch from db00165 to b054cac Compare July 21, 2026 08:55
@RonnyPfannschmidt
RonnyPfannschmidt merged commit a933cf5 into pytest-dev:main Jul 22, 2026
36 checks passed
@RonnyPfannschmidt RonnyPfannschmidt added the backport 9.1.x apply to PRs at any point; backports the changes to the 9.1.x branch label Sep 4, 2026
@patchback

patchback Bot commented Sep 4, 2026 •

Copy link
Copy Markdown

Backport to 9.1.x: 💚 backport PR created

✅ Backport PR branch: patchback/backports/9.1.x/a933cf5de4d64192d1d588a851ca4e76eec71efc/pr-14645

Backported as #14968

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

RonnyPfannschmidt added a commit that referenced this pull request Sep 6, 2026
…933cf5de4d64192d1d588a851ca4e76eec71efc/pr-14645

[PR #14645/a933cf5d backport][9.1.x] fix: deduplicate Directory nodes on re-collection to preserve fixture identity (#14635)
aviator-app Bot pushed a commit to Senkichi/jobcannon that referenced this pull request Sep 24, 2026
… fixture loss (#447)

Closes #374

## Root cause (upstream pytest defect, not repo code)

Instrumented the failing run against the installed pytest 9.1.1 and confirmed the mechanism:

- pytest 9.1 (pytest-dev/pytest#14098) defers conftest fixture parsing until a directory's `Directory` collector node is collected, and binds the resulting `FixtureDef`s to that node **instance** (`fixturedef.node in node.iter_parents()`; `Node` defines `__hash__` by nodeid but no `__eq__`, so matching is by identity).
- When a file CLI arg's parent directory is re-collected (`Session._collect_one_node` with `handle_dupes=False`), `Dir.collect()` produces **fresh child collector instances**, and the new collect report overwrites `_collection_cache[parent]`. Later file args in the same subtree are then parented under a different `Directory`/`Package` instance than the one the conftest fixtures were bound to — the bound node is orphaned, and every test under it errors with `fixture '<name>' not found`.

Probe output for the issue's exact argv (`tests/host/test_m0006_analytics_consent_version.py tests/test_migration_deploy_safety.py tests/host/test_company_attribution.py`):

- `tests/host` Package `id=...489824` collected first → `_pending_conftests` popped → `db_conn` fixturedef bound to `id=...489824`.
- `Dir(tests)` re-collected for the `test_migration_deploy_safety.py` arg → fresh `tests/host` Package `id=...222256` shadowed it in the cache.
- `test_company_attribution.py` items' parent chain contains `id=...222256`, not `id=...489824` → `fixture 'db_conn' not found`.

This is upstream pytest-dev/pytest#14635, fixed by pytest-dev/pytest#14645 — but that fix is only in the unreleased pytest 9.2 (latest PyPI release is 9.1.1; verified the same argv passes on 9.0.3 and fails on 9.1.1 with a minimal tmp-dir repro, so the regression is exactly the 9.1 series).

## Change

- `pyproject.toml` dev group: `pytest>=8.0` → `pytest>=8.0,!=9.1.*` (comment cites the upstream issue/PR and the sentinel test). `!=9.1.*` resolves to 9.0.3 today and automatically admits the fixed 9.2+ once released — the exclusion lifts itself.
- `uv.lock` re-locked: `pytest 9.1.1 → 9.0.3` (10-line diff: specifier + pytest version/artifact entries only).
- New `tests/test_fixture_scope_collection_order.py`: runs a real `sys.executable -m pytest` subprocess against a throwaway tree in `tmp_path` (subdir with `__init__.py` + conftest fixture, sibling file at the shared parent), parametrized over the trigger order (`pkg/test_first.py test_sibling.py pkg/test_second.py`) and a control order. The defect only manifests through collection-argument order, so a subprocess is required — it cannot be reproduced in-process.

## Verification

- Issue's repro command, BEFORE (pytest 9.1.1): `uv run pytest tests/host/test_m0006_analytics_consent_version.py tests/test_migration_deploy_safety.py tests/host/test_company_attribution.py -n0 -q` → `120 passed, 5 errors` — all 5 `db_conn`-requesting tests in `test_company_attribution.py` fail at setup with `fixture 'db_conn' not found` (the file now has 5 such tests; issue reported 4).
- Same command, AFTER (pytest 9.0.3): `125 passed in 10.47s`.
- New regression test on pinned env: `uv run pytest tests/test_fixture_scope_collection_order.py -n0 -q` → `2 passed`.
- `uv run ruff check .` → `All checks passed!`; `uv run ruff format --check .` → `715 files already formatted`.
- Full suite on pinned env (justified: the diff changes the test-runner version itself, blast radius = every test): `uv run pytest -q --tb=short -n auto` → `1 failed, 6463 passed, 77 skipped, 5 warnings in 379.92s`.
  - The one failure — `tests/engine/test_smartrecruiters_scanner.py::TestSmartRecruitersCompleteness::test_page_fetch_concurrency_clamps_to_range` (`expected=6, got=5`) — is a thread-pool timing assertion under `-n auto` load, unrelated to the pytest version; it passes standalone on the same tree: `uv run pytest tests/engine/test_smartrecruiters_scanner.py::TestSmartRecruitersCompleteness::test_page_fetch_concurrency_clamps_to_range -n0 -q` → `1 passed`.

## Mutation check

Reverted artifact: the `pytest` spec in `pyproject.toml` (the fix), applied as `uv pip install pytest==9.1.1` into the venv + `--no-sync` run — functionally identical to the unfixed dependency state.

Against pytest 9.1.1 the new test FAILS (trigger order only; control order passes — matching the issue's order-dependence):

```
FAILED tests/test_fixture_scope_collection_order.py::test_subdir_conftest_fixtures_survive_arg_order[sibling-between-subdir-files]
E       AssertionError: inner pytest run failed (order-dependent conftest fixture loss?)
E         argv: ['pkg/test_first.py', 'test_sibling.py', 'pkg/test_second.py']
E         ... ERROR at setup of test_second ... fixture 'shared_fix' not found ...
1 failed, 1 passed in 4.21s
```

After restoring the pin (`uv lock`/`uv sync --dev` → pytest 9.0.3):

```
uv run pytest tests/test_fixture_scope_collection_order.py -n0 -q
2 passed in 2.78s
```

## Risks / uncertain areas

- Dev env and CI resolve to pytest 9.0.3 instead of 9.1.1 until upstream ships 9.2; the `!=9.1.*` spec admits 9.2+ automatically, and the sentinel test proves the fix on whatever version is installed. Deliberately did NOT hand-patch pytest internals from a conftest — `_pending_conftests`/`parsefactories` are private APIs changing again in 9.2.
- The issue's open question — whether `-n auto` (xdist) collection is also affected — was not separately verified; it is moot under the pin. The full-suite local run under `-n auto` showed no fixture errors.
- 9.0.3 vs 9.1.1 behavior deltas across the suite are covered by the full-suite run above (6463 passed; the single failure is the disclosed timing flake).

## Test plan

- [x] `uv run pytest tests/host/test_m0006_analytics_consent_version.py tests/test_migration_deploy_safety.py tests/host/test_company_attribution.py -n0 -q` → 125 passed
- [x] `uv run pytest tests/test_fixture_scope_collection_order.py -n0 -q` → 2 passed
- [x] `uv run pytest -q --tb=short -n auto` → 6463 passed, 77 skipped, 1 disclosed timing flake
- [x] `uv run ruff check .` / `uv run ruff format --check .` → clean
@RonnyPfannschmidt
RonnyPfannschmidt deleted the fix/14635-deduplicate-directory-nodes-on-recollection branch October 4, 2026 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 9.1.x apply to PRs at any point; backports the changes to the 9.1.x branch bot:chronographer:provided (automation) changelog entry is part of PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error collecting tests on 9.1.1 - fixtures not found when a parent directory appears multiple times in a argument list

4 participants