Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/14135.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Pytest now warns when ``-p`` loads a module with no pytest hooks but a ``pytest11`` entry-point exists in one of its submodules, helping catch wrong plugin names when plugin autoloading is disabled.
41 changes: 40 additions & 1 deletion src/_pytest/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -878,8 +878,9 @@ def import_plugin(self, modname: str, consider_entry_points: bool = False) -> No
importspec = "_pytest." + modname if modname in builtin_plugins else modname
self.rewrite_hook.mark_rewrite(importspec)

loaded = False
if consider_entry_points:
loaded = self.load_setuptools_entrypoints("pytest11", name=modname)
loaded = bool(self.load_setuptools_entrypoints("pytest11", name=modname))
if loaded:
return

Expand All @@ -900,6 +901,44 @@ def import_plugin(self, modname: str, consider_entry_points: bool = False) -> No
self.skipped_plugins.append((modname, e.msg or ""))
else:
self.register(mod, modname)
if consider_entry_points and not loaded:
self._warn_about_submodule_entrypoint_plugin(modname, mod)

def _warn_about_submodule_entrypoint_plugin(
self, modname: str, mod: _PluggyPlugin
) -> None:
if self._plugin_has_pytest_hooks(mod):
return
Comment on lines +910 to +911
Copy link
Member

Choose a reason for hiding this comment

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

Let's inline this:

Suggested change
if self._plugin_has_pytest_hooks(mod):
return
plugin_has_pytest_hooks = any(
self.parse_hookimpl_opts(plugin, attr) is not None for attr in dir(plugin)
)
if plugin_has_pytest_hooks:
return


modname_prefix = f"{modname}."
suggested = {
ep.name
for dist in importlib.metadata.distributions()
for ep in dist.entry_points
if ep.group == "pytest11"
and ep.name != modname
and isinstance(getattr(ep, "value", None), str)
and getattr(ep, "value").split(":", 1)[0].startswith(modname_prefix)
Copy link
Member

Choose a reason for hiding this comment

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

This warrants a comment: examples of what ep.value might be and what we are trying to match with this split() call here with : (the tests use . as separator).

}
Comment on lines +914 to +922
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit dense, can we break it a bit?

ep_values = ...
suggested = ...

if not suggested:
return
Comment on lines +923 to +924
Copy link
Member

Choose a reason for hiding this comment

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

I think we should still warn about the plugin not having hooks, even if we don't find a suitable suggestion.


suggestion = (
f"-p {sorted(suggested)[0]}"
if len(suggested) == 1
else "one of: " + ", ".join(f"-p {name}" for name in sorted(suggested))
)
warnings.warn(
PytestConfigWarning(
f'Plugin "{modname}" contains no pytest hooks. Did you mean to use {suggestion}?'
),
stacklevel=3,
)

def _plugin_has_pytest_hooks(self, plugin: _PluggyPlugin) -> bool:
return any(
self.parse_hookimpl_opts(plugin, attr) is not None for attr in dir(plugin)
)


def _get_plugin_specs_as_list(
Expand Down
193 changes: 193 additions & 0 deletions testing/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
import re
import sys
import textwrap
import types
from typing import Any
import warnings

import _pytest._code
from _pytest.config import _get_plugin_specs_as_list
Expand All @@ -30,6 +32,7 @@
from _pytest.monkeypatch import MonkeyPatch
from _pytest.pathlib import absolutepath
from _pytest.pytester import Pytester
from _pytest.warning_types import PytestConfigWarning
from _pytest.warning_types import PytestDeprecationWarning
import pytest

Expand Down Expand Up @@ -1690,6 +1693,196 @@ def distributions():
# __spec__ is present when testing locally on pypy, but not in CI ????


def test_disable_plugin_autoload_warns_for_submodule_entrypoint(
pytester: Pytester, monkeypatch: MonkeyPatch
) -> None:
class DummyEntryPoint:
project_name = "pytest-recording"
name = "recording"
group = "pytest11"
version = "1.0"
value = "pytest_recording.plugin"

class Distribution:
metadata = {"name": "pytest-recording"}
entry_points = (DummyEntryPoint(),)
files = ()

def distributions():
return (Distribution(),)

top_level_plugin = types.ModuleType("pytest_recording")
submodule_plugin = types.ModuleType("pytest_recording.plugin")
setattr(submodule_plugin, "pytest_addoption", lambda parser: None)

monkeypatch.setattr(importlib.metadata, "distributions", distributions)
monkeypatch.setitem(sys.modules, "pytest_recording", top_level_plugin)
monkeypatch.setitem(sys.modules, "pytest_recording.plugin", submodule_plugin)

with pytest.warns(
PytestConfigWarning,
match=r'Plugin "pytest_recording" contains no pytest hooks\. Did you mean to use -p recording\?',
):
config = pytester.parseconfig(
"--disable-plugin-autoload", "-p", "pytest_recording"
)

assert config.pluginmanager.get_plugin("pytest_recording") is not None


def test_disable_plugin_autoload_does_not_warn_when_module_has_hooks(
pytester: Pytester, monkeypatch: MonkeyPatch
) -> None:
class DummyEntryPoint:
project_name = "pytest-recording"
name = "recording"
group = "pytest11"
version = "1.0"
value = "pytest_recording.plugin"

class Distribution:
metadata = {"name": "pytest-recording"}
entry_points = (DummyEntryPoint(),)
files = ()

def distributions():
return (Distribution(),)

plugin_with_hooks = types.ModuleType("pytest_recording")

def pytest_addoption(parser):
parser.addoption("--block-network")

setattr(plugin_with_hooks, "pytest_addoption", pytest_addoption)

monkeypatch.setattr(importlib.metadata, "distributions", distributions)
monkeypatch.setitem(sys.modules, "pytest_recording", plugin_with_hooks)

with warnings.catch_warnings(record=True) as captured:
warnings.simplefilter("always")
config = pytester.parseconfig(
"--disable-plugin-autoload", "-p", "pytest_recording"
)

assert config.pluginmanager.get_plugin("pytest_recording") is not None
assert not [
w
for w in captured
if isinstance(w.message, PytestConfigWarning)
and "contains no pytest hooks" in str(w.message)
Copy link
Member

Choose a reason for hiding this comment

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

This check is a bit brittle in case we tweak the message later. Could we instead just check that there are no warnings at all?

]


def test_disable_plugin_autoload_does_not_warn_when_no_submodule_entrypoint(
pytester: Pytester, monkeypatch: MonkeyPatch
) -> None:
class DummyEntryPoint:
project_name = "pytest-recording"
name = "recording"
group = "pytest11"
version = "1.0"
value = "other_plugin.plugin"

class Distribution:
metadata = {"name": "pytest-recording"}
entry_points = (DummyEntryPoint(),)
files = ()

def distributions():
return (Distribution(),)

monkeypatch.setattr(importlib.metadata, "distributions", distributions)
monkeypatch.setitem(
sys.modules, "pytest_recording", types.ModuleType("pytest_recording")
)

with warnings.catch_warnings(record=True) as captured:
warnings.simplefilter("always")
config = pytester.parseconfig(
"--disable-plugin-autoload", "-p", "pytest_recording"
)

assert config.pluginmanager.get_plugin("pytest_recording") is not None
assert not [
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as the other test.

w
for w in captured
if isinstance(w.message, PytestConfigWarning)
and "contains no pytest hooks" in str(w.message)
]


def test_disable_plugin_autoload_warns_for_multiple_submodule_entrypoints(
pytester: Pytester, monkeypatch: MonkeyPatch
) -> None:
class DummyEntryPoint:
project_name = "pytest-recording"
group = "pytest11"
version = "1.0"

def __init__(self, name: str, value: str) -> None:
self.name = name
self.value = value

class Distribution:
metadata = {"name": "pytest-recording"}
entry_points = (
DummyEntryPoint("recording", "pytest_recording.plugin"),
DummyEntryPoint("recording_alt", "pytest_recording.alt"),
)
files = ()

def distributions():
return (Distribution(),)

monkeypatch.setattr(importlib.metadata, "distributions", distributions)
monkeypatch.setitem(
sys.modules, "pytest_recording", types.ModuleType("pytest_recording")
)

with pytest.warns(
PytestConfigWarning,
match=(
r'Plugin "pytest_recording" contains no pytest hooks\. '
r"Did you mean to use one of: -p recording, -p recording_alt\?"
),
):
config = pytester.parseconfig(
"--disable-plugin-autoload", "-p", "pytest_recording"
)

assert config.pluginmanager.get_plugin("pytest_recording") is not None


def test_warn_about_submodule_entrypoint_plugin_direct(pytester: Pytester, monkeypatch):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def test_warn_about_submodule_entrypoint_plugin_direct(pytester: Pytester, monkeypatch):
def test_warn_about_submodule_entrypoint_plugin_direct(pytester: Pytester, monkeypatch: MonkeyPatch) -> None:

Copy link
Member

Choose a reason for hiding this comment

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

Not sure I follow the intent behind this test, could you add a docstring please?

class DummyEntryPoint:
group = "pytest11"
version = "1.0"

def __init__(self, name: str, value: str):
self.name = name
self.value = value

class Distribution:
metadata = {"name": "pytest-recording"}
entry_points = (DummyEntryPoint("recording", "pytest_recording.plugin"),)
files = ()

def distributions():
return (Distribution(),)

monkeypatch.setattr(importlib.metadata, "distributions", distributions)
config = pytester.parseconfig()
plugin = types.ModuleType("pytest_recording")

with pytest.warns(
PytestConfigWarning,
match=r'Plugin "pytest_recording" contains no pytest hooks\. Did you mean to use -p recording\?',
):
config.pluginmanager._warn_about_submodule_entrypoint_plugin(
"pytest_recording", plugin
)


def test_plugin_loading_order(pytester: Pytester) -> None:
"""Test order of plugin loading with `-p`."""
p1 = pytester.makepyfile(
Expand Down