Merge pull request #13857 from bluetech/config-cleanups

Config cleanups

Author: bluetech

src/_pytest/config/__init__.py

@@ -10,6 +10,7 @@
 from collections.abc import Generator
 from collections.abc import Iterable
 from collections.abc import Iterator
+from collections.abc import Mapping
 from collections.abc import Sequence
 import contextlib
 import copy
@@ -55,6 +56,7 @@
 from _pytest._io import TerminalWriter
 from _pytest.compat import assert_never
 from _pytest.config.argparsing import Argument
+from _pytest.config.argparsing import FILE_OR_DIR
 from _pytest.config.argparsing import Parser
 import _pytest.deprecated
 import _pytest.hookspec
@@ -290,23 +292,21 @@ def directory_arg(path: str, optname: str) -> str:
 
 
 def get_config(
-    args: list[str] | None = None,
+    args: Iterable[str] | None = None,
     plugins: Sequence[str | _PluggyPlugin] | None = None,
 ) -> Config:
     # Subsequent calls to main will create a fresh instance.
     pluginmanager = PytestPluginManager()
-    config = Config(
-        pluginmanager,
-        invocation_params=Config.InvocationParams(
-            args=args or (),
-            plugins=plugins,
-            dir=pathlib.Path.cwd(),
-        ),
+    invocation_params = Config.InvocationParams(
+        args=args or (),
+        plugins=plugins,
+        dir=pathlib.Path.cwd(),
     )
+    config = Config(pluginmanager, invocation_params=invocation_params)
 
-    if args is not None:
+    if invocation_params.args:
         # Handle any "-p no:plugin" args.
-        pluginmanager.consider_preparse(args, exclude_only=True)
+        pluginmanager.consider_preparse(invocation_params.args, exclude_only=True)
 
     for spec in default_plugins:
         pluginmanager.import_plugin(spec)
@@ -1202,7 +1202,7 @@ def cwd_relative_nodeid(self, nodeid: str) -> str:
         return nodeid
 
     @classmethod
-    def fromdictargs(cls, option_dict, args) -> Config:
+    def fromdictargs(cls, option_dict: Mapping[str, Any], args: list[str]) -> Config:
         """Constructor usable for subprocesses."""
         config = get_config(args)
         config.option.__dict__.update(option_dict)
@@ -1246,35 +1246,6 @@ def pytest_load_initial_conftests(self, early_config: Config) -> None:
             ),
         )
 
-    def _initini(self, args: Sequence[str]) -> None:
-        ns, unknown_args = self._parser.parse_known_and_unknown_args(
-            args, namespace=copy.copy(self.option)
-        )
-        rootpath, inipath, inicfg, ignored_config_files = determine_setup(
-            inifile=ns.inifilename,
-            override_ini=ns.override_ini,
-            args=ns.file_or_dir + unknown_args,
-            rootdir_cmd_arg=ns.rootdir or None,
-            invocation_dir=self.invocation_params.dir,
-        )
-        self._rootpath = rootpath
-        self._inipath = inipath
-        self._ignored_config_files = ignored_config_files
-        self.inicfg = inicfg
-        self._parser.extra_info["rootdir"] = str(self.rootpath)
-        self._parser.extra_info["inifile"] = str(self.inipath)
-        self._parser.addini("addopts", "Extra command line options", "args")
-        self._parser.addini("minversion", "Minimally required pytest version")
-        self._parser.addini(
-            "pythonpath", type="paths", help="Add paths to sys.path", default=[]
-        )
-        self._parser.addini(
-            "required_plugins",
-            "Plugins that must be present for pytest to run",
-            type="args",
-            default=[],
-        )
-
     def _consider_importhook(self, args: Sequence[str]) -> None:
         """Install the PEP 302 import hook if using assertion rewriting.
 
@@ -1336,13 +1307,13 @@ def _unconfigure_python_path(self) -> None:
 
     def _validate_args(self, args: list[str], via: str) -> list[str]:
         """Validate known args."""
-        self._parser._config_source_hint = via  # type: ignore
+        self._parser.extra_info["config source"] = via
         try:
             self._parser.parse_known_and_unknown_args(
                 args, namespace=copy.copy(self.option)
             )
         finally:
-            del self._parser._config_source_hint  # type: ignore
+            self._parser.extra_info.pop("config source", None)
 
         return args
 
@@ -1399,7 +1370,35 @@ def _preparse(self, args: list[str], addopts: bool = True) -> None:
                     self._validate_args(shlex.split(env_addopts), "via PYTEST_ADDOPTS")
                     + args
                 )
-        self._initini(args)
+
+        ns, unknown_args = self._parser.parse_known_and_unknown_args(
+            args, namespace=copy.copy(self.option)
+        )
+        rootpath, inipath, inicfg, ignored_config_files = determine_setup(
+            inifile=ns.inifilename,
+            override_ini=ns.override_ini,
+            args=ns.file_or_dir + unknown_args,
+            rootdir_cmd_arg=ns.rootdir or None,
+            invocation_dir=self.invocation_params.dir,
+        )
+        self._rootpath = rootpath
+        self._inipath = inipath
+        self._ignored_config_files = ignored_config_files
+        self.inicfg = inicfg
+        self._parser.extra_info["rootdir"] = str(self.rootpath)
+        self._parser.extra_info["inifile"] = str(self.inipath)
+        self._parser.addini("addopts", "Extra command line options", "args")
+        self._parser.addini("minversion", "Minimally required pytest version")
+        self._parser.addini(
+            "pythonpath", type="paths", help="Add paths to sys.path", default=[]
+        )
+        self._parser.addini(
+            "required_plugins",
+            "Plugins that must be present for pytest to run",
+            type="args",
+            default=[],
+        )
+
         if addopts:
             args[:] = (
                 self._validate_args(self.getini("addopts"), "via addopts config") + args
@@ -1540,19 +1539,17 @@ def parse(self, args: list[str], addopts: bool = True) -> None:
         self._preparse(args, addopts=addopts)
         self._parser.after_preparse = True  # type: ignore
         try:
-            args = self._parser.parse_setoption(
-                args, self.option, namespace=self.option
-            )
-            self.args, self.args_source = self._decide_args(
-                args=args,
-                pyargs=self.known_args_namespace.pyargs,
-                testpaths=self.getini("testpaths"),
-                invocation_dir=self.invocation_params.dir,
-                rootpath=self.rootpath,
-                warn=True,
-            )
+            parsed = self._parser.parse(args, namespace=self.option)
         except PrintHelp:
-            pass
+            return
+        self.args, self.args_source = self._decide_args(
+            args=getattr(parsed, FILE_OR_DIR),
+            pyargs=self.known_args_namespace.pyargs,
+            testpaths=self.getini("testpaths"),
+            invocation_dir=self.invocation_params.dir,
+            rootpath=self.rootpath,
+            warn=True,
+        )
 
     def issue_config_time_warning(self, warning: Warning, stacklevel: int) -> None:
         """Issue and handle a warning during the "configure" stage.

src/_pytest/config/argparsing.py

@@ -7,7 +7,6 @@
 from collections.abc import Sequence
 import os
 from typing import Any
-from typing import cast
 from typing import final
 from typing import Literal
 from typing import NoReturn
@@ -112,12 +111,12 @@ def parse(
         self.optparser = self._getparser()
         try_argcomplete(self.optparser)
         strargs = [os.fspath(x) for x in args]
-        return self.optparser.parse_args(strargs, namespace=namespace)
+        return self.optparser.parse_intermixed_args(strargs, namespace=namespace)
 
-    def _getparser(self) -> MyOptionParser:
+    def _getparser(self) -> PytestArgumentParser:
         from _pytest._argcomplete import filescompleter
 
-        optparser = MyOptionParser(self, self.extra_info, prog=self.prog)
+        optparser = PytestArgumentParser(self, self.extra_info, prog=self.prog)
         groups = [*self._groups, self._anonymous]
         for group in groups:
             if group.options:
@@ -133,17 +132,6 @@ def _getparser(self) -> MyOptionParser:
         file_or_dir_arg.completer = filescompleter  # type: ignore
         return optparser
 
-    def parse_setoption(
-        self,
-        args: Sequence[str | os.PathLike[str]],
-        option: argparse.Namespace,
-        namespace: argparse.Namespace | None = None,
-    ) -> list[str]:
-        parsedoption = self.parse(args, namespace=namespace)
-        for name, value in parsedoption.__dict__.items():
-            setattr(option, name, value)
-        return cast(list[str], getattr(parsedoption, FILE_OR_DIR))
-
     def parse_known_args(
         self,
         args: Sequence[str | os.PathLike[str]],
@@ -331,9 +319,7 @@ def names(self) -> list[str]:
 
     def attrs(self) -> Mapping[str, Any]:
         # Update any attributes set by processopt.
-        attrs = "default dest help".split()
-        attrs.append(self.dest)
-        for attr in attrs:
+        for attr in ("default", "dest", "help", self.dest):
             try:
                 self._attrs[attr] = getattr(self, attr)
             except AttributeError:
@@ -436,7 +422,7 @@ def _addoption_instance(self, option: Argument, shortupper: bool = False) -> Non
         self.options.append(option)
 
 
-class MyOptionParser(argparse.ArgumentParser):
+class PytestArgumentParser(argparse.ArgumentParser):
     def __init__(
         self,
         parser: Parser,
@@ -459,32 +445,12 @@ def __init__(
     def error(self, message: str) -> NoReturn:
         """Transform argparse error message into UsageError."""
         msg = f"{self.prog}: error: {message}"
-
-        if hasattr(self._parser, "_config_source_hint"):
-            msg = f"{msg} ({self._parser._config_source_hint})"
-
+        if self.extra_info:
+            msg += "\n" + "\n".join(
+                f"  {k}: {v}" for k, v in sorted(self.extra_info.items())
+            )
         raise UsageError(self.format_usage() + msg)
 
-    # Type ignored because typeshed has a very complex type in the superclass.
-    def parse_args(  # type: ignore
-        self,
-        args: Sequence[str] | None = None,
-        namespace: argparse.Namespace | None = None,
-    ) -> argparse.Namespace:
-        """Allow splitting of positional arguments."""
-        parsed, unrecognized = self.parse_known_args(args, namespace)
-        if unrecognized:
-            for arg in unrecognized:
-                if arg and arg[0] == "-":
-                    lines = [
-                        "unrecognized arguments: {}".format(" ".join(unrecognized))
-                    ]
-                    for k, v in sorted(self.extra_info.items()):
-                        lines.append(f"  {k}: {v}")
-                    self.error("\n".join(lines))
-            getattr(parsed, FILE_OR_DIR).extend(unrecognized)
-        return parsed
-
 
 class DropShorterLongHelpFormatter(argparse.HelpFormatter):
     """Shorten help for long options that differ only in extra hyphens.

src/_pytest/helpconfig.py

@@ -3,20 +3,23 @@
 
 from __future__ import annotations
 
-from argparse import Action
+import argparse
 from collections.abc import Generator
+from collections.abc import Sequence
 import os
 import sys
+from typing import Any
 
 from _pytest.config import Config
 from _pytest.config import ExitCode
 from _pytest.config import PrintHelp
 from _pytest.config.argparsing import Parser
+from _pytest.config.argparsing import PytestArgumentParser
 from _pytest.terminal import TerminalReporter
 import pytest
 
 
-class HelpAction(Action):
+class HelpAction(argparse.Action):
     """An argparse Action that will raise an exception in order to skip the
     rest of the argument parsing when --help is passed.
 
@@ -26,20 +29,29 @@ class HelpAction(Action):
     implemented by raising SystemExit.
     """
 
-    def __init__(self, option_strings, dest=None, default=False, help=None):
+    def __init__(
+        self, option_strings: Sequence[str], dest: str, *, help: str | None = None
+    ) -> None:
         super().__init__(
             option_strings=option_strings,
             dest=dest,
-            const=True,
-            default=default,
             nargs=0,
+            const=True,
+            default=False,
             help=help,
         )
 
-    def __call__(self, parser, namespace, values, option_string=None):
+    def __call__(
+        self,
+        parser: argparse.ArgumentParser,
+        namespace: argparse.Namespace,
+        values: str | Sequence[Any] | None,
+        option_string: str | None = None,
+    ) -> None:
         setattr(namespace, self.dest, self.const)
 
         # We should only skip the rest of the parsing after preparse is done.
+        assert isinstance(parser, PytestArgumentParser)
         if getattr(parser._parser, "after_preparse", False):
             raise PrintHelp
 
@@ -245,9 +257,6 @@ def showhelp(config: Config) -> None:
         tw.line("warning : " + warningreport.message, red=True)
 
 
-conftest_options = [("pytest_plugins", "list of plugin names to load")]
-
-
 def getpluginversioninfo(config: Config) -> list[str]:
     lines = []
     plugininfo = config.pluginmanager.list_plugin_distinfo()

testing/test_config.py

@@ -1425,7 +1425,7 @@ def test_inifilename(self, tmp_path: Path) -> None:
         )
         with MonkeyPatch.context() as mp:
             mp.chdir(cwd)
-            config = Config.fromdictargs(option_dict, ())
+            config = Config.fromdictargs(option_dict, [])
             inipath = absolutepath(inifilename)
 
         assert config.args == [str(cwd)]
@@ -2290,9 +2290,10 @@ def test_addopts_from_env_not_concatenated(
         with pytest.raises(UsageError) as excinfo:
             config._preparse(["cache_dir=ignored"], addopts=True)
         assert (
-            "error: argument -o/--override-ini: expected one argument (via PYTEST_ADDOPTS)"
+            "error: argument -o/--override-ini: expected one argument"
             in excinfo.value.args[0]
         )
+        assert "via PYTEST_ADDOPTS" in excinfo.value.args[0]
 
     def test_addopts_from_ini_not_concatenated(self, pytester: Pytester) -> None:
         """`addopts` from configuration should not take values from normal args (#4265)."""
@@ -2303,10 +2304,11 @@ def test_addopts_from_ini_not_concatenated(self, pytester: Pytester) -> None:
         """
         )
         result = pytester.runpytest("cache_dir=ignored")
+        config = pytester._request.config
         result.stderr.fnmatch_lines(
             [
-                f"{pytester._request.config._parser.optparser.prog}: error: "
-                f"argument -o/--override-ini: expected one argument (via addopts config)"
+                f"{config._parser.optparser.prog}: error: argument -o/--override-ini: expected one argument",
+                "  config source: via addopts config",
             ]
         )
         assert result.ret == _pytest.config.ExitCode.USAGE_ERROR