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 AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ Gene Wood
George Kussumoto
Georgy Dyuldin
Gergely Kalmár
Giovana Malaia
Gleb Nikonorov
Graeme Smecher
Graham Horler
Expand Down
1 change: 1 addition & 0 deletions changelog/11514.improvement.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The error raised when two plugins register the same command-line option now names the plugins involved when they can be identified, for example ``option names {'--browser'} already added by plugins: ['plugin1', 'plugin2']``, and mentions an unidentified source otherwise.
16 changes: 15 additions & 1 deletion src/_pytest/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,9 @@ def __init__(self) -> None:
# just stored here to be used later.
self.skipped_plugins: list[tuple[str, str]] = []

# Parser receiving the options of plugins as they are registered; set by Config.
self._option_parser: Parser | None = None

self.add_hookspecs(_pytest.hookspec)
self.register(self)
if os.environ.get("PYTEST_DEBUG"):
Expand Down Expand Up @@ -601,7 +604,17 @@ def register(self, plugin: _PluggyPlugin, name: str | None = None) -> str | None
)
)
return None
plugin_name = super().register(plugin, name)
# Nameless plugins without ``__name__`` stay unattributed instead of using
# pluggy's id()-based fallback name.
option_owner = name or getattr(plugin, "__name__", None)
parser = self._option_parser
scope = (
parser._plugin_scope(option_owner)
if parser is not None
else contextlib.nullcontext()
)
with scope:
plugin_name = super().register(plugin, name)
if plugin_name is not None:
self.hook.pytest_plugin_registered.call_historic(
kwargs=dict(
Expand Down Expand Up @@ -1248,6 +1261,7 @@ def __init__(

:type: PytestPluginManager
"""
pluginmanager._option_parser = self._parser

self.stash = Stash()
"""A place where plugins can store information on the config for their
Expand Down
58 changes: 55 additions & 3 deletions src/_pytest/config/argparsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@

import argparse
from collections.abc import Callable
from collections.abc import Iterable
from collections.abc import Iterator
from collections.abc import Sequence
import contextlib
import dataclasses
import os
import re
Expand Down Expand Up @@ -138,6 +141,10 @@ def __init__(
self._groups = [self._anonymous]
# Maps option strings -> dest, e.g. "-V" and "--version" to "version".
self._opt2dest: dict[str, str] = {}
# Maps option strings -> name of the plugin that registered them, None if unknown.
self._opt2plugin: dict[str, str | None] = {}
# Name of the plugin whose options are being registered, None if unknown.
self._current_plugin: str | None = None
file_or_dir_arg = self.optparser.add_argument(FILE_OR_DIR, nargs="*")
file_or_dir_arg.completer = filescompleter # type: ignore

Expand All @@ -158,6 +165,37 @@ def processoption(self, option: Argument) -> None:
if option.dest:
self._processopt(option)

@contextlib.contextmanager
def _plugin_scope(self, plugin_name: str | None) -> Iterator[None]:
"""Attribute the options added within the block to ``plugin_name``."""
previous, self._current_plugin = self._current_plugin, plugin_name
try:
yield
finally:
self._current_plugin = previous

def _conflict_origin(
self, opts: Iterable[str], *, identified_only: bool = False
) -> str | None:
"""Describe who registered ``opts`` and the plugin now conflicting with them.

Returns None if none of ``opts`` is known to this parser, or, with
``identified_only``, if no plugin name is known.
"""
wanted = set(opts)
# Dict order is registration order.
owners = [plugin for opt, plugin in self._opt2plugin.items() if opt in wanted]
if not owners:
return None
current = self._current_plugin
names = list(dict.fromkeys(n for n in (*owners, current) if n is not None))
if not names:
return None if identified_only else "by an unidentified source"
origin = f"by plugins: {names}"
if None in owners or current is None:
origin += " and an unidentified source"
return origin

def getgroup(
self, name: str, description: str = "", after: str | None = None
) -> OptionGroup:
Expand Down Expand Up @@ -455,18 +493,24 @@ def addoption(self, *opts: str, **attrs: Any) -> None:
name for opt in self.options for name in opt.names()
)
if conflict:
raise ValueError(f"option names {conflict} already added")
origin = self.parser._conflict_origin(conflict) if self.parser else None
suffix = f" {origin}" if origin else ""
raise ValueError(f"option names {conflict} already added{suffix}")

if self.parser and "dest" not in attrs:
dest = _get_argparse_dest(opts)
for group in self.parser._groups:
for option in group.options:
if option.dest == dest:
origin = self.parser._conflict_origin(
option.names(), identified_only=True
)
suffix = f" (registered {origin})" if origin else ""
raise ValueError(
f"option dest {dest!r} already used by "
f"{option.names()!r} (this is the option that maps to "
f"dest {dest!r}); pass dest={dest!r} explicitly "
"to share the destination"
f"to share the destination{suffix}"
)
self._addoption_inner(opts, attrs, allow_reserved=False)

Expand All @@ -483,12 +527,20 @@ def _addoption_inner(
if len(opt) >= 2 and opt[0] == "-" and opt[1].islower():
raise ValueError("lowercase short options are reserved")

action = self._arggroup.add_argument(*opts, **attrs)
try:
action = self._arggroup.add_argument(*opts, **attrs)
except argparse.ArgumentError as exc:
origin = self.parser._conflict_origin(opts) if self.parser else None
if origin is not None:
# Keep the original exception (type, argument_name) and extend it.
exc.message = f"{exc.message} added {origin}"
raise
option = Argument(action)
self.options.append(option)
if self.parser:
for name in option.names():
self.parser._opt2dest[name] = option.dest
self.parser._opt2plugin[name] = self.parser._current_plugin
self.parser.processoption(option)


Expand Down
Loading
Loading