From fe2c883c23f72f51cb031f70595f54bdcd810585 Mon Sep 17 00:00:00 2001
From: David Steele <david@pgbackrest.org>
Date: Sun, 27 Sep 2026 17:42:33 +0300
Subject: [PATCH] Allow test.py options before the command.

When a command is given, argparse parses the options of the command into a new namespace and copies every value, including defaults, over the values the main parser set from options before the command. As a result, `test.py --vm=u24 vm-build` ran vm-build with --vm=none.

Move the command to the front of the argument list before parsing so the command parser parses every option. An option before the command that the command does not define is now an error instead of being ignored.
---
 test/lib/config/cli.py                  | 37 +++++++++++++++++++++++--
 test/src/module/test/config/cli_test.py | 19 +++++++++++--
 2 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/test/lib/config/cli.py b/test/lib/config/cli.py
index d5836cc35..4673fae8e 100644
--- a/test/lib/config/cli.py
+++ b/test/lib/config/cli.py
@@ -13,8 +13,7 @@ the help shows them without being asked for the command.
 The unit command is not in the command list the help shows since a test run is what calls it, in a container where only the
 repository copy is available, rather than something to run by hand.
 
-A command parses into the same configuration as the test run, so an option belongs after the command, which replaces whatever was
-given before it."""
+Options may be given before or after the command. The command parser parses all of them."""
 
 ####################################################################################################################################
 import argparse
@@ -157,6 +156,37 @@ def _parser_vm_build(command, parent):
     result.add_argument("--no-cache", dest="cache", action="store_false", help="build the containers without the cache")
 
 
+####################################################################################################################################
+def _command_first(arg_list, parser, command_map):
+    """Move the command to the front of the argument list.
+
+    The command is the first argument that is not an option or an option value. The argument list is unchanged when there is no
+    command."""
+
+    # Options that take a value in the main parser or any command parser
+    option_value = set()
+
+    for parser_option in [parser, *command_map.values()]:
+        for action in parser_option._actions:
+            if action.nargs != 0:
+                option_value.update(action.option_strings)
+
+    index = 0
+
+    while index < len(arg_list):
+        arg = arg_list[index]
+
+        if not arg.startswith("-"):
+            if arg in command_map:
+                return [arg] + arg_list[:index] + arg_list[index + 1 :]
+
+            break
+
+        index += 2 if arg in option_value else 1
+
+    return arg_list
+
+
 ####################################################################################################################################
 def cli_parse(arg_list, version):
     """Build the parser and parse the command line."""
@@ -191,7 +221,8 @@ def cli_parse(arg_list, version):
     )
     code_format.add_argument("--check", action="store_true", help="check the formatting rather than changing it")
 
-    result = parser.parse_args(arg_list)
+    # The command parser parses every option, including those given before the command
+    result = parser.parse_args(_command_first(arg_list, parser, command.choices))
 
     # Fill in the command a command line with no command was asking for, so nothing downstream has to know it was left out
     if result.command is None:
diff --git a/test/src/module/test/config/cli_test.py b/test/src/module/test/config/cli_test.py
index f718b058d..56d30b4f3 100644
--- a/test/src/module/test/config/cli_test.py
+++ b/test/src/module/test/config/cli_test.py
@@ -160,8 +160,8 @@ def test_cli_test():
 
     assert_true(config.lint_only)
 
-    # The command can be named, which is the same run with the same options
-    config = cli_parse(["test", "--vm=u24", "--module=common/error"], VERSION)
+    # The command can be named, which is the same run with the same options before or after it
+    config = cli_parse(["--vm", "u24", "test", "--module=common/error"], VERSION)
 
     assert_equal(config.command, "test")
     assert_equal(config.vm, "u24")
@@ -181,7 +181,8 @@ def test_cli_vm_build():
     assert_true(config.cache)
     assert_is_none(config.vm_arch)
 
-    config = cli_parse(["vm-build", "--vm=all", "--vm-arch=aarch64", "--no-cache"], VERSION)
+    # Options before the command
+    config = cli_parse(["--vm=all", "--vm-arch", "aarch64", "--no-cache", "vm-build"], VERSION)
 
     assert_equal(config.vm, "all")
     assert_equal(config.vm_arch, "aarch64")
@@ -201,6 +202,18 @@ def test_cli_error():
     assert_equal(status, 2)
     assert_in("unrecognized arguments: --bogus", output)
 
+    # An option before the command that the command does not define
+    status, output = _cli_parse_exit(["--module=common/error", "vm-build"])
+
+    assert_equal(status, 2)
+    assert_in("unrecognized arguments: --module=common/error", output)
+
+    # An argument after the options that is not a command
+    status, output = _cli_parse_exit(["--vm=u24", "bogus"])
+
+    assert_equal(status, 2)
+    assert_in("invalid choice: 'bogus'", output)
+
     # A level outside the log levels is rejected here rather than later when it is converted
     status, output = _cli_parse_exit(["unit", "common/error", "--log-level=bogus"])