Skip to content

fix: don't count or record entry points that register() declined - #804

Closed
simpleqt wants to merge 1 commit into
pytest-dev:mainfrom
simpleqt:fix-declined-entrypoint-distinfo
Closed

simpleqt wants to merge 1 commit into
pytest-dev:mainfrom
simpleqt:fix-declined-entrypoint-distinfo

Conversation

@simpleqt

Copy link
Copy Markdown

Problem

PluginManager.load_setuptools_entrypoints() counted every entry point it found as a loaded plugin and remembered its distribution, without looking at what register() did:

                plugin = ep.load()
                self.register(plugin, name=ep.name)
                self._name2dist[ep.name] = dist
                count += 1

register() registers nothing and returns None in two cases that reach this code:

  • the entry point loads to None (register(None) is ignored since fix: reject PluginManager.register(None) #731), and
  • a PluginManager subclass declines the plugin. pytest's PytestPluginManager.register() does exactly that for the plugin names in DEPRECATED_EXTERNAL_PLUGINS (pytest_faulthandler, pytest_catchlog, pytest_capturelog, pytest_subtests): it warns "plugin has been merged into the core" and returns None.

Since #767, the recorded distributions are keyed by plugin name and list_plugin_distributions() resolves those names in the plugin registry:

    def list_plugin_distributions(self):
        return [
            (self._name2plugin[name], dist) for name, dist in self._name2dist.items()
        ]

so a declined-but-recorded entry point makes list_plugin_distributions() — and list_plugin_distinfo(), which calls it — raise KeyError.

Real world impact: TerminalReporter.pytest_report_header() renders the plugins: line through config.pluginmanager.list_plugin_distinfo(). With the (still common) pytest-faulthandler package installed, every pytest run aborts with

INTERNALERROR>   File ".../pluggy/_manager.py", line 551, in list_plugin_distributions
INTERNALERROR>     (self._name2plugin[name], dist) for name, dist in self._name2dist.items()
INTERNALERROR> KeyError: 'pytest_faulthandler'

Config._validate_plugins() (required_plugins ini) and --trace-config / pytest --help reach the same code.

Reproduction

1. pluggy only, public API (scratch_repro.py in the working copy, not committed)

The script writes a scratch module plus a real *.dist-info with two [repro] entry points onto sys.path — one loading to None, one declined by a PluginManager subclass that mirrors pytest's register() — so importlib.metadata finds genuine entry points; nothing is monkeypatched.

$ python scratch_repro.py
entry points visible to importlib.metadata: ['repro_declined', 'repro_null']
load_setuptools_entrypoints() returned: 2
plugins actually registered:         []
distributions recorded: {'repro_null': <importlib.metadata.PathDistribution object at 0x00000192E1F55150>, 'repro_declined': <importlib.metadata.PathDistribution object at 0x00000192E1F55150>}
list_plugin_distributions():
Traceback (most recent call last):
  File "G:\pr\work\pg2\scratch_repro.py", line 92, in <module>
    main()
  File "G:\pr\work\pg2\scratch_repro.py", line 88, in main
    print(pm.list_plugin_distributions())
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "G:\pr\work\pg2\src\pluggy\_manager.py", line 550, in list_plugin_distributions
    return [
           ^
  File "G:\pr\work\pg2\src\pluggy\_manager.py", line 551, in <listcomp>
    (self._name2plugin[name], dist) for name, dist in self._name2dist.items()
     ~~~~~~~~~~~~~~~~~^^^^^^
KeyError: 'repro_null'

Observed: load_setuptools_entrypoints() reports 2 loaded plugins, the registry is empty, and listing the distributions crashes with KeyError.
Expected: the declined entry points are neither counted nor remembered, so the call returns 0 and list_plugin_distributions() returns [].

2. pytest end-to-end

A dummy distribution with a pytest11 entry point named pytest_faulthandler (one file plus a *.dist-info directory in site-packages; the real pytest-faulthandler package declares the same name) and a one-line test file:

$ python -m pytest -p no:cacheprovider            # pluggy main / base
INTERNALERROR>   File "...\pluggy\_manager.py", line 551, in list_plugin_distributions
INTERNALERROR>     (self._name2plugin[name], dist) for name, dist in self._name2dist.items()
INTERNALERROR> KeyError: 'pytest_faulthandler'
[exit code 3]

$ python -m pytest -p no:cacheprovider            # pluggy 1.6.0 and 1.7.0, unchanged
... plugins: platformdirs-4.13.5, benchmark-5.3.0, faulthandler-1.0
1 passed

$ python -m pytest -p no:cacheprovider            # with this patch
... plugins: platformdirs-4.13.5, benchmark-5.3.0
1 passed

(1.6.0 / 1.7.0 were run through PYTHONPATH against their released sources: pluggy-1.6.0, pluggy-1.7.0.)

Root cause

load_setuptools_entrypoints() assumed register() always registers, so it updated _name2dist/count unconditionally. That was harmless while the distributions were a list of (plugin, dist) tuples, but #767 keyed them by plugin name, and list_plugin_distributions() looks the keys up in _name2plugin — where a declined plugin never appears.

Fix

                plugin = ep.load()
                registered_name = self.register(plugin, name=ep.name)
                if registered_name is None:
                    # register() declined to register this plugin: it returns
                    # None for a None plugin, and a PluginManager subclass may
                    # do the same (pytest's ignores deprecated plugin names).
                    # Nothing was registered, so nothing may be counted or
                    # remembered -- list_plugin_distributions() looks the names
                    # up in the plugin registry.
                    continue
                self._name2dist[registered_name] = dist
                count += 1

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 name register() reports, which keeps _name2dist consistent with _name2plugin even 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 (fake EntryPoint/Distribution + monkeypatch.setattr(importlib.metadata, "distributions", ...)); it covers both a None plugin and a subclass which declines.

RED (test on the base source, fix stashed out — git checkout HEAD~1 -- src/pluggy/_manager.py):

$ python -m pytest "testing/test_pluginmanager.py::test_load_setuptools_declined_plugin_is_not_recorded" -q
        pm = DecliningPluginManager("example")
>       assert pm.load_setuptools_entrypoints("hello") == 0
E       AssertionError: assert 2 == 0
E        +  where 2 = load_setuptools_entrypoints('hello')
E        +    where load_setuptools_entrypoints = <test_pluginmanager.test_load_setuptools_declined_plugin_is_not_recorded.<locals>.DecliningPluginManager object at 0x000001457C983610>.load_setuptools_entrypoints

testing\test_pluginmanager.py:1033: AssertionError
=========================== short test summary info ============================
FAILED testing/test_pluginmanager.py::test_load_setuptools_declined_plugin_is_not_recorded
1 failed in 0.10s

GREEN (with the fix):

$ python -m pytest "testing/test_pluginmanager.py::test_load_setuptools_declined_plugin_is_not_recorded" -q
.                                                                        [100%]
1 passed in 0.02s

Full suite and checks:

$ python -m pytest testing -q
226 passed in 0.32s          # 225 before this PR

$ python -m coverage run --branch -m pytest testing -q && python -m coverage report -m --include="src/pluggy/*"
src\pluggy\_manager.py        253      0    102      0   100%     # new branch covered

$ pre-commit run --files src/pluggy/_manager.py testing/test_pluginmanager.py changelog/804.bugfix.rst
ruff check...............................................................Passed
ruff format..............................................................Passed
autoflake................................................................Passed
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
rst ``code`` is two backticks............................................Passed
pyupgrade................................................................Passed
blacken-docs.............................................................Passed
rst..................................................(no files to check)Skipped
mypy.....................................................................Passed

The rst hook (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>)], and list_plugin_distributions() records exactly the registered plugin's distribution.

Changelog

changelog/804.bugfix.rst. Per changelog/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 named 804 as 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 in src/pluggy/_decorators.py and src/pluggy/_manager.py::_verify_hook. This PR is about entry-point/distribution bookkeeping in load_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_entrypoints or list_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.
  • The fix skips _name2dist only for declined registrations; the unregister()/set_blocked() paths that already popped the entries are untouched.
  • Verified on Windows, CPython 3.11, pytest 9.1.1. The end-to-end pytest check needs the dummy pytest_faulthandler distribution; 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.

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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants