config: use one ArgumentParser instead of creating new one for each parse

Previously, `Parser` would create a new `ArgumentParser` for each
parsing call (of which there are several during pytest initialization).

Besides being a bit wasteful, it makes things hard to follow, especially
since one `ArgumentParser` was saved on the `Parser` (in `optparser`).
It makes a few refactorings I am planning difficult to pull off.

So now, `Parser` instantiates one `ArgumentParser` in `__init__` and
just adds groups/arguments to it immediately. There's more
synchronization needed, but since arguments can only be added, never
removed, it's not bad.

One gotcha, in order to control `--help` group orders we must access
argparse internals since it doesn't provide a way to do it. I checked
and the relevant code hasn't changed since argparse was introduced 15
years ago. So hopefully it will continue to be fine.

Author: bluetech

src/_pytest/config/__init__.py

@@ -1157,7 +1157,7 @@ def pytest_cmdline_parse(
             elif (
                 getattr(self.option, "help", False) or "--help" in args or "-h" in args
             ):
-                self._parser._getparser().print_help()
+                self._parser.optparser.print_help()
                 sys.stdout.write(
                     "\nNOTE: displaying only minimal help due to UsageError.\n\n"
                 )

src/_pytest/config/argparsing.py

@@ -36,8 +36,6 @@ class Parser:
         there's an error processing the command line arguments.
     """
 
-    prog: str | None = None
-
     def __init__(
         self,
         usage: str | None = None,
@@ -46,14 +44,31 @@ def __init__(
         _ispytest: bool = False,
     ) -> None:
         check_ispytest(_ispytest)
-        self._anonymous = OptionGroup("Custom options", parser=self, _ispytest=True)
-        self._groups: list[OptionGroup] = []
+
+        from _pytest._argcomplete import filescompleter
+
         self._processopt = processopt
-        self._usage = usage
+        self.extra_info: dict[str, Any] = {}
+        self.optparser = PytestArgumentParser(self, usage, self.extra_info)
+        anonymous_arggroup = self.optparser.add_argument_group("Custom options")
+        self._anonymous = OptionGroup(
+            anonymous_arggroup, "_anonymous", self, _ispytest=True
+        )
+        self._groups = [self._anonymous]
+        file_or_dir_arg = self.optparser.add_argument(FILE_OR_DIR, nargs="*")
+        file_or_dir_arg.completer = filescompleter  # type: ignore
+
         self._inidict: dict[str, tuple[str, str, Any]] = {}
         # Maps alias -> canonical name.
         self._ini_aliases: dict[str, str] = {}
-        self.extra_info: dict[str, Any] = {}
+
+    @property
+    def prog(self) -> str:
+        return self.optparser.prog
+
+    @prog.setter
+    def prog(self, value: str) -> None:
+        self.optparser.prog = value
 
     def processoption(self, option: Argument) -> None:
         if self._processopt:
@@ -78,12 +93,17 @@ def getgroup(
         for group in self._groups:
             if group.name == name:
                 return group
-        group = OptionGroup(name, description, parser=self, _ispytest=True)
+
+        arggroup = self.optparser.add_argument_group(description or name)
+        group = OptionGroup(arggroup, name, self, _ispytest=True)
         i = 0
         for i, grp in enumerate(self._groups):
             if grp.name == after:
                 break
         self._groups.insert(i + 1, group)
+        # argparse doesn't provide a way to control `--help` order, so must
+        # access its internals ☹.
+        self.optparser._action_groups.insert(i + 1, self.optparser._action_groups.pop())
         return group
 
     def addoption(self, *opts: str, **attrs: Any) -> None:
@@ -102,25 +122,6 @@ def addoption(self, *opts: str, **attrs: Any) -> None:
         """
         self._anonymous.addoption(*opts, **attrs)
 
-    def _getparser(self) -> PytestArgumentParser:
-        from _pytest._argcomplete import filescompleter
-
-        optparser = PytestArgumentParser(self, self.extra_info, prog=self.prog)
-        groups = [*self._groups, self._anonymous]
-        for group in groups:
-            if group.options:
-                desc = group.description or group.name
-                arggroup = optparser.add_argument_group(desc)
-                for option in group.options:
-                    n = option.names()
-                    a = option.attrs()
-                    arggroup.add_argument(*n, **a)
-        file_or_dir_arg = optparser.add_argument(FILE_OR_DIR, nargs="*")
-        # bash like autocompletion for dirs (appending '/')
-        # Type ignored because typeshed doesn't know about argcomplete.
-        file_or_dir_arg.completer = filescompleter  # type: ignore
-        return optparser
-
     def parse(
         self,
         args: Sequence[str | os.PathLike[str]],
@@ -135,7 +136,6 @@ def parse(
         """
         from _pytest._argcomplete import try_argcomplete
 
-        self.optparser = self._getparser()
         try_argcomplete(self.optparser)
         strargs = [os.fspath(x) for x in args]
         return self.optparser.parse_intermixed_args(strargs, namespace=namespace)
@@ -163,13 +163,12 @@ def parse_known_and_unknown_args(
             A tuple containing an argparse namespace object for the known
             arguments, and a list of unknown flag arguments.
         """
-        optparser = self._getparser()
         strargs = [os.fspath(x) for x in args]
         # Workaround `UserWarning: Do not expect file_or_dir in Namespace` in older argparse.
         if sys.version_info < (3, 12):
             if namespace is not None and getattr(namespace, FILE_OR_DIR, None) == []:
                 delattr(namespace, FILE_OR_DIR)
-        return optparser.parse_known_intermixed_args(strargs, namespace=namespace)
+        return self.optparser.parse_known_intermixed_args(strargs, namespace=namespace)
 
     def addini(
         self,
@@ -386,15 +385,14 @@ class OptionGroup:
 
     def __init__(
         self,
+        arggroup: argparse._ArgumentGroup,
         name: str,
-        description: str = "",
-        parser: Parser | None = None,
-        *,
+        parser: Parser | None,
         _ispytest: bool = False,
     ) -> None:
         check_ispytest(_ispytest)
+        self._arggroup = arggroup
         self.name = name
-        self.description = description
         self.options: list[Argument] = []
         self.parser = parser
 
@@ -429,30 +427,32 @@ def _addoption_instance(self, option: Argument, shortupper: bool = False) -> Non
             for opt in option._short_opts:
                 if opt[0] == "-" and opt[1].islower():
                     raise ValueError("lowercase shortoptions reserved")
+
         if self.parser:
             self.parser.processoption(option)
+
+        self._arggroup.add_argument(*option.names(), **option.attrs())
         self.options.append(option)
 
 
 class PytestArgumentParser(argparse.ArgumentParser):
     def __init__(
         self,
         parser: Parser,
-        extra_info: dict[str, Any] | None = None,
-        prog: str | None = None,
+        usage: str | None,
+        extra_info: dict[str, str],
     ) -> None:
         self._parser = parser
         super().__init__(
-            prog=prog,
-            usage=parser._usage,
+            usage=usage,
             add_help=False,
             formatter_class=DropShorterLongHelpFormatter,
             allow_abbrev=False,
             fromfile_prefix_chars="@",
         )
         # extra_info is a dict of (param -> value) to display if there's
         # an usage error to provide more contextual information to the user.
-        self.extra_info = extra_info if extra_info else {}
+        self.extra_info = extra_info
 
     def error(self, message: str) -> NoReturn:
         """Transform argparse error message into UsageError."""

testing/test_config.py

@@ -2304,10 +2304,9 @@ 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"{config._parser.optparser.prog}: error: argument -o/--override-ini: expected one argument",
+                "*: error: argument -o/--override-ini: expected one argument",
                 "  config source: via addopts config",
             ]
         )
@@ -2410,8 +2409,7 @@ def pytest_addoption(parser):
     result.stderr.fnmatch_lines(
         [
             "ERROR: usage: *",
-            f"{pytester._request.config._parser.optparser.prog}: error: "
-            f"argument --invalid-option-should-allow-for-help: expected one argument",
+            "*: error: argument --invalid-option-should-allow-for-help: expected one argument",
         ]
     )
     # Does not display full/default help.

testing/test_parseopt.py

@@ -28,9 +28,9 @@ def test_no_help_by_default(self) -> None:
 
     def test_custom_prog(self, parser: parseopt.Parser) -> None:
         """Custom prog can be set for `argparse.ArgumentParser`."""
-        assert parser._getparser().prog == argparse.ArgumentParser().prog
+        assert parser.optparser.prog == argparse.ArgumentParser().prog
         parser.prog = "custom-prog"
-        assert parser._getparser().prog == "custom-prog"
+        assert parser.optparser.prog == "custom-prog"
 
     def test_argument(self) -> None:
         with pytest.raises(parseopt.ArgumentError):
@@ -71,14 +71,12 @@ def test_argument_processopt(self) -> None:
         assert res["dest"] == "abc"
 
     def test_group_add_and_get(self, parser: parseopt.Parser) -> None:
-        group = parser.getgroup("hello", description="desc")
+        group = parser.getgroup("hello")
         assert group.name == "hello"
-        assert group.description == "desc"
 
     def test_getgroup_simple(self, parser: parseopt.Parser) -> None:
-        group = parser.getgroup("hello", description="desc")
+        group = parser.getgroup("hello")
         assert group.name == "hello"
-        assert group.description == "desc"
         group2 = parser.getgroup("hello")
         assert group2 is group
 
@@ -88,16 +86,20 @@ def test_group_ordering(self, parser: parseopt.Parser) -> None:
         parser.getgroup("3", after="1")
         groups = parser._groups
         groups_names = [x.name for x in groups]
-        assert groups_names == list("132")
+        assert groups_names == ["_anonymous", "1", "3", "2"]
 
     def test_group_addoption(self) -> None:
-        group = parseopt.OptionGroup("hello", _ispytest=True)
+        optparser = argparse.ArgumentParser()
+        arggroup = optparser.add_argument_group("hello")
+        group = parseopt.OptionGroup(arggroup, "hello", None, _ispytest=True)
         group.addoption("--option1", action="store_true")
         assert len(group.options) == 1
         assert isinstance(group.options[0], parseopt.Argument)
 
     def test_group_addoption_conflict(self) -> None:
-        group = parseopt.OptionGroup("hello again", _ispytest=True)
+        optparser = argparse.ArgumentParser()
+        arggroup = optparser.add_argument_group("hello again")
+        group = parseopt.OptionGroup(arggroup, "hello again", None, _ispytest=True)
         group.addoption("--option1", "--option-1", action="store_true")
         with pytest.raises(ValueError) as err:
             group.addoption("--option1", "--option-one", action="store_true")