Repository navigation
Conversation
load_setuptools_entrypoints() recorded the distribution of every entry point it found and counted it as loaded, without looking at what register() did. register() registers nothing and returns None for a None plugin, and a PluginManager subclass may decline as well: pytest's declines the names of plugins which were merged into pytest (pytest_faulthandler, pytest_catchlog, ...). The recorded distributions are keyed by plugin name (pytest-dev#763), so list_plugin_distributions() looks those names up in the plugin registry, and list_plugin_distinfo() calls it: for a declined but recorded entry point both raised KeyError. With pytest-faulthandler installed, a plain pytest run died with "INTERNALERROR ... KeyError: 'pytest_faulthandler'" while printing the header, which lists plugins through list_plugin_distinfo(). Skip the bookkeeping when register() reports no registered name, and key the distribution under the name register() returned. Tests: new test_load_setuptools_declined_plugin_is_not_recorded covers a None plugin and a manager subclass which declines one, and fails on the previous code with "assert 2 == 0". Signed-off-by: simpleqt <2015900818@qq.com>
Contributor
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
Member
|
🤖 Written by Claude Opus 5.5 via Claude Code for the pluggy maintainers; I prompted it, it did the work, I read it. Closing. We don't accept PRs from unattended AI agents. This account opened 30+ PRs across unrelated projects today. The bug is real and is now tracked in #807. A maintainer will fix it. Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
PluginManager.load_setuptools_entrypoints()counted every entry point it found as a loaded plugin and remembered its distribution, without looking at whatregister()did:register()registers nothing and returnsNonein two cases that reach this code:None(register(None)is ignored since fix: reject PluginManager.register(None) #731), andPluginManagersubclass declines the plugin. pytest'sPytestPluginManager.register()does exactly that for the plugin names inDEPRECATED_EXTERNAL_PLUGINS(pytest_faulthandler,pytest_catchlog,pytest_capturelog,pytest_subtests): it warns "plugin has been merged into the core" and returnsNone.Since #767, the recorded distributions are keyed by plugin name and
list_plugin_distributions()resolves those names in the plugin registry:so a declined-but-recorded entry point makes
list_plugin_distributions()— andlist_plugin_distinfo(), which calls it — raiseKeyError.Real world impact:
TerminalReporter.pytest_report_header()renders theplugins:line throughconfig.pluginmanager.list_plugin_distinfo(). With the (still common)pytest-faulthandlerpackage installed, every pytest run aborts withConfig._validate_plugins()(required_pluginsini) and--trace-config/pytest --helpreach the same code.Reproduction
1. pluggy only, public API (
scratch_repro.pyin the working copy, not committed)The script writes a scratch module plus a real
*.dist-infowith two[repro]entry points ontosys.path— one loading toNone, one declined by aPluginManagersubclass that mirrors pytest'sregister()— soimportlib.metadatafinds genuine entry points; nothing is monkeypatched.Observed:
load_setuptools_entrypoints()reports 2 loaded plugins, the registry is empty, and listing the distributions crashes withKeyError.Expected: the declined entry points are neither counted nor remembered, so the call returns
0andlist_plugin_distributions()returns[].2. pytest end-to-end
A dummy distribution with a
pytest11entry point namedpytest_faulthandler(one file plus a*.dist-infodirectory insite-packages; the realpytest-faulthandlerpackage declares the same name) and a one-line test file:(1.6.0 / 1.7.0 were run through
PYTHONPATHagainst their released sources:pluggy-1.6.0,pluggy-1.7.0.)Root cause
load_setuptools_entrypoints()assumedregister()always registers, so it updated_name2dist/countunconditionally. That was harmless while the distributions were a list of(plugin, dist)tuples, but #767 keyed them by plugin name, andlist_plugin_distributions()looks the keys up in_name2plugin— where a declined plugin never appears.Fix
The entry point's
ep.load()still happens (it is what discovers the plugin); only the bookkeeping is skipped. The distribution is keyed by the nameregister()reports, which keeps_name2distconsistent with_name2plugineven for a subclass that renames.Behaviour note: such an entry point is no longer counted by
load_setuptools_entrypoints(). Its docstring says "the number of plugins loaded by this call", and nothing was loaded, but if you prefer counting entry points that were imported and then declined, that one line can be split out.Tests
New
testing/test_pluginmanager.py::test_load_setuptools_declined_plugin_is_not_recorded, next to the other entry-point tests, using the local style (fakeEntryPoint/Distribution+monkeypatch.setattr(importlib.metadata, "distributions", ...)); it covers both aNoneplugin and a subclass which declines.RED (test on the base source, fix stashed out —
git checkout HEAD~1 -- src/pluggy/_manager.py):GREEN (with the fix):
Full suite and checks:
The
rsthook (files: ^(HOWTORELEASE.rst|README.rst)$) reported "no files to check"; nothing needed network. The pre-commit environments (ruff 0.17.1, mypy 2.4.0) installed and ran locally.Extra sanity check, a mix of registered and declined entry points in one call:
count == 1,list_name_plugin()is[("normal", <Plugin>)], andlist_plugin_distributions()records exactly the registered plugin's distribution.Changelog
changelog/804.bugfix.rst. Perchangelog/README.rst("If there is no issue, then after you submit the PR and get the PR number you can add a changelog using that instead") the file is named804as a placeholder for the PR number — please rename it to the real one when the PR number is known, or to the issue number if you prefer me to file an issue first.Why this is not a duplicate of #803
#803 (
deprecate-keyword-only-hook-parameters, "Deprecate keyword-only hook parameters", refs #742) changes argument discovery/validation for keyword-only hook parameters insrc/pluggy/_decorators.pyandsrc/pluggy/_manager.py::_verify_hook. This PR is about entry-point/distribution bookkeeping inload_setuptools_entrypoints()/_name2dist/list_plugin_dist*(); it shares no code path, no issue and no test with it. Among the other open pluggy PRs (#772, #775, #777, #778, #779, #783) none touches_name2dist,load_setuptools_entrypointsorlist_plugin_dist*either.Risks / limitations
load_setuptools_entrypoints()now returns a smaller count when a subclass declines a plugin (0 instead of 1 for this case). I believe that matches its docstring; pytest ignores the return value._name2distonly for declined registrations; theunregister()/set_blocked()paths that already popped the entries are untouched.pytest_faulthandlerdistribution; it was removed again, so the venv is clean.AI assistance
This defect, the reproduction, the fix, the test, the changelog fragment and this draft were produced by an AI agent (DeepSeek Harness, model dsv41) working autonomously in a scratch clone; every command output quoted above is from that session, and the diff was reviewed before committing. No human wrote any part of the change.