diff --git a/tests/test_run/test_websocket_socket.py b/tests/test_run/test_websocket_socket.py index 324b60f..de45c55 100644 --- a/tests/test_run/test_websocket_socket.py +++ b/tests/test_run/test_websocket_socket.py @@ -288,6 +288,85 @@ def test_browser_peer_warning_not_shown_for_unrelated_failure(self): echoed = " ".join(str(c) for call in mock_echo.call_args_list for c in call[0]) assert "BROWSER TAB REQUIRED" not in echoed + def test_terminal_state_recorded_in_final_states(self): + case = _make_case() + suite = _make_suite(cases=[case]) + s = _make_socket(suites=[suite]) + self._call(s, self._update(state="passed")) + assert s.test_case_final_states[(0, 0)] == "passed" + + def test_later_update_overwrites_earlier_final_state_for_same_case(self): + case = _make_case() + suite = _make_suite(cases=[case]) + s = _make_socket(suites=[suite]) + self._call(s, self._update(state="error")) + self._call(s, self._update(state="passed")) + assert s.test_case_final_states[(0, 0)] == "passed" + + def test_non_terminal_state_not_recorded(self): + case = _make_case() + suite = _make_suite(cases=[case]) + s = _make_socket(suites=[suite]) + self._call(s, self._update(state="executing")) + assert (0, 0) not in s.test_case_final_states + + +# --------------------------------------------------------------------------- +# Results summary / exit code tallying (issue #1095) +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestResultsSummary: + + def _call(self, socket: TestRunSocket, update: TestCaseUpdate): + socket._TestRunSocket__log_test_case_update(update) + + def _update(self, case_idx=0, suite_idx=0, state="passed") -> TestCaseUpdate: + return TestCaseUpdate( + state=state, + test_case_execution_index=case_idx, + test_suite_execution_index=suite_idx, + errors=None, + ) + + def _socket_with_cases(self, states: list[str]) -> TestRunSocket: + cases = [_make_case(idx=i) for i in range(len(states))] + suite = _make_suite(cases=cases) + s = _make_socket(suites=[suite]) + for i, state in enumerate(states): + self._call(s, self._update(case_idx=i, state=state)) + return s + + def test_no_cases_executed_summary(self): + s = _make_socket() + assert s.format_results_summary() == "0 test cases executed" + assert s.has_test_failures() is False + assert s.test_case_result_counts() == {} + + def test_all_passed_no_failures(self): + s = self._socket_with_cases(["passed", "passed", "passed"]) + assert s.has_test_failures() is False + assert s.format_results_summary() == "3 passed" + + def test_mixed_states_counted_and_ordered(self): + s = self._socket_with_cases(["passed", "failed", "error", "not_applicable", "cancelled"]) + assert s.has_test_failures() is True + assert s.format_results_summary() == "1 passed, 1 failed, 1 error, 1 not applicable, 1 cancelled" + + def test_not_applicable_and_cancelled_do_not_count_as_failures(self): + s = self._socket_with_cases(["passed", "not_applicable", "cancelled"]) + assert s.has_test_failures() is False + + def test_error_state_counts_as_failure(self): + s = self._socket_with_cases(["passed", "error"]) + assert s.has_test_failures() is True + + def test_case_updated_more_than_once_counted_once(self): + s = self._socket_with_cases(["executing"]) + self._call(s, self._update(case_idx=0, state="passed")) + assert s.test_case_result_counts() == {"passed": 1} + # --------------------------------------------------------------------------- # __handle_test_update — dispatch diff --git a/tests/test_run_tests.py b/tests/test_run_tests.py index 5d0765b..7a20b0a 100644 --- a/tests/test_run_tests.py +++ b/tests/test_run_tests.py @@ -73,6 +73,8 @@ def test_run_tests_success_minimal_args( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act @@ -116,6 +118,8 @@ def test_run_tests_success_with_custom_config( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act @@ -166,6 +170,8 @@ def test_run_tests_success_with_pics_config( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act @@ -210,6 +216,8 @@ def test_run_tests_success_with_project_id( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act @@ -249,6 +257,8 @@ def test_run_tests_success_with_no_color( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act @@ -426,6 +436,8 @@ def test_run_tests_api_error_starting_test_run( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act @@ -503,6 +515,8 @@ def test_run_tests_various_test_lists( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act @@ -544,6 +558,8 @@ def test_run_tests_test_selection_building( mock_build_test_selection.return_value = {"mock_collection": {"mock_suite": {"mock": 1}}} mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act @@ -584,6 +600,8 @@ def test_run_tests_logger_configuration( mock_configure_logger.return_value = "/path/to/test_logs/custom_run.log" mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act @@ -626,6 +644,8 @@ def test_run_tests_default_title_generation( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act @@ -675,6 +695,8 @@ def test_run_tests_config_data_processing( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act @@ -720,6 +742,8 @@ def test_run_tests_prompt_timeout_merges_into_execution_config( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act @@ -766,6 +790,8 @@ def test_run_tests_prompt_timeout_wins_over_config_file( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act @@ -819,6 +845,132 @@ def test_run_tests_client_cleanup_on_exception(self, cli_runner: CliRunner, mock mock_api_client.aclose.assert_called_once() +@pytest.mark.unit +@pytest.mark.cli +class TestRunTestsExitCodeAndSummary: + """Test cases for the run-tests results summary and exit code contract (issue #1095).""" + + def _invoke( + self, + cli_runner: CliRunner, + mock_async_apis: Mock, + mock_api_client: Mock, + sample_test_collections: api_models.TestCollections, + sample_test_run_execution: api_models.TestRunExecutionWithChildren, + sample_default_config_dict: dict, + has_test_failures: bool, + results_summary: str, + ): + project_api = mock_async_apis.projects_api.default_config_api_v1_projects_default_config_get + test_collection_api = mock_async_apis.test_collections_api.read_test_collections_api_v1_test_collections__get + test_run_executions_api = mock_async_apis.test_run_executions_api + cli_api = test_run_executions_api.create_cli_test_run_execution_api_v1_test_run_executions_cli_post + id_start = test_run_executions_api.start_test_run_execution_api_v1_test_run_executions__id__start_post + + project_api.return_value = sample_default_config_dict + test_collection_api.return_value = sample_test_collections + cli_api.return_value = sample_test_run_execution + id_start.return_value = sample_test_run_execution + with ( + patch("th_cli.commands.run_tests.get_client", return_value=mock_api_client), + patch("th_cli.commands.run_tests.AsyncApis", return_value=mock_async_apis), + patch( + "th_cli.commands.run_tests.test_logging.configure_logger_for_run", return_value="./test_logs/test.log" + ), + patch("th_cli.commands.run_tests.TestRunSocket") as mock_socket_class, + patch("th_cli.commands.run_tests.convert_nested_to_dict", return_value=sample_default_config_dict), + ): + mock_socket = Mock() + mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = has_test_failures + mock_socket.format_results_summary.return_value = results_summary + mock_socket_class.return_value = mock_socket + + return cli_runner.invoke(run_tests, ["--tests-list", "TC-ACE-1.1,TC-ACE-1.2"]) + + def test_all_passed_exits_zero_with_summary( + self, + cli_runner: CliRunner, + mock_async_apis: Mock, + mock_api_client: Mock, + sample_test_collections: api_models.TestCollections, + sample_test_run_execution: api_models.TestRunExecutionWithChildren, + sample_default_config_dict: dict, + ) -> None: + """[Test Case 1] All tests pass: exit code 0, summary reflects the pass count.""" + result = self._invoke( + cli_runner, + mock_async_apis, + mock_api_client, + sample_test_collections, + sample_test_run_execution, + sample_default_config_dict, + has_test_failures=False, + results_summary="2 passed", + ) + + assert result.exit_code == 0 + assert "Results: 2 passed" in result.output + + def test_failures_present_exit_nonzero_with_summary( + self, + cli_runner: CliRunner, + mock_async_apis: Mock, + mock_api_client: Mock, + sample_test_collections: api_models.TestCollections, + sample_test_run_execution: api_models.TestRunExecutionWithChildren, + sample_default_config_dict: dict, + ) -> None: + """[Test Case 2] At least one test fails: non-zero exit code, summary reflects the failure.""" + result = self._invoke( + cli_runner, + mock_async_apis, + mock_api_client, + sample_test_collections, + sample_test_run_execution, + sample_default_config_dict, + has_test_failures=True, + results_summary="1 passed, 1 failed", + ) + + assert result.exit_code != 0 + assert "Results: 1 passed, 1 failed" in result.output + + def test_not_applicable_and_cancelled_do_not_force_nonzero_exit( + self, + cli_runner: CliRunner, + mock_async_apis: Mock, + mock_api_client: Mock, + sample_test_collections: api_models.TestCollections, + sample_test_run_execution: api_models.TestRunExecutionWithChildren, + sample_default_config_dict: dict, + ) -> None: + """[Test Case 3] PICS-inapplicable/cancelled cases alone should not trigger a non-zero exit.""" + result = self._invoke( + cli_runner, + mock_async_apis, + mock_api_client, + sample_test_collections, + sample_test_run_execution, + sample_default_config_dict, + has_test_failures=False, + results_summary="2 passed, 1 not applicable, 1 cancelled", + ) + + assert result.exit_code == 0 + assert "Results: 2 passed, 1 not applicable, 1 cancelled" in result.output + + def test_infrastructure_failure_keeps_cli_error_path(self, cli_runner: CliRunner, mock_api_client: Mock) -> None: + """[Test Case 4] Infrastructure failures still surface as CLIError, not the results summary.""" + with patch("th_cli.commands.run_tests.get_client", return_value=mock_api_client): + with patch("th_cli.commands.run_tests.AsyncApis", side_effect=Exception("connection refused")): + result = cli_runner.invoke(run_tests, ["--tests-list", "TC-ACE-1.1"]) + + assert result.exit_code == 1 + assert "connection refused" in result.output + assert "Results:" not in result.output + + @pytest.mark.unit @pytest.mark.cli class TestParseExtraArgs: @@ -971,6 +1123,8 @@ def test_run_tests_with_extra_args_basic( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act @@ -1012,6 +1166,8 @@ def test_run_tests_with_multiple_extra_args( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act @@ -1067,6 +1223,8 @@ def test_run_tests_without_extra_args( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act @@ -1109,6 +1267,8 @@ def test_run_tests_extra_args_with_config_file( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act @@ -1158,6 +1318,8 @@ def test_run_tests_verify_deep_copy_isolation( mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.has_test_failures.return_value = False + mock_socket.format_results_summary.return_value = "0 test cases executed" mock_socket_class.return_value = mock_socket # Act diff --git a/th_cli/commands/run_tests.py b/th_cli/commands/run_tests.py index d4f354b..b89d129 100644 --- a/th_cli/commands/run_tests.py +++ b/th_cli/commands/run_tests.py @@ -31,9 +31,11 @@ from th_cli.client import get_client from th_cli.colorize import ( colorize_cmd_help, + colorize_error, colorize_header, colorize_help, colorize_key_value, + colorize_success, colorize_warning, italic, set_colors_enabled, @@ -163,7 +165,9 @@ async def run_tests( prompt_timeout: Optional override for the user-prompt response timeout (seconds) Raises: - CLIError: If there are validation or execution errors + CLIError: If there are validation or execution errors (e.g. bad config, API/connection + failures). This is distinct from individual test case failures, which are reported + via the results summary and a non-zero process exit code instead. """ # Extract and parse extra arguments from context (args after --) extra_test_params = _parse_extra_args(list(ctx.args)) if ctx.args else {} @@ -189,6 +193,11 @@ async def run_tests( client = None _webrtc_handler = None + # Set when at least one test case ended in FAILED/ERROR, so run-tests can be used as a + # CI gate. Checked and acted on after the try/except/finally below so that a test-case + # failure is never mistaken for (or reported through) the CLIError/infrastructure-failure + # path. + exit_code = 0 try: client = get_client() async_apis = AsyncApis(client) @@ -328,6 +337,14 @@ async def run_tests( new_test_run = await _start_test_run(async_apis, new_test_run) socket.run = new_test_run await socket_task + + results_summary = socket.format_results_summary() + click.echo("") + if socket.has_test_failures(): + click.echo(colorize_error(f"Results: {results_summary}")) + exit_code = 1 + else: + click.echo(colorize_success(f"Results: {results_summary}")) click.echo(colorize_key_value("Log output in", italic(log_path))) except CLIError: raise # Re-raise CLI errors @@ -342,6 +359,9 @@ async def run_tests( if _webrtc_handler: _webrtc_handler.stop() + if exit_code: + ctx.exit(exit_code) + async def _get_cli_project(async_apis: AsyncApis, project_id: int | None = None) -> m.Project: """Retrieve the project to use for the CLI test run execution. diff --git a/th_cli/test_run/websocket.py b/th_cli/test_run/websocket.py index 887c9be..3ea61ac 100644 --- a/th_cli/test_run/websocket.py +++ b/th_cli/test_run/websocket.py @@ -14,6 +14,7 @@ # limitations under the License. # import asyncio +from collections import Counter import click import websockets @@ -59,6 +60,30 @@ WEBSOCKET_MAX_MESSAGE_SIZE = 32 * 1024 * 1024 # 32MB +# Test case states that represent a final outcome (as opposed to in-progress +# states like "pending"/"executing"/"pending_actuation"). +TERMINAL_TEST_CASE_STATES: frozenset[str] = frozenset( + { + TestStateEnum.PASSED.value, + TestStateEnum.FAILED.value, + TestStateEnum.ERROR.value, + TestStateEnum.NOT_APPLICABLE.value, + TestStateEnum.CANCELLED.value, + } +) + +# Terminal states that should be treated as a test-case failure for exit code purposes. +FAILURE_TEST_CASE_STATES: frozenset[str] = frozenset({TestStateEnum.FAILED.value, TestStateEnum.ERROR.value}) + +# Display order and labels for the end-of-run results summary. +_SUMMARY_STATE_LABELS: list[tuple[str, str]] = [ + (TestStateEnum.PASSED.value, "passed"), + (TestStateEnum.FAILED.value, "failed"), + (TestStateEnum.ERROR.value, "error"), + (TestStateEnum.NOT_APPLICABLE.value, "not applicable"), + (TestStateEnum.CANCELLED.value, "cancelled"), +] + # After the test run reaches a terminal state, the backend may still have a # trailing batch of log records queued/in-flight (it flushes and broadcasts # any pending log entries *after* sending the terminal state update - see @@ -94,6 +119,25 @@ def __init__( # Track test step errors for logging # Key: (suite_index, case_index), Value: list of error strings from all steps self.test_case_step_errors: dict[tuple[int, int], list[str]] = {} + # Track the final state of each test case for the end-of-run summary. + # Key: (suite_index, case_index), Value: final TestStateEnum value. + # A dict (rather than a running counter) so a case that is updated more + # than once with a terminal state is only counted once, in its latest state. + self.test_case_final_states: dict[tuple[int, int], str] = {} + + def test_case_result_counts(self) -> Counter[str]: + """Return a count of test cases by final state (passed/failed/error/...).""" + return Counter(self.test_case_final_states.values()) + + def has_test_failures(self) -> bool: + """Return True if any test case ended in FAILED or ERROR.""" + return any(state in FAILURE_TEST_CASE_STATES for state in self.test_case_final_states.values()) + + def format_results_summary(self) -> str: + """Format the end-of-run test case tally, e.g. '12 passed, 2 failed, 1 error'.""" + counts = self.test_case_result_counts() + parts = [f"{counts[state]} {label}" for state, label in _SUMMARY_STATE_LABELS if counts[state]] + return ", ".join(parts) if parts else "0 test cases executed" async def connect_websocket(self) -> None: try: @@ -279,6 +323,11 @@ def __log_test_case_update(self, update: TestCaseUpdate) -> None: colored_state = colorize_state(update.state.value) click.echo(f" - {colored_title} {colored_state}") + # Tally the case's final state for the end-of-run results summary. + if update.state.value in TERMINAL_TEST_CASE_STATES: + case_key = (update.test_suite_execution_index, update.test_case_execution_index) + self.test_case_final_states[case_key] = update.state.value + # Log any errors when a test case fails if update.state.value in ("failed", "error"): all_errors = []