Skip to content

fix #14004 - connect conftests to nodeids/nodes instead of matching string prefixes - ensure we connect in the collect phase in case of directory confusion#14098

Open
RonnyPfannschmidt wants to merge 5 commits into
pytest-dev:mainfrom
RonnyPfannschmidt:fix-14004-contest-nodeid-assingnment
Open

Conversation

@RonnyPfannschmidt
Copy link
Copy Markdown
Member

fixes #14004

the key issue was that nodeid/baseid for fixtures of such conftest.py would be "" instead of something fitting

ths pr is a chain of changes

  • 694e2f6 is a quickfix i volunteer to backport
  • the rest migrates to assigning fixtures to nodes nodes as @bluetech introduced directory nodes that make string matching unnecessary and deprecates it

@psf-chronographer psf-chronographer Bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Jan 9, 2026
@RonnyPfannschmidt RonnyPfannschmidt marked this pull request as ready for review January 9, 2026 12:06
Copilot AI review requested due to automatic review settings January 9, 2026 12:06
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes conftest fixture scoping when testpaths points outside rootdir using relative paths (issue #14004). The fix migrates from fragile string prefix matching to robust node-based matching for fixture scoping. Conftest fixtures are now parsed during Directory collection, using the Directory node's nodeid for proper scoping instead of calculating string-based nodeids during plugin registration.

  • Implements deferred conftest parsing tied to Directory collection
  • Adds node-based fixture matching alongside string-based fallback for compatibility
  • Deprecates baseid/nodeid string parameters in favor of node parameter

Reviewed changes

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

Show a summary per file
File Description
testing/test_conftest.py Adds comprehensive regression test for fixture scoping with testpaths outside rootdir
src/_pytest/fixtures.py Implements deferred conftest parsing, node-based matching, and deprecates string-based APIs
src/_pytest/unittest.py Updates fixture registration calls to use new node parameter
src/_pytest/python.py Updates fixture registration calls to use new node parameter
changelog/14004.deprecation.rst Documents deprecation of baseid/nodeid string parameters
changelog/14004.bugfix.rst Documents the fixture scoping fix

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

@bluetech
Copy link
Copy Markdown
Member

Interesting!

The "tests outside of the rootdir" scenario is ill-advised but I guess we need to support it.


Regarding the first commit, I'm looking at the test test_conftest_fixture_scoping_with_testpaths_outside_rootdir, and first thing I wanted to understand is the collection tree and nodeid's generated in this case (when pytest is run from the rootdir pytester.path/sdk). The result is (I added the nodeid reprs after the names):

<Dir tests> '::tests'
  <Dir sdk> ''
    <Dir inner> 'inner'
      <Module test_inner.py> 'inner/test_inner.py'
        <Function test_inner> 'inner/test_inner.py::test_inner'
    <Module test_outer.py> 'test_outer.py'
      <Function test_outer> 'test_outer.py::test_outer'

That seems odd and broken to me, there's obviously a confusion going on here. Generally the intention is for nodeid's to be relative to the rootdir, so what happens when the collection path is not under the rootdir? I've already written about this issue in #11245.

The commit "fix #14004 - connect conftests to nodeids/nodes instead of matching string prefixes - ensure we connect in the collect phase in case of directory confusion" works around the issue using some "fixup" code in fixtures.py. I think it's a bit confusing, particularly the function name "_get_nodeid_for_path_outside_rootpath", it basically invents a "fake" nodeid because the existing nodeid is broken, but it's not an actual ID of a node...

I think that if we fix the underlying issue #11245, that would be a better way to go? In that issue we had a couple of suggestions.

Also regarding the implementation itself, I fear that the loop over the config.args might create an O(n^2) situation which could be problematic if lot's of args are given (I know some users pass lot's of args). I haven't investigated in detail, just something to look out for.


Regarding the "fix: assign conftest fixtures to Directory nodes during collection" idea and subsequent changes, I think I like the direction, I definitely think we should to try to integrate conftests with the collection tree better, and get away from the current path matching. Thanks for taking a look at this.

I need to look into it more, but can you describe the interaction with initial conftests? Are they associated with a Directory or are they "global"?

The _pending_conftests adds a bit of tricky state, I wonder if we can't pass the node directly instead of relying on the collection report hook? I'm thinking here:

def pytest_make_collect_report(collector: Collector) -> CollectReport:
def collect() -> list[Item | Collector]:
# Before collecting, if this is a Directory, load the conftests.
# If a conftest import fails to load, it is considered a collection
# error of the Directory collector. This is why it's done inside of the
# CallInfo wrapper.
#
# Note: initial conftests are loaded early, not here.
if isinstance(collector, Directory):
collector.config.pluginmanager._loadconftestmodules(
collector.path,
collector.config.getoption("importmode"),
rootpath=collector.config.rootpath,
consider_namespace_packages=collector.config.getini(
"consider_namespace_packages"
),
)

@RonnyPfannschmidt
Copy link
Copy Markdown
Member Author

the main confusion thats happening is that initial conftest files are loaded before we have a node, so we cant parse factories until we see them again in collection

the case of tets outside of a rootdir is not as ill advised as one might think the moment one ships integration testsuites as installed python packages and suddenly many tests collect as part of site-packages instead of the cwd

@bluetech
Copy link
Copy Markdown
Member

The "add: nodes.norm_sep helper for nodeid path separator normalization" change looks sensible, you can submit and merge it to main with my review if you'd like. As an internal refactoring it doesn't need a changelog entry though.

Can you submit "improve: compute meaningful nodeids for paths outside rootdir" in its own PR? I have some thoughts but it's a substantial change so will be better considered separately before this PR.

@bluetech
Copy link
Copy Markdown
Member

the main confusion thats happening is that initial conftest files are loaded before we have a node, so we cant parse factories until we see them again in collection

But do all initial conftests will have a node? Conftests are looked up the filesystem tree (up to confcutdir) while nodes are not. So I figure it's possible to have a conftest plugin without a corresponding Directory, though I haven't verified this so might be wrong about that.

@RonnyPfannschmidt
Copy link
Copy Markdown
Member Author

As far as I understand any conftest not having a attached node isn't a valid fixture source

@bluetech
Copy link
Copy Markdown
Member

You may be right but that would surprise me! I'd imagine these fixtures would be "global" (empty/None baseid, don't remember which one of them). Are you able to verify this?

@RonnyPfannschmidt
Copy link
Copy Markdown
Member Author

a conftest is a plugin that is local to a directory tree, the only reason we do initial conftests to begin with is ci arg and config value handling for full testsuites - so no - a conftest should never be a global plugi never

@RonnyPfannschmidt RonnyPfannschmidt force-pushed the fix-14004-contest-nodeid-assingnment branch 2 times, most recently from 9f24dcd to 585cdc4 Compare January 29, 2026 14:15
@bluetech
Copy link
Copy Markdown
Member

a conftest is a plugin that is local to a directory tree, the only reason we do initial conftests to begin with is ci arg and config value handling for full testsuites - so no - a conftest should never be a global plugi never

I don't know, I think having the ability for a project to locally define fixtures with global visibility is useful, even necessary. I agree it's a bit unfortunate to conflate it with conftests which are meant to be Directory-local, but initial conftests is what we have and I'm not sure it's worth it to add another mechanism1. So I think my vote is to give initial conftests global visibility (i.e. their node would be Session).

Footnotes

  1. Well, I actually would like to have a config pointing at a local plugin file (what we currently call initial conftest), rather than having to rely on the initial conftest search heuristics. And if we add a config we might as well call it something other than conftest, and then maybe we can keep conftest as Directory-local since we provide an alternative. But I'm not sure it's worth the extra complexity.

@RonnyPfannschmidt
Copy link
Copy Markdown
Member Author

that would stil lbe a attached node for me - im absolutely fine with making initial conftests something for the session/root dirrectory - but i di suspect edge cases coming up

@bluetech
Copy link
Copy Markdown
Member

OK, great.

Overall, I think attaching conftests and FixtureDefs explicitly to nodes will be a great step in bringing more clarity to pytest's core model. So if I have time I'd like to look into it more, maybe using this PR as a starting point.

@bluetech
Copy link
Copy Markdown
Member

I looked at it a bit. I wanted to start with conftest <-> Directory change. As mentioned earlier, I really want to avoid the _pending_conftests deferred loading solution, since I think it makes things harder to understand. So I'd prefer if whenever we want to load a conftest, we are ready to give it a node.

For non-initial nodes it's not a problem, we already have the Directory node as we are loading the conftest during collection (ref).

For initial conftests, the loading is done before collection so we don't have Directory nodes, and my idea above was to use Session (global visibility) for them. However, there are a few things to consider first:


Due to the way initial conftests are currently decided, I think that might be very confusing. Specifically, if the user passes a directory arg to pytest, then we load initial conftests from that directory. So with my suggestion, if the user writes e.g. pytest some/deep/path, suddenly random fixtures from some/deep/path/conftest.py become global, which I think is unexpected.

Really, I think the problem with this one is not the global visibility idea, but the fact that we even consider random directory args as initial conftest candidates. To me it seems incoherent that a selection arg can affect whether a conftest is initial or not. I might open an issue about this.


The two other places where conftest path is used are:

  • collect_ignore/collect_ignore_glob variables in conftest.py, which only affect the specific directory (consulted in pytest_ignore_collect).
  • For path-dependent hooks that only consider hook implementations in conftests in its filesystem ancestors. AKA gethookproxy/ihook.

The thing to decide here is whether the hook visibility of initial fixtures will also become global, for consistency with fixture visibility.

I think we do want them to be consistent, so if the answer here is "no", then I think we should scrape the idea.

If we make initial conftest loading more sane as discussed in the previous point, I think it might be sensible to make initial conftest have global hook-visibility. So I'll defer until we decide on that.


Expanding from the previous point a bit, in a future where we strictly attach conftests to nodes, we should decide between two models:

  • A conftest affects the filesystem directory sub-tree it is located in.
  • A conftest affects the collection sub-tree of the node it is attached to.

The first is what we do now. In practical terms, when we look up conftests which affect something, we do it by path.

The second would mean that once a conftest is attached to a node, we completely "forget" the conftest's path, and when looking up conftests, we always do it by node.

Philosophically I much prefer to lean in fully on the collection tree. Seems much more "pytest native" and clean, than to keep relying on the parallel filesystem tree post-collection. For common usage in practice the two would be mostly equivalent, but the second gives plugins more room to play.

Happy to hear opinions.

@RonnyPfannschmidt
Copy link
Copy Markdown
Member Author

by path is broken and not fixable in a sensible manner (i tried in the quickfix and badly failed running into one edge case after another)

my understanding of conftests has always been that they are plugins local do a collection tree, their behaviour got strange when directoy nodes got removed initially - now that they are back we can undo that

the pending check is necessary as initial conftests currently exist before initial nodes, im happy to make that unnecesary, but right now its not a simple/feasible change - in between initial conftests and plugin fixture parsing

in future i expect to move fixture parsing entirely to collection time and mapping conftests there without the need for pending - but the requried changes are something i consider out of scope for this pr

@nicoddemus
Copy link
Copy Markdown
Member

I didn't have time to read the PR code (sorry about that), but wanted to comment on #14098 (comment):

Really, I think the problem with this one is not the global visibility idea, but the fact that we even consider random directory args as initial conftest candidates. To me it seems incoherent that a selection arg can affect whether a conftest is initial or not.

I agree completely, this always felt like a wart to me.

At work, we put the source code in a deeply nested directory, source/python, due to legacy reasons and also because we often have other languages in the same project (source/cpp, source/js, etc.). Because of this layout, if we want to load source/python/eden/conftest.py as initial conftest, then we need to add source/python to the testpaths config variable.

The special rules around initial conftests are really confusing and hard to explain to newcomers.

The thing to decide here is whether the hook visibility of initial fixtures will also become global, for consistency with fixture visibility.

(I assume you mean "initial conftests" above).

I don't think fixtures should become global, as much as I understand the "consistency" reationale.

Philosophically I much prefer to lean in fully on the collection tree.

I agree, we should definitely do that. Theoretically, it should be possible to have fixtures and tests which are not even associated with a file system path.

my understanding of conftests has always been that they are plugins local do a collection tree,

Also my understanding.

Seems like "contests as a directory-local mini plugin" is an easy concept to understand and explain. On the other hand "initial conftests" always seems esoteric and hard to understand (so much I did not find anything in the docs that clearly explain what they are).

@RonnyPfannschmidt
Copy link
Copy Markdown
Member Author

im of the impression that initial conftests should be deprecated and we should have a better way to spell out per project local plug-in that arent conftests

i vaguely recall that in the early days - ALL conftests where always added as plugins - and it was a disaster

this could play into my proposal for testroots - having a actual config that sets up initial collection defaults and initial local plugins

@RonnyPfannschmidt RonnyPfannschmidt force-pushed the fix-14004-contest-nodeid-assingnment branch 3 times, most recently from a1ac5e1 to f6a6122 Compare May 1, 2026 15:16
@RonnyPfannschmidt
Copy link
Copy Markdown
Member Author

@nicoddemus @bluetech please have another look

@bluetech
Copy link
Copy Markdown
Member

bluetech commented May 9, 2026

Thanks for the update.

I've come to terms with _pending_conftests for now; maybe we can eliminate in the future but for now it's good progress and unlocks further improvements.

I'd like to discuss the deprecations. Basically I think we should skip the deprecation here, as these are internal APIs. It will break some plugins, but plugins which use private details should be ready for this. Normally I wouldn't object to a deprecation, I don't mean to make life unnecessarily hard for plugins, but just in this case the need to keep supporting nodeids until pytest 10 will block further progress. As a small consolation I'm pretty sure this change will unlock stabilizing a pytest.register_fixture function, so at least for this functionality we could offer a stable alternative. The main pain will then be the break in parsefactories.

If you feel strongly about the deprecation, I'll be happy to approve with it, but hopefully I can convince you :)

@RonnyPfannschmidt
Copy link
Copy Markdown
Member Author

i absolutely want to replace pending/initial conftests by something akin to my collection roots proposal - but that's a bit further away

i consider the deprecation a important goodwill to plug-ins like pytest-cases and a few more

Comment thread changelog/14004.bugfix.rst Outdated
Comment thread changelog/14004.bugfix.rst Outdated
Comment on lines +6 to +7
Conftest fixtures are now parsed during Directory collection, using the Directory node's
nodeid for proper scoping.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Conftest fixtures are now parsed during :class:Directory <pytest.Directory> collection, using the Directory node for proper scoping.

Comment thread changelog/14004.deprecation.rst Outdated
Comment on lines +1 to +2
Passing ``baseid`` to :class:`~pytest.FixtureDef` or ``nodeid`` strings to fixture registration
APIs is now deprecated.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Passing ``baseid`` to :class:`~pytest.FixtureDef` or ``nodeid`` strings to fixture registration
APIs is now deprecated.
Passing ``baseid`` to :class:`~pytest.FixtureDef` or ``nodeid`` strings to fixture registration APIs is now deprecated. These are internal pytest APIs that are used by some plugins.

Comment thread src/_pytest/fixtures.py Outdated
# baseid=None (global plugins) and baseid="" (synthetic fixtures) are fine.
if baseid and node is None:
warnings.warn(
"Passing baseid to FixtureDef is deprecated. "
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you move the Warning to a constant in deprecated.py? It makes it easier on major releases to track the deprecations.

Comment thread src/_pytest/fixtures.py Outdated
# nodeid=None (global plugins) is fine.
if nodeid and node is None:
warnings.warn(
"Passing nodeid to _register_fixture is deprecated. "
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here, can you move to a constant in deprecated.py?

Comment thread src/_pytest/fixtures.py
Comment on lines +1923 to +1924
# Global plugin autouse fixtures go under Session.
self._node_autousenames.setdefault(self.session, []).append(name)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not backward compatible when passing nodeid. Previously would have nodeid autouse visibility, now session. Maybe it'd be filtered anyway in matchfactories? But still seems undesirable.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

nodeid or "" is kind of an equivalent of node or session

but plain nodeids in that mapping need some exta consideration

@bluetech
Copy link
Copy Markdown
Member

bluetech commented May 9, 2026

One more thing I forgot, we should add an entry in deprecations.rst.

RonnyPfannschmidt and others added 5 commits May 12, 2026 08:52
Fixes pytest-dev#14004 - conftest fixtures now properly scoped when testpaths
points outside rootdir.

Instead of computing fixture nodeids from path strings during plugin
registration, conftest fixtures are now deferred until their Directory
is collected. The Directory node is stored on FixtureDef and used for
node-identity matching, which is more robust than string prefix matching.

Changes:
- Add _pending_conftests dict to defer conftest fixture parsing
- Parse conftest fixtures via pytest_make_collect_report hook
- Add node parameter to FixtureDef and _register_fixture
- Add parsefactories(holder=, node=) keyword-only API
- Use node-based matching in _matchfactories with string fallback

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

Update all internal call sites to use node= parameter instead of nodeid=
string for fixture registration, enabling node-based matching.

Changes:
- python.py: Module and Class xunit fixtures now use node=self
- python.py: Class.collect parsefactories uses holder=..., node=self
- unittest.py: UnitTestCase fixtures now use node=self
- unittest.py: UnitTestCase.collect parsefactories uses holder=..., node=self

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

Add deprecation warnings for string-based fixture scoping APIs:
- FixtureDef baseid parameter: use node parameter instead
- _register_fixture nodeid parameter: use node parameter instead
- parsefactories nodeid string: use parsefactories(holder=, node=) instead

Warnings only trigger when a non-empty nodeid string is passed without
a node. Global plugins (nodeid=None) and synthetic fixtures (baseid='')
do not trigger warnings.

Co-authored-by: Cursor AI <ai@cursor.sh>
Co-authored-by: Anthropic Claude Opus 4 <claude@anthropic.com>
- Refactor _nodeid_autousenames to _node_autousenames: use Node objects
  instead of nodeid strings to prevent duplicates and improve consistency
- Flush above-rootdir conftests to Session scope in __init__ rather than
  in pytest_make_collect_report, ensuring they're processed even when
  collection fails before Session collection starts (e.g. bad args)
- Add pytest_collection_finish cleanup for interrupted-collection leftovers
- Add regression test for pytest-dev#14004 (conftest fixture leak across siblings)
- Add test for ancestor conftests above rootdir via confcutdir

Co-authored-by: Cursor AI <ai@cursor.sh>
Co-authored-by: Anthropic Claude Opus 4 <claude@anthropic.com>
- Use :confval:, :ref:, and :class: rst formatting in changelog
- Note that deprecated APIs are internal, used by some plugins
- Move deprecation warnings to constants in deprecated.py for easier tracking
- Fix backward compat: legacy nodeid-based autouse scoping preserved via
  _nodeid_autousenames fallback dict instead of falling through to Session

Co-authored-by: Cursor AI <ai@cursor.sh>
Co-authored-by: Anthropic Claude Opus 4 <claude@anthropic.com>
@RonnyPfannschmidt RonnyPfannschmidt force-pushed the fix-14004-contest-nodeid-assingnment branch from b07ea7d to a2398a7 Compare May 12, 2026 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided (automation) changelog entry is part of PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Conftest fixtures leak to sibling directories when testpaths points outside rootdir

4 participants