Test command line argument parsing (sphinx-doc#12795)

AA-Turner · web-flow · commit 334e69fbb40e · 2024-08-17T02:53:48.000+01:00
diff --git a/sphinx/cmd/build.py b/sphinx/cmd/build.py
@@ -210,6 +210,9 @@ def get_parser() -> argparse.ArgumentParser:
                        dest='exception_on_warning',
                        help=__('raise an exception on warnings'))
 
+    if parser.prog == '__main__.py':
+        parser.prog = 'sphinx-build'
+
     return parser
 
 
@@ -386,7 +389,8 @@ def main(argv: Sequence[str] = (), /) -> int:
     if argv[:1] == ['--bug-report']:
         return _bug_report_info()
     if argv[:1] == ['-M']:
-        return make_main(argv)
+        from sphinx.cmd import make_mode
+        return make_mode.run_make_mode(argv[1:])
     else:
         return build_main(argv)
 
diff --git a/sphinx/cmd/make_mode.py b/sphinx/cmd/make_mode.py
@@ -58,31 +58,31 @@
 
 
 class Make:
-    def __init__(self, srcdir: str, builddir: str, opts: Sequence[str]) -> None:
-        self.srcdir = srcdir
-        self.builddir = builddir
+    def __init__(self, *, source_dir: str, build_dir: str, opts: Sequence[str]) -> None:
+        self.source_dir = source_dir
+        self.build_dir = build_dir
         self.opts = [*opts]
 
-    def builddir_join(self, *comps: str) -> str:
-        return path.join(self.builddir, *comps)
+    def build_dir_join(self, *comps: str) -> str:
+        return path.join(self.build_dir, *comps)
 
     def build_clean(self) -> int:
-        srcdir = path.abspath(self.srcdir)
-        builddir = path.abspath(self.builddir)
-        if not path.exists(self.builddir):
+        source_dir = path.abspath(self.source_dir)
+        build_dir = path.abspath(self.build_dir)
+        if not path.exists(self.build_dir):
             return 0
-        elif not path.isdir(self.builddir):
-            print("Error: %r is not a directory!" % self.builddir)
+        elif not path.isdir(self.build_dir):
+            print("Error: %r is not a directory!" % self.build_dir)
             return 1
-        elif srcdir == builddir:
-            print("Error: %r is same as source directory!" % self.builddir)
+        elif source_dir == build_dir:
+            print("Error: %r is same as source directory!" % self.build_dir)
             return 1
-        elif path.commonpath([srcdir, builddir]) == builddir:
-            print("Error: %r directory contains source directory!" % self.builddir)
+        elif path.commonpath([source_dir, build_dir]) == build_dir:
+            print("Error: %r directory contains source directory!" % self.build_dir)
             return 1
-        print("Removing everything under %r..." % self.builddir)
-        for item in os.listdir(self.builddir):
-            rmtree(self.builddir_join(item))
+        print("Removing everything under %r..." % self.build_dir)
+        for item in os.listdir(self.build_dir):
+            rmtree(self.build_dir_join(item))
         return 0
 
     def build_help(self) -> None:
@@ -105,7 +105,7 @@ def build_latexpdf(self) -> int:
         if not makecmd.lower().startswith('make'):
             raise RuntimeError('Invalid $MAKE command: %r' % makecmd)
         try:
-            with chdir(self.builddir_join('latex')):
+            with chdir(self.build_dir_join('latex')):
                 if '-Q' in self.opts:
                     with open('__LATEXSTDOUT__', 'w') as outfile:
                         returncode = subprocess.call([makecmd,
@@ -117,7 +117,7 @@ def build_latexpdf(self) -> int:
                                                      )
                     if returncode:
                         print('Latex error: check %s' %
-                              self.builddir_join('latex', '__LATEXSTDOUT__')
+                              self.build_dir_join('latex', '__LATEXSTDOUT__')
                               )
                 elif '-q' in self.opts:
                     returncode = subprocess.call(
@@ -129,7 +129,7 @@ def build_latexpdf(self) -> int:
                     )
                     if returncode:
                         print('Latex error: check .log file in %s' %
-                              self.builddir_join('latex')
+                              self.build_dir_join('latex')
                               )
                 else:
                     returncode = subprocess.call([makecmd, 'all-pdf'])
@@ -148,7 +148,7 @@ def build_latexpdfja(self) -> int:
         if not makecmd.lower().startswith('make'):
             raise RuntimeError('Invalid $MAKE command: %r' % makecmd)
         try:
-            with chdir(self.builddir_join('latex')):
+            with chdir(self.build_dir_join('latex')):
                 return subprocess.call([makecmd, 'all-pdf'])
         except OSError:
             print('Error: Failed to run: %s' % makecmd)
@@ -163,41 +163,44 @@ def build_info(self) -> int:
         if not makecmd.lower().startswith('make'):
             raise RuntimeError('Invalid $MAKE command: %r' % makecmd)
         try:
-            with chdir(self.builddir_join('texinfo')):
+            with chdir(self.build_dir_join('texinfo')):
                 return subprocess.call([makecmd, 'info'])
         except OSError:
             print('Error: Failed to run: %s' % makecmd)
             return 1
 
     def build_gettext(self) -> int:
-        dtdir = self.builddir_join('gettext', '.doctrees')
+        dtdir = self.build_dir_join('gettext', '.doctrees')
         if self.run_generic_build('gettext', doctreedir=dtdir) > 0:
             return 1
         return 0
 
     def run_generic_build(self, builder: str, doctreedir: str | None = None) -> int:
         # compatibility with old Makefile
-        papersize = os.getenv('PAPER', '')
-        opts = self.opts
-        if papersize in ('a4', 'letter'):
-            opts.extend(['-D', 'latex_elements.papersize=' + papersize + 'paper'])
+        paper_size = os.getenv('PAPER', '')
+        if paper_size in {'a4', 'letter'}:
+            self.opts.extend(['-D', f'latex_elements.papersize={paper_size}paper'])
         if doctreedir is None:
-            doctreedir = self.builddir_join('doctrees')
+            doctreedir = self.build_dir_join('doctrees')
 
-        args = ['-b', builder,
-                '-d', doctreedir,
-                self.srcdir,
-                self.builddir_join(builder)]
-        return build_main(args + opts)
+        args = [
+            '--builder', builder,
+            '--doctree-dir', doctreedir,
+            self.source_dir,
+            self.build_dir_join(builder),
+        ]
+        return build_main(args + self.opts)
 
 
 def run_make_mode(args: Sequence[str]) -> int:
     if len(args) < 3:
         print('Error: at least 3 arguments (builder, source '
               'dir, build dir) are required.', file=sys.stderr)
         return 1
-    make = Make(args[1], args[2], args[3:])
-    run_method = 'build_' + args[0]
+
+    builder_name = args[0]
+    make = Make(source_dir=args[1], build_dir=args[2], opts=args[3:])
+    run_method = f'build_{builder_name}'
     if hasattr(make, run_method):
         return getattr(make, run_method)()
-    return make.run_generic_build(args[0])
+    return make.run_generic_build(builder_name)
diff --git a/tests/test_command_line.py b/tests/test_command_line.py
@@ -0,0 +1,217 @@
+from __future__ import annotations
+
+import os.path
+from typing import Any
+
+import pytest
+
+from sphinx.cmd import make_mode
+from sphinx.cmd.build import get_parser
+from sphinx.cmd.make_mode import run_make_mode
+
+DEFAULTS = {
+    'filenames': [],
+    'jobs': 1,
+    'force_all': False,
+    'freshenv': False,
+    'doctreedir': None,
+    'confdir': None,
+    'noconfig': False,
+    'define': [],
+    'htmldefine': [],
+    'tags': [],
+    'nitpicky': False,
+    'verbosity': 0,
+    'quiet': False,
+    'really_quiet': False,
+    'color': 'auto',
+    'warnfile': None,
+    'warningiserror': False,
+    'keep_going': False,
+    'traceback': False,
+    'pdb': False,
+    'exception_on_warning': False,
+}
+
+EXPECTED_BUILD_MAIN = {
+    'builder': 'html',
+    'sourcedir': 'source_dir',
+    'outputdir': 'build_dir',
+    'filenames': ['filename1', 'filename2'],
+    'freshenv': True,
+    'noconfig': True,
+    'quiet': True,
+}
+
+EXPECTED_MAKE_MODE = {
+    'builder': 'html',
+    'sourcedir': 'source_dir',
+    'outputdir': os.path.join('build_dir', 'html'),
+    'doctreedir': os.path.join('build_dir', 'doctrees'),
+    'filenames': ['filename1', 'filename2'],
+    'freshenv': True,
+    'noconfig': True,
+    'quiet': True,
+}
+
+BUILDER_BUILD_MAIN = [
+    '--builder',
+    'html',
+]
+BUILDER_MAKE_MODE = [
+    'html',
+]
+POSITIONAL_DIRS = [
+    'source_dir',
+    'build_dir',
+]
+POSITIONAL_FILENAMES = [
+    'filename1',
+    'filename2',
+]
+POSITIONAL = POSITIONAL_DIRS + POSITIONAL_FILENAMES
+POSITIONAL_MAKE_MODE = BUILDER_MAKE_MODE + POSITIONAL
+EARLY_OPTS = [
+    '--quiet',
+]
+LATE_OPTS = [
+    '-E',
+    '--isolated',
+]
+OPTS = EARLY_OPTS + LATE_OPTS
+OPTS_BUILD_MAIN = BUILDER_BUILD_MAIN + OPTS
+
+
+def parse_arguments(args: list[str]) -> dict[str, Any]:
+    parsed = vars(get_parser().parse_args(args))
+    return {k: v for k, v in parsed.items() if k not in DEFAULTS or v != DEFAULTS[k]}
+
+
+def test_build_main_parse_arguments_pos_first() -> None:
+    # <positional...> <opts>
+    args = [
+        *POSITIONAL,
+        *OPTS,
+    ]
+    assert parse_arguments(args) == EXPECTED_BUILD_MAIN
+
+
+def test_build_main_parse_arguments_pos_last() -> None:
+    # <opts> <positional...>
+    args = [
+        *OPTS,
+        *POSITIONAL,
+    ]
+    assert parse_arguments(args) == EXPECTED_BUILD_MAIN
+
+
+def test_build_main_parse_arguments_pos_middle() -> None:
+    # <opts> <positional...> <opts>
+    args = [
+        *EARLY_OPTS,
+        *BUILDER_BUILD_MAIN,
+        *POSITIONAL,
+        *LATE_OPTS,
+    ]
+    assert parse_arguments(args) == EXPECTED_BUILD_MAIN
+
+
+@pytest.mark.xfail(reason='sphinx-build does not yet support filenames after options')
+def test_build_main_parse_arguments_filenames_last() -> None:
+    args = [
+        *POSITIONAL_DIRS,
+        *OPTS,
+        *POSITIONAL_FILENAMES,
+    ]
+    assert parse_arguments(args) == EXPECTED_BUILD_MAIN
+
+
+def test_build_main_parse_arguments_pos_intermixed(
+    capsys: pytest.CaptureFixture[str],
+) -> None:
+    args = [
+        *EARLY_OPTS,
+        *BUILDER_BUILD_MAIN,
+        *POSITIONAL_DIRS,
+        *LATE_OPTS,
+        *POSITIONAL_FILENAMES,
+    ]
+    with pytest.raises(SystemExit):
+        parse_arguments(args)
+    stderr = capsys.readouterr().err.splitlines()
+    assert stderr[-1].endswith('error: unrecognized arguments: filename1 filename2')
+
+
+def test_make_mode_parse_arguments_pos_first(monkeypatch: pytest.MonkeyPatch) -> None:
+    # -M <positional...> <opts>
+    monkeypatch.setattr(make_mode, 'build_main', parse_arguments)
+    args = [
+        *POSITIONAL_MAKE_MODE,
+        *OPTS,
+    ]
+    assert run_make_mode(args) == EXPECTED_MAKE_MODE
+
+
+def test_make_mode_parse_arguments_pos_last(
+    monkeypatch: pytest.MonkeyPatch,
+    capsys: pytest.CaptureFixture[str],
+) -> None:
+    # -M <opts> <positional...>
+    monkeypatch.setattr(make_mode, 'build_main', parse_arguments)
+    args = [
+        *OPTS,
+        *POSITIONAL_MAKE_MODE,
+    ]
+    with pytest.raises(SystemExit):
+        run_make_mode(args)
+    stderr = capsys.readouterr().err.splitlines()
+    assert stderr[-1].endswith('error: argument --builder/-b: expected one argument')
+
+
+def test_make_mode_parse_arguments_pos_middle(
+    monkeypatch: pytest.MonkeyPatch,
+    capsys: pytest.CaptureFixture[str],
+) -> None:
+    # -M <opts> <positional...> <opts>
+    monkeypatch.setattr(make_mode, 'build_main', parse_arguments)
+    args = [
+        *EARLY_OPTS,
+        *POSITIONAL_MAKE_MODE,
+        *LATE_OPTS,
+    ]
+    with pytest.raises(SystemExit):
+        run_make_mode(args)
+    stderr = capsys.readouterr().err.splitlines()
+    assert stderr[-1].endswith('error: argument --builder/-b: expected one argument')
+
+
+@pytest.mark.xfail(reason='sphinx-build does not yet support filenames after options')
+def test_make_mode_parse_arguments_filenames_last(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.setattr(make_mode, 'build_main', parse_arguments)
+    args = [
+        *BUILDER_MAKE_MODE,
+        *POSITIONAL_DIRS,
+        *OPTS,
+        *POSITIONAL_FILENAMES,
+    ]
+    assert run_make_mode(args) == EXPECTED_MAKE_MODE
+
+
+def test_make_mode_parse_arguments_pos_intermixed(
+    monkeypatch: pytest.MonkeyPatch,
+    capsys: pytest.CaptureFixture[str],
+) -> None:
+    monkeypatch.setattr(make_mode, 'build_main', parse_arguments)
+    args = [
+        *EARLY_OPTS,
+        *BUILDER_MAKE_MODE,
+        *POSITIONAL_DIRS,
+        *LATE_OPTS,
+        *POSITIONAL_FILENAMES,
+    ]
+    with pytest.raises(SystemExit):
+        run_make_mode(args)
+    stderr = capsys.readouterr().err.splitlines()
+    assert stderr[-1].endswith('error: argument --builder/-b: expected one argument')
