Fix conftest fixture scoping when testpaths points outside rootdir #14118
Conversation
- Add compute_nodeid_prefix_for_path() for better nodeid computation: - Paths in site-packages use site:// prefix - Nearby paths (≤2 levels up) use relative paths with .. - Far-away paths use absolute paths - Add _path_in_site_packages() with optional site_packages parameter for testing - Fix _getautousenames() to skip duplicate nodeids (Session and root Dir both have nodeid='') - Add unit tests for nodeid prefix computation Co-authored-by: Cursor AI <ai@cursor.sh> Co-authored-by: Anthropic Claude <claude@anthropic.com>
Fix conftest fixture scoping when testpaths points outside rootdir (pytest-dev#14004). Previously, conftests outside rootpath got nodeid '' (empty string), making their fixtures global and leaking to sibling directories. Changes: - Remove initial_paths from compute_nodeid_prefix_for_path since nodeids relate to collection tree structure, not command-line paths - Use compute_nodeid_prefix_for_path in pytest_plugin_registered for consistent conftest baseid computation - Paths outside rootpath now consistently use bestrelpath from invocation_dir This ensures conftest fixtures are properly scoped regardless of whether tests are inside or outside rootpath. Co-authored-by: Cursor AI <ai@cursor.sh> Co-authored-by: Anthropic Claude Opus 4 <claude@anthropic.com>
psf-chronographer
bot
added
the
bot:chronographer:provided
|
@bluetech after some consideration i decided to drop iterating initial-paths - only rootdir/invocation dir should be considered |
There was a problem hiding this comment.
Pull request overview
This PR fixes conftest.py fixture scoping when testpaths configuration points outside the rootdir. Previously, conftests outside the root path received empty nodeids, causing their fixtures to become global instead of being scoped to their directory.
Changes:
- Introduces
compute_nodeid_prefix_for_path()function that computes nodeids consistently for paths outside rootdir using relative paths from invocation_dir, site:// prefixes for site-packages, or absolute paths for distant locations - Replaces manual nodeid computation in
FSCollector.__init__andFixtureManager.pytest_plugin_registeredwith the new centralized function - Adds deduplication logic in
_getautousenames()to prevent duplicate fixtures when multiple nodes share the same nodeid
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/_pytest/nodes.py | Adds compute_nodeid_prefix_for_path() and helper functions for site-packages detection; updates FSCollector.__init__ to use the new function |
| src/_pytest/fixtures.py | Updates pytest_plugin_registered() to use new nodeid computation; adds deduplication in _getautousenames() |
| testing/test_nodes.py | Adds comprehensive unit tests for the new nodeid computation functions covering various path scenarios |
| changelog/14004.bugfix.rst | Documents the bug fix and new nodeid computation behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for sp in site.getsitepackages(): | ||
| try: | ||
| dirs.add(Path(sp).resolve()) | ||
| except OSError: | ||
| pass | ||
| # User site-packages | ||
| user_site = site.getusersitepackages() |
There was a problem hiding this comment.
The site.getsitepackages() function may not be available in all Python environments. In some older versions or certain virtualenv configurations, this function might not exist. Consider wrapping the call in a try-except block to handle AttributeError if the function doesn't exist, similar to how OSError is already handled for individual paths.
| for sp in site.getsitepackages(): | |
| try: | |
| dirs.add(Path(sp).resolve()) | |
| except OSError: | |
| pass | |
| # User site-packages | |
| user_site = site.getusersitepackages() | |
| try: | |
| site_packages = site.getsitepackages() | |
| except AttributeError: | |
| site_packages = [] | |
| for sp in site_packages: | |
| try: | |
| dirs.add(Path(sp).resolve()) | |
| except OSError: | |
| pass | |
| # User site-packages | |
| try: | |
| user_site = site.getusersitepackages() | |
| except AttributeError: | |
| user_site = None |
| ) | ||
|
|
||
| # Should use absolute path since it's more than 2 levels up | ||
| assert result == str(far_away) |
There was a problem hiding this comment.
The test assertion assert result == str(far_away) will fail on Windows because compute_nodeid_prefix_for_path always returns paths with forward slashes (line 673 replaces os.sep with SEP), but str(far_away) will contain backslashes on Windows. The assertion should be assert result == str(far_away).replace(os.sep, nodes.SEP) to ensure cross-platform compatibility.
| for sp in site.getsitepackages(): | ||
| try: | ||
| dirs.add(Path(sp).resolve()) | ||
| except OSError: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| if user_site: | ||
| try: | ||
| dirs.add(Path(user_site).resolve()) | ||
| except OSError: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
1 participant
Fixes #14004.
Conftests outside rootpath previously got empty nodeid, making fixtures global. Now uses
compute_nodeid_prefix_for_pathfor consistent nodeids based onbestrelpathfrom invocation_dir.