From b3a019bebef764bf8217791d916b09580d61b7ca Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Tue, 1 Sep 2026 09:42:33 -0300 Subject: [PATCH 1/2] Add results summary and non-zero exit code to run-tests (#1095) After a run completes, run-tests now prints an aggregate count of test cases by final state (e.g. "2 passed, 1 failed") and exits with a non-zero status if any test case ended in FAILED or ERROR, so it can be used as a CI gate. TestRunSocket tracks each test case's final terminal state as case updates stream in, keyed by (suite_index, case_index) so a case that receives more than one terminal update is only counted once, using its latest state. New TestRunSocket methods: - format_results_summary() - e.g. "12 passed, 2 failed, 1 error" - has_test_failures() - True if any case ended FAILED/ERROR (not_applicable/cancelled do not count as failures) - test_case_result_counts() - raw Counter of final states run_tests prints "Results: " after the run finishes and calls ctx.exit(1) if there were failures. The exit-code check sits outside the existing try/except/finally block, since click.exceptions.Exit (raised by ctx.exit()) is itself an Exception subclass and would otherwise be caught and misreported by the existing "except Exception as e: raise CLIError(...)" handler. Infrastructure failures (bad config, API/connection errors, unexpected exceptions) continue to raise CLIError exactly as before, unaffected by this change. Adds test coverage mirroring the issue's acceptance criteria: all tests pass, at least one test fails/errors, mixed states including not_applicable/cancelled, and infrastructure failures leaving the existing CLIError behavior unchanged. --- tests/test_run/test_websocket_socket.py | 79 ++++++++++++ tests/test_run_tests.py | 162 ++++++++++++++++++++++++ th_cli/commands/run_tests.py | 22 +++- th_cli/test_run/websocket.py | 49 +++++++ 4 files changed, 311 insertions(+), 1 deletion(-) 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 = [] From 29abfc697c576b24b1706ec0a880455479a60c47 Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Tue, 8 Sep 2026 12:13:22 -0300 Subject: [PATCH 2/2] Detect and surface incomplete test runs from dropped websocket connections If the websocket connection to the backend closed (cleanly or abruptly) before the test run reached a terminal state, connect_websocket() would silently treat it as a completed run - has_test_failures() and format_results_summary() would report on whatever partial results had been tallied so far as if the run had finished successfully. connect_websocket() now raises IncompleteTestRunError when the connection closes before self._run_finished is set, for both the clean (ConnectionClosedOK) and abrupt (ConnectionClosed) closure paths. This flows through the existing generic exception handler in run_tests() and surfaces as a CLIError instead of a false results summary. As defense in depth, also add TestRunSocket.expected_test_case_count() and check it against the tallied test_case_final_states after the websocket task completes, in case individual test_case updates were dropped despite the run itself reporting a terminal state. --- tests/test_run/test_websocket_connect.py | 168 +++++++++++++++++++++++ tests/test_run_tests.py | 121 ++++++++++++++++ th_cli/commands/run_tests.py | 16 ++- th_cli/test_run/websocket.py | 46 ++++++- 4 files changed, 348 insertions(+), 3 deletions(-) create mode 100644 tests/test_run/test_websocket_connect.py diff --git a/tests/test_run/test_websocket_connect.py b/tests/test_run/test_websocket_connect.py new file mode 100644 index 0000000..bea5e59 --- /dev/null +++ b/tests/test_run/test_websocket_connect.py @@ -0,0 +1,168 @@ +# +# Copyright (c) 2026 Project CHIP Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Unit tests for TestRunSocket.connect_websocket() and expected_test_case_count(). + +Covers the fix for the false-success bug where a websocket connection that +closed (cleanly or abruptly) before the run reached a terminal state was +silently treated as a successfully completed run. +""" + +from unittest.mock import AsyncMock, patch + +import pytest +import websockets.exceptions as ws_exceptions + +from th_cli.api_lib_autogen.models import ( + TestCaseExecution, + TestCaseMetadata, + TestRunExecutionWithChildren, + TestStateEnum, + TestSuiteExecution, + TestSuiteMetadata, +) +from th_cli.test_run.websocket import IncompleteTestRunError, TestRunSocket + +_METADATA_DEFAULTS = dict(description="d", version="1.0", source_hash="x", mandatory=False, id=1) + + +def _make_case(public_id="TC_X_1_1", title="Case", idx=0) -> TestCaseExecution: + return TestCaseExecution( + state=TestStateEnum.passed, + public_id=public_id, + execution_index=idx, + id=idx + 200, + test_suite_execution_id=1, + test_case_metadata_id=1, + test_case_metadata=TestCaseMetadata(public_id=public_id, title=title, **_METADATA_DEFAULTS), + test_step_executions=[], + ) + + +def _make_suite(cases=None, idx=0) -> TestSuiteExecution: + return TestSuiteExecution( + state=TestStateEnum.passed, + public_id="S1", + collection_id="c1", + execution_index=idx, + id=idx + 300, + test_run_execution_id=1, + test_suite_metadata_id=1, + test_case_executions=cases or [], + test_suite_metadata=TestSuiteMetadata(public_id="S1", title="Suite", **_METADATA_DEFAULTS), + ) + + +def _make_run(suites=None) -> TestRunExecutionWithChildren: + return TestRunExecutionWithChildren( + title="Run", id=1, state=TestStateEnum.executing, test_suite_executions=suites or [] + ) + + +def _make_socket(suites=None) -> TestRunSocket: + return TestRunSocket(run=_make_run(suites=suites)) + + +class _FakeWSSocket: + """Fake websocket connection: an async context manager wrapping a mocked recv()/close().""" + + def __init__(self, recv_side_effect): + self.recv = AsyncMock(side_effect=recv_side_effect) + self.close = AsyncMock() + + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc, tb): + return False + + +def _patch_connect(fake_socket): + return patch("th_cli.test_run.websocket.websocket_connect", return_value=fake_socket) + + +# --------------------------------------------------------------------------- +# expected_test_case_count +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestExpectedTestCaseCount: + def test_no_suites(self): + s = _make_socket() + assert s.expected_test_case_count() == 0 + + def test_sums_cases_across_suites(self): + suite1 = _make_suite(cases=[_make_case(idx=0), _make_case(idx=1)]) + suite2 = _make_suite(cases=[_make_case(idx=0)], idx=1) + s = _make_socket(suites=[suite1, suite2]) + assert s.expected_test_case_count() == 3 + + +# --------------------------------------------------------------------------- +# connect_websocket — premature/abrupt closure detection +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestConnectWebsocketIncompleteClosure: + @pytest.mark.asyncio + async def test_clean_close_before_run_finished_raises(self): + """Socket closes cleanly, but no terminal TestRunUpdate was ever received.""" + s = _make_socket() + fake_socket = _FakeWSSocket(recv_side_effect=ws_exceptions.ConnectionClosedOK(None, None)) + + with _patch_connect(fake_socket): + with pytest.raises(IncompleteTestRunError): + await s.connect_websocket() + + # Run never finished, so the explicit close()-on-finish path must not fire. + fake_socket.close.assert_not_called() + + @pytest.mark.asyncio + async def test_clean_close_after_run_finished_does_not_raise(self): + """Existing behavior: clean close during the post-terminal drain period is fine.""" + s = _make_socket() + s._run_finished = True + fake_socket = _FakeWSSocket(recv_side_effect=ws_exceptions.ConnectionClosedOK(None, None)) + + with _patch_connect(fake_socket): + await s.connect_websocket() # must not raise + + fake_socket.close.assert_called_once() + + @pytest.mark.asyncio + async def test_abrupt_close_before_run_finished_raises(self): + """Socket drops (no close handshake) before the run reached a terminal state.""" + s = _make_socket() + fake_socket = _FakeWSSocket(recv_side_effect=ws_exceptions.ConnectionClosedError(None, None)) + + with _patch_connect(fake_socket): + with pytest.raises(IncompleteTestRunError): + await s.connect_websocket() + + fake_socket.close.assert_not_called() + + @pytest.mark.asyncio + async def test_abrupt_close_after_run_finished_does_not_raise(self): + """Existing behavior: a dropped close handshake after the run finished is tolerated.""" + s = _make_socket() + s._run_finished = True + fake_socket = _FakeWSSocket(recv_side_effect=ws_exceptions.ConnectionClosedError(None, None)) + + with _patch_connect(fake_socket): + await s.connect_websocket() # must not raise + + fake_socket.close.assert_called_once() diff --git a/tests/test_run_tests.py b/tests/test_run_tests.py index 7a20b0a..7890082 100644 --- a/tests/test_run_tests.py +++ b/tests/test_run_tests.py @@ -34,6 +34,7 @@ run_tests, ) from th_cli.exceptions import ConfigurationError +from th_cli.test_run.websocket import IncompleteTestRunError @pytest.mark.unit @@ -73,6 +74,8 @@ def test_run_tests_success_minimal_args( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -118,6 +121,8 @@ def test_run_tests_success_with_custom_config( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -170,6 +175,8 @@ def test_run_tests_success_with_pics_config( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -216,6 +223,8 @@ def test_run_tests_success_with_project_id( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -257,6 +266,8 @@ def test_run_tests_success_with_no_color( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -436,6 +447,8 @@ def test_run_tests_api_error_starting_test_run( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -515,6 +528,8 @@ def test_run_tests_various_test_lists( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -558,6 +573,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.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -600,6 +617,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.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -644,6 +663,8 @@ def test_run_tests_default_title_generation( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -695,6 +716,8 @@ def test_run_tests_config_data_processing( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -742,6 +765,8 @@ def test_run_tests_prompt_timeout_merges_into_execution_config( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -790,6 +815,8 @@ def test_run_tests_prompt_timeout_wins_over_config_file( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -882,6 +909,8 @@ def _invoke( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -970,6 +999,88 @@ def test_infrastructure_failure_keeps_cli_error_path(self, cli_runner: CliRunner assert "connection refused" in result.output assert "Results:" not in result.output + def test_incomplete_test_run_error_surfaces_as_cli_error( + 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 5] A dropped websocket mid-run must not be reported as a completed run.""" + 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( + side_effect=IncompleteTestRunError("Websocket connection closed before the test run finished") + ) + mock_socket_class.return_value = mock_socket + + result = cli_runner.invoke(run_tests, ["--tests-list", "TC-ACE-1.1,TC-ACE-1.2"]) + + assert result.exit_code == 1 + assert "Results:" not in result.output + assert "Websocket connection closed before the test run finished" in result.output + + def test_case_count_mismatch_after_finished_run_raises_cli_error( + 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 6] Run reports finished, but fewer case results were tallied than were selected.""" + 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.expected_test_case_count.return_value = 2 + mock_socket.test_case_final_states = {(0, 0): "passed"} + mock_socket_class.return_value = mock_socket + + result = cli_runner.invoke(run_tests, ["--tests-list", "TC-ACE-1.1,TC-ACE-1.2"]) + + assert result.exit_code == 1 + assert "Results:" not in result.output + assert "only 1 of 2 selected" in result.output + @pytest.mark.unit @pytest.mark.cli @@ -1123,6 +1234,8 @@ def test_run_tests_with_extra_args_basic( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -1166,6 +1279,8 @@ def test_run_tests_with_multiple_extra_args( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -1223,6 +1338,8 @@ def test_run_tests_without_extra_args( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -1267,6 +1384,8 @@ def test_run_tests_extra_args_with_config_file( ): mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 @@ -1318,6 +1437,8 @@ def test_run_tests_verify_deep_copy_isolation( mock_socket = Mock() mock_socket.connect_websocket = AsyncMock() + mock_socket.expected_test_case_count.return_value = 0 + mock_socket.test_case_final_states = {} 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 diff --git a/th_cli/commands/run_tests.py b/th_cli/commands/run_tests.py index b89d129..554a21d 100644 --- a/th_cli/commands/run_tests.py +++ b/th_cli/commands/run_tests.py @@ -43,7 +43,7 @@ from th_cli.config import config as th_config from th_cli.exceptions import CLIError, handle_api_error from th_cli.test_run.camera.two_way_talk_handler import TwoWayTalkHandler -from th_cli.test_run.websocket import TestRunSocket +from th_cli.test_run.websocket import IncompleteTestRunError, TestRunSocket from th_cli.utils import ( DEFAULT_CLI_PROJECT_NAME, build_test_selection, @@ -338,6 +338,20 @@ async def run_tests( socket.run = new_test_run await socket_task + # Defense in depth: connect_websocket() already raises IncompleteTestRunError + # if the connection dropped before the run reached a terminal state. This + # covers the other way results can be incomplete - the terminal TestRunUpdate + # arrived, but fewer test_case updates were tallied than were selected (e.g. + # a case update was dropped in transit). Either way, don't trust a partial + # tally as a genuine pass/fail summary. + expected_count = socket.expected_test_case_count() + actual_count = len(socket.test_case_final_states) + if actual_count < expected_count: + raise IncompleteTestRunError( + f"Test run finished, but only {actual_count} of {expected_count} selected " + "test case(s) have results; the run may have been interrupted." + ) + results_summary = socket.format_results_summary() click.echo("") if socket.has_test_failures(): diff --git a/th_cli/test_run/websocket.py b/th_cli/test_run/websocket.py index 3ea61ac..d6e1b98 100644 --- a/th_cli/test_run/websocket.py +++ b/th_cli/test_run/websocket.py @@ -104,6 +104,18 @@ NON_TERMINAL_RUN_STATES = (TestStateEnum.PENDING, TestStateEnum.EXECUTING) +class IncompleteTestRunError(RuntimeError): + """Raised when the test run ends without confirmed, complete results. + + Covers two cases: the websocket connection closed (cleanly or not) before + a terminal TestRunUpdate was received, and the terminal update arrived but + fewer test case results were tallied than the run selected - e.g. because + individual case updates were dropped. Either way, run_tests() should treat + this as an infrastructure failure (CLIError) rather than trust whatever + partial results were collected as a genuine pass/fail summary. + """ + + class TestRunSocket: def __init__( self, @@ -129,6 +141,16 @@ 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 expected_test_case_count(self) -> int: + """Return the total number of test cases selected for this run. + + Used as a sanity check against len(test_case_final_states): if the + run reports a terminal state but fewer case results were tallied + than were selected, some test_case updates were dropped and the + results summary can't be trusted as complete. + """ + return sum(len(suite.test_case_executions or []) for suite in self.run.test_suite_executions or []) + 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()) @@ -140,6 +162,11 @@ def format_results_summary(self) -> str: return ", ".join(parts) if parts else "0 test cases executed" async def connect_websocket(self) -> None: + # Set when the connection closes (cleanly or not) before self._run_finished + # is True, i.e. before a terminal TestRunUpdate was ever received. Checked + # once at the end so we still run the existing cleanup (socket.close(), + # ConnectionClosed suppression) unchanged before deciding whether to raise. + incomplete_closure = False try: async with websocket_connect( WEBSOCKET_URL, @@ -160,6 +187,11 @@ async def connect_websocket(self) -> None: else: message = await socket.recv() except websockets.exceptions.ConnectionClosedOK: + if not self._run_finished: + # Connection closed cleanly, but before the run + # reached a terminal state - the run itself was + # interrupted, not just the drain period. + incomplete_closure = True break except asyncio.TimeoutError: # No more trailing messages arrived during the @@ -188,10 +220,20 @@ async def connect_websocket(self) -> None: # This is acceptable as test run completed successfully pass except websockets.exceptions.ConnectionClosed: - # Handle case where backend doesn't complete close handshake properly + # Handle case where backend doesn't complete close handshake properly. # This can happen with long-running test executions # Error: "sent 1000 (OK); no close frame received" - pass + # But if it happened before the run finished, the connection was + # actually dropped mid-run - flag it instead of treating it as a + # benign handshake quirk. + if not self._run_finished: + incomplete_closure = True + + if incomplete_closure: + raise IncompleteTestRunError( + "Websocket connection closed before the test run reached a terminal state; " + "results may be incomplete." + ) async def __handle_incoming_socket_message(self, socket: WebSocketClientProtocol, message: SocketMessage) -> None: if isinstance(message.payload, TestUpdate):