From 7667f7a75052618afbd8cfaec53e9a764beed082 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Sat, 3 Oct 2026 09:59:49 -0700 Subject: [PATCH] Reuse the shared positive integer parser in benchmark The benchmark parser kept its own positive_int(), which converted the argument with int() and rejected values below 1. common.positive_int() already does the same for the performance, microVM test, and adversarial parsers, but rejects with "must be greater than zero", while the benchmark options report "must be at least 1". Give common.positive_int() a keyword-only message argument, whose default keeps the existing message for those callers, and make the benchmark positive_int() call it with "must be at least 1". Keep the benchmark parser a named function rather than a functools.partial. argparse names the type callable in its "invalid value" error, and a partial has no __name__, so a non-integer value would be reported with the partial's repr, which includes a process-specific function address, instead of as an "invalid positive_int value". Test the custom message of the shared parser, and parse "benchmark --runs" through the NVX CLI parser to check that a non-integer value and zero keep their existing diagnostics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- scripts/nvx_tools/benchmark.py | 5 +---- scripts/nvx_tools/common.py | 4 ++-- scripts/test_nvx_tools.py | 19 +++++++++++++++++++ 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/scripts/nvx_tools/benchmark.py b/scripts/nvx_tools/benchmark.py index 353c004d8..2f207cf88 100644 --- a/scripts/nvx_tools/benchmark.py +++ b/scripts/nvx_tools/benchmark.py @@ -510,10 +510,7 @@ def configure_parser( def positive_int(value: str) -> int: - parsed = int(value) - if parsed < 1: - raise argparse.ArgumentTypeError("must be at least 1") - return parsed + return common.positive_int(value, message="must be at least 1") def apply_benchmark_suite_defaults(args: argparse.Namespace) -> None: diff --git a/scripts/nvx_tools/common.py b/scripts/nvx_tools/common.py index fb7077753..4db94aaef 100644 --- a/scripts/nvx_tools/common.py +++ b/scripts/nvx_tools/common.py @@ -39,10 +39,10 @@ def strict_json_object(pairs: list[tuple[str, object]]) -> dict[str, object]: return result -def positive_int(value: str) -> int: +def positive_int(value: str, *, message: str = "must be greater than zero") -> int: parsed = int(value) if parsed <= 0: - raise argparse.ArgumentTypeError("must be greater than zero") + raise argparse.ArgumentTypeError(message) return parsed diff --git a/scripts/test_nvx_tools.py b/scripts/test_nvx_tools.py index f596dc6f4..eedd6a99e 100644 --- a/scripts/test_nvx_tools.py +++ b/scripts/test_nvx_tools.py @@ -508,6 +508,21 @@ def test_benchmark_preserves_canonical_shell_memory_defaults(self): self.assertEqual(args.shell_memories, [128, 256, 512]) + def test_benchmark_positive_integer_diagnostics_are_stable(self): + for value, message in ( + ("not-an-int", "argument --runs: invalid positive_int value: 'not-an-int'"), + ("0", "argument --runs: must be at least 1"), + ): + with self.subTest(value=value): + with ( + patch("sys.stderr", new_callable=io.StringIO) as errors, + self.assertRaises(SystemExit) as exit_context, + ): + nvx.parse_args(["benchmark", "--runs", value]) + + self.assertEqual(exit_context.exception.code, 2) + self.assertIn(message, errors.getvalue()) + def test_release_commands_keep_their_cli_contract(self): download = nvx.parse_args( ["download", "--repository", "example/nvx", "--hypervisor", "auto"] @@ -11858,6 +11873,10 @@ def test_rejects_non_integer_input(self): with self.assertRaises(ValueError): common.positive_int("not-an-int") + def test_accepts_custom_error_message(self): + with self.assertRaisesRegex(argparse.ArgumentTypeError, "^must be at least 1$"): + common.positive_int("0", message="must be at least 1") + class PositiveFloatTests(unittest.TestCase): def test_accepts_positive_finite_float(self):