diff --git a/batch_runner.py b/batch_runner.py index f2f00e0de1..af6dd75c1e 100644 --- a/batch_runner.py +++ b/batch_runner.py @@ -1246,21 +1246,21 @@ def main( # Validate required arguments if not dataset_file: print("āŒ Error: --dataset_file is required") - return - + raise SystemExit(1) + if not batch_size or batch_size < 1: print("āŒ Error: --batch_size must be a positive integer") - return - + raise SystemExit(1) + if not run_name: print("āŒ Error: --run_name is required") - return - + raise SystemExit(1) + # Parse provider preferences (comma-separated strings to lists) providers_allowed_list = [p.strip() for p in providers_allowed.split(",")] if providers_allowed else None providers_ignored_list = [p.strip() for p in providers_ignored.split(",")] if providers_ignored else None providers_order_list = [p.strip() for p in providers_order.split(",")] if providers_order else None - + # Build reasoning_config from CLI flags # --reasoning_disabled takes priority, then --reasoning_effort, then default (medium) reasoning_config = None @@ -1273,10 +1273,10 @@ def main( valid_efforts = ["none", "minimal", "low", "medium", "high", "xhigh", "max", "ultra"] if reasoning_effort not in valid_efforts: print(f"āŒ Error: --reasoning_effort must be one of: {', '.join(valid_efforts)}") - return + raise SystemExit(1) reasoning_config = {"enabled": True, "effort": reasoning_effort} print(f"🧠 Reasoning effort: {reasoning_effort}") - + # Load prefill messages from JSON file if provided prefill_messages = None if prefill_messages_file: @@ -1285,12 +1285,12 @@ def main( prefill_messages = json.load(f) if not isinstance(prefill_messages, list): print("āŒ Error: prefill_messages_file must contain a JSON array of messages") - return + raise SystemExit(1) print(f"šŸ’¬ Loaded {len(prefill_messages)} prefill messages from {prefill_messages_file}") except Exception as e: print(f"āŒ Error loading prefill messages: {e}") - return - + raise SystemExit(1) + # Initialize and run batch runner try: runner = BatchRunner( @@ -1317,12 +1317,12 @@ def main( ) runner.run(resume=resume) - + except Exception as e: print(f"\nāŒ Fatal error: {e}") if verbose: traceback.print_exc() - return 1 + raise SystemExit(1) if __name__ == "__main__": diff --git a/tests/test_batch_runner_exit_code.py b/tests/test_batch_runner_exit_code.py new file mode 100644 index 0000000000..7329c4622c --- /dev/null +++ b/tests/test_batch_runner_exit_code.py @@ -0,0 +1,64 @@ +"""Regression tests for batch_runner process exit codes. + +Python Fire serializes the return value of the wrapped function but does not +use it as the process exit code. Before the fix, all of ``main``'s error paths +returned ``0`` because a bare ``return`` or ``return 1`` was treated as the +function result, not a non-zero exit status. +""" + +import subprocess +import sys +from pathlib import Path + +import pytest + + +REPO_ROOT = Path(__file__).parent.parent +PYTHON = sys.executable + + +def _run(*args): + return subprocess.run( + [PYTHON, "batch_runner.py", *args], + cwd=REPO_ROOT, + text=True, + capture_output=True, + ) + + +def test_missing_dataset_file_exits_nonzero(): + result = _run("--run_name=test") + assert result.returncode == 1 + assert "--dataset_file is required" in result.stdout + + +def test_invalid_batch_size_exits_nonzero(): + result = _run("--dataset_file=/tmp/data.jsonl", "--batch_size=-1", "--run_name=test") + assert result.returncode == 1 + assert "--batch_size must be a positive integer" in result.stdout + + +def test_invalid_reasoning_effort_exits_nonzero(): + result = _run( + "--dataset_file=/tmp/data.jsonl", + "--batch_size=1", + "--run_name=test", + "--reasoning_effort=invalid", + ) + assert result.returncode == 1 + assert "--reasoning_effort must be one of" in result.stdout + + +def test_invalid_prefill_messages_file_exits_nonzero(tmp_path): + bad_prefill = tmp_path / "prefill.json" + bad_prefill.write_text("not json", encoding="utf-8") + result = _run( + "--dataset_file=/tmp/data.jsonl", + "--batch_size=1", + "--run_name=test", + f"--prefill_messages_file={bad_prefill}", + ) + assert result.returncode == 1 + assert "Error loading prefill messages" in result.stdout + +