From 62e386fe9ee1ab7bfff409d164cc577e72612fbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=99=B4=E6=96=8B?= Date: Wed, 21 Oct 2026 15:05:33 +0800 Subject: [PATCH] eval: require a positive --limit --- eval/run_eval.py | 17 ++++++- tests/eval/test_eval_limit_arg.py | 79 +++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 tests/eval/test_eval_limit_arg.py diff --git a/eval/run_eval.py b/eval/run_eval.py index 4c7bbbe..5ebc6fb 100644 --- a/eval/run_eval.py +++ b/eval/run_eval.py @@ -43,12 +43,27 @@ def _reasoning_effort(value: str) -> int | str: return effort +def _positive_limit(value: str) -> int: + """Accept a positive integer only; the default (None) keeps "run every problem". + + build_records() checks the budget after appending a record, so a non-positive limit evaluates + exactly one problem instead of none. + """ + try: + limit = int(value) + except ValueError: + raise argparse.ArgumentTypeError(f"expected a positive integer, got {value!r}") from None + if limit < 1: + raise argparse.ArgumentTypeError(f"expected a positive integer (omit --limit to run every problem), got {limit}") + return limit + + parser = argparse.ArgumentParser(description="eval benchmark") # benchmark setting parser.add_argument("--bench", default="aime", choices=list(SUPPORTED)) parser.add_argument("--model-name", default=DEFAULT_MODEL_NAME, help="HF model id or HF ckpt directory") parser.add_argument("--data-file", required=True, help="problem jsonl") -parser.add_argument("--limit", type=int, default=None, help="run only the first N problems") +parser.add_argument("--limit", type=_positive_limit, default=None, help="run only the first N problems; N >= 1 (default: all)") parser.add_argument("--out-dir", default=None) parser.add_argument("--tag", default=None, help="output dir suffix, to compare settings of the same model") # sample param diff --git a/tests/eval/test_eval_limit_arg.py b/tests/eval/test_eval_limit_arg.py new file mode 100644 index 0000000..624b902 --- /dev/null +++ b/tests/eval/test_eval_limit_arg.py @@ -0,0 +1,79 @@ +"""`--limit` must be a positive integer: 0 and negative values used to evaluate one problem.""" + +from __future__ import annotations + +import subprocess +import sys +from collections.abc import Iterator +from pathlib import Path +from types import ModuleType + +import pytest + +pytestmark = [pytest.mark.ci, pytest.mark.ci_npu] + +REPO = Path(__file__).resolve().parents[2] +EVAL_DIR = REPO / "eval" +RUN_EVAL = EVAL_DIR / "run_eval.py" + + +@pytest.fixture(autouse=True) +def _drop_run_eval_module() -> Iterator[None]: + """eval/run_eval.py parses argv at import time; never let a parsed module leak between tests.""" + sys.modules.pop("run_eval", None) + yield + sys.modules.pop("run_eval", None) + + +def _import_with_argv(monkeypatch: pytest.MonkeyPatch, *argv: str) -> ModuleType: + # Drop the cached module so the import below re-executes the module body, including parse_args(). + sys.modules.pop("run_eval", None) + monkeypatch.setattr(sys, "argv", ["run_eval.py", *argv]) + monkeypatch.syspath_prepend(str(EVAL_DIR)) + import run_eval # noqa: PLC0415 + + return run_eval + + +@pytest.mark.parametrize("value", ["0", "-1", "-42"]) +def test_non_positive_limit_is_rejected( + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + value: str, +) -> None: + with pytest.raises(SystemExit) as excinfo: + _import_with_argv(monkeypatch, "--data-file", "unused.jsonl", "--limit", value) + assert excinfo.value.code == 2 + stderr = capsys.readouterr().err + assert "argument --limit" in stderr + assert "expected a positive integer" in stderr + + +def test_positive_limit_and_default_are_unchanged(monkeypatch: pytest.MonkeyPatch) -> None: + assert _import_with_argv(monkeypatch, "--data-file", "unused.jsonl", "--limit", "3").args.limit == 3 + assert _import_with_argv(monkeypatch, "--data-file", "unused.jsonl").args.limit is None + + +def test_rejection_happens_before_any_run_artifact(tmp_path: Path) -> None: + """The CLI exits while parsing, before main() creates the output dir or launches the engine.""" + out_dir = tmp_path / "out" + proc = subprocess.run( + [ + sys.executable, + str(RUN_EVAL), + "--bench", + "aime", + "--data-file", + str(tmp_path / "unused.jsonl"), + "--out-dir", + str(out_dir), + "--limit", + "0", + ], + cwd=str(REPO), + capture_output=True, + text=True, + ) + assert proc.returncode == 2 + assert "argument --limit: expected a positive integer" in proc.stderr + assert not out_dir.exists()