diff --git a/src/google/adk/cli/cli_tools_click.py b/src/google/adk/cli/cli_tools_click.py index 49fb819388..61190c78b1 100644 --- a/src/google/adk/cli/cli_tools_click.py +++ b/src/google/adk/cli/cli_tools_click.py @@ -1392,6 +1392,17 @@ def cli_eval( ) pretty_print_eval_result(eval_result) + # The printed "Eval Run Summary" above is otherwise the only place this + # command's verdict is visible -- without this, the process exit code is + # always 0 regardless of any failed test, indistinguishable from every + # test genuinely passing. Every other subcommand in this file that has a + # pass/fail outcome (e.g. `test`, `run --query`) already does this; `eval` + # was the one command whose exit code carried no signal at all. + total_failed = sum( + pass_fail_count[1] for pass_fail_count in eval_run_summary.values() + ) + sys.exit(1 if total_failed else 0) + @main.command("optimize", cls=HelpfulCommand) @click.argument( diff --git a/tests/unittests/cli/utils/test_cli_tools_click.py b/tests/unittests/cli/utils/test_cli_tools_click.py index dddda4471f..9a2723828c 100644 --- a/tests/unittests/cli/utils/test_cli_tools_click.py +++ b/tests/unittests/cli/utils/test_cli_tools_click.py @@ -1738,7 +1738,14 @@ def test_cli_eval_with_eval_set_file_path( [str(agent_path), str(eval_set_file)], ) - assert result.exit_code == 0 + # This eval case has no invocations and no configured criteria, so it + # never produces a PASSED verdict ("Tests failed: 1" in the printed + # summary, unchanged by this assertion) -- exit_code == 1 is the correct, + # honest reflection of that. This assertion used to read `== 0`, which + # was only ever true because `cli_eval`'s exit code carried no real + # signal at all prior to this fix; it was not a deliberate claim that + # this specific eval run passed. + assert result.exit_code == 1 # Assert that we wrote eval set results eval_set_results_manager = LocalEvalSetResultsManager( agents_dir=str(tmp_path) @@ -1777,7 +1784,11 @@ def test_cli_eval_with_eval_set_id( [str(agent_path), "test_eval_set_id:case1,case2"], ) - assert result.exit_code == 0 + # Same reasoning as test_cli_eval_with_eval_set_file_path above: both + # cases have no invocations and no configured criteria, so neither + # produces a PASSED verdict -- exit_code == 1 is what this run actually + # produces, now that the exit code reflects the real verdict. + assert result.exit_code == 1 # Assert that we wrote eval set results eval_set_results_manager = LocalEvalSetResultsManager( agents_dir=str(tmp_path) @@ -1788,6 +1799,75 @@ def test_cli_eval_with_eval_set_id( assert len(eval_set_results) == 1 +@pytest.mark.parametrize( + "final_eval_status, expected_exit_code", + [ + (pytest.param("PASSED", 0, id="all_passed_exits_zero")), + (pytest.param("FAILED", 1, id="any_failed_exits_nonzero")), + ], +) +def test_cli_eval_exit_code_reflects_final_eval_status( + mock_load_eval_set_from_file, + mock_get_root_agent, + tmp_path, + final_eval_status, + expected_exit_code, +): + """`cli_eval`'s process exit code must reflect PASSED/FAILED. + + Before this fix, `cli_eval` never called `sys.exit` at all, so the + process always exited 0 regardless of the printed "Tests failed" count + -- indistinguishable from every test genuinely passing, which made the + command unusable as a CI gate on its own exit code. + """ + from google.adk.evaluation.eval_result import EvalCaseResult + from google.adk.evaluation.evaluator import EvalStatus + + agent_path = tmp_path / "my_agent" + agent_path.mkdir() + (agent_path / "__init__.py").touch() + + eval_set_file = tmp_path / "my_evals.json" + eval_set_file.write_text("{}") + + mock_load_eval_set_from_file.return_value = EvalSet( + eval_set_id="my_evals", + eval_cases=[EvalCase(eval_id="case1", conversation=[])], + ) + + canned_result = EvalCaseResult( + eval_set_file="my_evals", + eval_set_id="my_evals", + eval_id="case1", + final_eval_status=getattr(EvalStatus, final_eval_status), + overall_eval_metric_results=[], + eval_metric_result_per_invocation=[], + session_id="", + ) + + with ( + mock.patch( + "google.adk.cli.cli_eval._collect_inferences", + new_callable=mock.AsyncMock, + return_value=[], + ), + mock.patch( + "google.adk.cli.cli_eval._collect_eval_results", + new_callable=mock.AsyncMock, + return_value=[canned_result], + ), + ): + result = CliRunner().invoke( + cli_tools_click.cli_eval, + [str(agent_path), str(eval_set_file)], + ) + + assert result.exit_code == expected_exit_code, ( + result.output, + result.exception, + ) + + def test_cli_create_eval_set(tmp_path: Path): app_name = "test_app" eval_set_id = "test_eval_set"