[CP] Websocket and Log Viewer

tests/test_run/test_log_stream_handler.py

@@ -52,11 +52,6 @@ def test_clients_is_empty_set(self):
         assert isinstance(h._clients, set)
         assert len(h._clients) == 0
 
-    def test_log_file_path_initially_none(self):
-        with patch("th_cli.test_run.log_stream_handler.LogsHTTPServer"):
-            h = LogStreamHandler()
-        assert h.log_file_path is None
-
 
 # ---------------------------------------------------------------------------
 # Helpers
@@ -99,16 +94,9 @@ def test_calls_http_server_start(self):
         h, mock_srv = _make_handler()
         with patch.object(h, "_get_local_ip", return_value="10.0.0.1"):
             with patch("th_cli.test_run.log_stream_handler.logger"):
-                h.start(test_run_title="run", log_file_path="/tmp/test.log")
+                h.start(test_run_title="run")
         mock_srv.start.assert_called_once()
 
-    def test_stores_log_file_path(self):
-        h, mock_srv = _make_handler()
-        with patch.object(h, "_get_local_ip", return_value="10.0.0.1"):
-            with patch("th_cli.test_run.log_stream_handler.logger"):
-                h.start(test_run_title="run", log_file_path="/var/log/test.log")
-        assert h.log_file_path == "/var/log/test.log"
-
     def test_already_running_returns_url_without_restart(self):
         h, mock_srv = _make_handler()
         h.is_running = True

tests/test_run/test_logs_http_server.py

@@ -26,7 +26,6 @@
 import pytest
 
 from th_cli.test_run.logs_http_server import (
-    ENDPOINT_DOWNLOAD_LOGS,
     ENDPOINT_LOGS_STREAM,
     ENDPOINT_ROOT,
     ENDPOINT_STATUS,
@@ -98,73 +97,13 @@ def test_stream_endpoint_calls_stream_logs(self):
             h.do_GET()
         mock_fn.assert_called_once()
 
-    def test_download_endpoint_calls_download_logs(self):
-        h = _make_handler(path=ENDPOINT_DOWNLOAD_LOGS)
-        with patch.object(h, "download_logs") as mock_fn:
-            h.do_GET()
-        mock_fn.assert_called_once()
-
     def test_unknown_path_sends_404(self):
         h = _make_handler(path="/nonexistent")
         with patch("th_cli.test_run.logs_http_server.logger"):
             h.do_GET()
         assert h._error_code == 404
 
 
-# ---------------------------------------------------------------------------
-# download_logs()
-# ---------------------------------------------------------------------------
-
-
-@pytest.mark.unit
-class TestDownloadLogs:
-    def test_404_when_no_log_file_path(self):
-        h = _make_handler(server_attrs={"log_file_path": None})
-        with patch("th_cli.test_run.logs_http_server.logger"):
-            h.download_logs()
-        assert h._error_code == 404
-
-    def test_404_when_file_does_not_exist(self, tmp_path):
-        h = _make_handler(server_attrs={"log_file_path": str(tmp_path / "missing.log")})
-        with patch("th_cli.test_run.logs_http_server.logger"):
-            h.download_logs()
-        assert h._error_code == 404
-
-    def test_200_and_correct_headers_for_existing_file(self, tmp_path):
-        log_file = tmp_path / "test.log"
-        log_file.write_bytes(b"log content here")
-        h = _make_handler(server_attrs={"log_file_path": str(log_file)})
-
-        with patch("th_cli.test_run.logs_http_server.logger"):
-            h.download_logs()
-
-        assert h._response_code == 200
-        assert h._headers_sent.get("Content-Type") == "text/plain; charset=utf-8"
-        assert "Content-Disposition" in h._headers_sent
-
-    def test_file_content_written_to_wfile(self, tmp_path):
-        content = b"line 1\nline 2\n"
-        log_file = tmp_path / "run.log"
-        log_file.write_bytes(content)
-        h = _make_handler(server_attrs={"log_file_path": str(log_file)})
-
-        with patch("th_cli.test_run.logs_http_server.logger"):
-            h.download_logs()
-
-        assert h.wfile.getvalue() == content
-
-    def test_handles_broken_pipe_gracefully(self, tmp_path):
-        log_file = tmp_path / "run.log"
-        log_file.write_bytes(b"data")
-        h = _make_handler(server_attrs={"log_file_path": str(log_file)})
-        # Make wfile.write raise BrokenPipeError
-        h.wfile = MagicMock()
-        h.wfile.write.side_effect = BrokenPipeError
-
-        with patch("th_cli.test_run.logs_http_server.logger"):
-            h.download_logs()  # must not raise
-
-
 # ---------------------------------------------------------------------------
 # _send_sse_event()
 # ---------------------------------------------------------------------------
@@ -419,14 +358,13 @@ def test_sets_server_attributes(self):
                         tree_state={},
                         test_run_title="MyTitle",
                         local_ip="1.2.3.4",
-                        log_file_path="/tmp/f.log",
                     )
 
         assert mock_ths.active_clients is clients
         assert mock_ths.clients_lock is lock
         assert mock_ths.test_run_title == "MyTitle"
         assert mock_ths.local_ip == "1.2.3.4"
-        assert mock_ths.log_file_path == "/tmp/f.log"
+        assert mock_ths.run_id is None
 
     def test_propagates_oserror(self):
         srv = LogsHTTPServer(port=0)
@@ -498,24 +436,6 @@ def test_body_contains_run_title_and_start_time(self):
         assert body["start_time"] == "2025-06-01"
 
 
-# ---------------------------------------------------------------------------
-# download_logs() — generic exception path
-# ---------------------------------------------------------------------------
-
-
-@pytest.mark.unit
-class TestDownloadLogsExceptions:
-    def test_handles_generic_exception_gracefully(self, tmp_path):
-        log_file = tmp_path / "test.log"
-        log_file.write_bytes(b"data")
-        h = _make_handler(server_attrs={"log_file_path": str(log_file)})
-        h.wfile = MagicMock()
-        h.wfile.write.side_effect = RuntimeError("unexpected write error")
-        with patch("th_cli.test_run.logs_http_server.logger") as mock_logger:
-            h.download_logs()
-        mock_logger.error.assert_called_once_with("Error serving log file: unexpected write error")
-
-
 # ---------------------------------------------------------------------------
 # _send_sse_event() — generic exception path
 # ---------------------------------------------------------------------------

tests/test_run/test_websocket_socket.py

@@ -28,7 +28,14 @@
     TestSuiteExecution,
     TestSuiteMetadata,
 )
-from th_cli.test_run.socket_schemas import TestCaseUpdate, TestRunUpdate, TestStepUpdate, TestSuiteUpdate, TestUpdate
+from th_cli.test_run.socket_schemas import (
+    TestCaseUpdate,
+    TestLogRecord,
+    TestRunUpdate,
+    TestStepUpdate,
+    TestSuiteUpdate,
+    TestUpdate,
+)
 from th_cli.test_run.websocket import TestRunSocket
 
 # ---------------------------------------------------------------------------
@@ -304,7 +311,7 @@ async def test_step_update_routed_correctly(self):
             ),
         )
         with patch.object(s, "_TestRunSocket__log_test_step_update") as mock_fn:
-            await s._TestRunSocket__handle_test_update(socket=AsyncMock(), update=update)
+            await s._TestRunSocket__handle_test_update(update=update)
 
         mock_fn.assert_called_once()
 
@@ -319,7 +326,7 @@ async def test_case_update_routed_correctly(self):
             body=TestCaseUpdate(state="passed", test_case_execution_index=0, test_suite_execution_index=0),
         )
         with patch.object(s, "_TestRunSocket__log_test_case_update") as mock_fn:
-            await s._TestRunSocket__handle_test_update(socket=AsyncMock(), update=update)
+            await s._TestRunSocket__handle_test_update(update=update)
 
         mock_fn.assert_called_once()
 
@@ -333,28 +340,55 @@ async def test_suite_update_routed_correctly(self):
             body=TestSuiteUpdate(state="passed", test_suite_execution_index=0),
         )
         with patch.object(s, "_TestRunSocket__log_test_suite_update") as mock_fn:
-            await s._TestRunSocket__handle_test_update(socket=AsyncMock(), update=update)
+            await s._TestRunSocket__handle_test_update(update=update)
 
         mock_fn.assert_called_once()
 
     @pytest.mark.asyncio
-    async def test_run_update_executing_does_not_close_socket(self):
+    async def test_run_update_executing_leaves_run_not_finished(self):
         s = _make_socket()
-        mock_socket = AsyncMock()
 
         update = TestUpdate(test_type="test_run", body=TestRunUpdate(state="executing", test_run_execution_id=1))
         with patch.object(s, "_TestRunSocket__log_test_run_update", new_callable=AsyncMock):
-            await s._TestRunSocket__handle_test_update(socket=mock_socket, update=update)
+            await s._TestRunSocket__handle_test_update(update=update)
 
-        mock_socket.close.assert_not_called()
+        assert s._run_finished is False
 
     @pytest.mark.asyncio
-    async def test_run_update_non_executing_closes_socket(self):
+    async def test_run_update_non_executing_marks_run_finished(self):
         s = _make_socket()
-        mock_socket = AsyncMock()
 
         update = TestUpdate(test_type="test_run", body=TestRunUpdate(state="passed", test_run_execution_id=1))
         with patch.object(s, "_TestRunSocket__log_test_run_update", new_callable=AsyncMock):
-            await s._TestRunSocket__handle_test_update(socket=mock_socket, update=update)
+            await s._TestRunSocket__handle_test_update(update=update)
+
+        assert s._run_finished is True
+
+    @pytest.mark.asyncio
+    async def test_run_update_pending_leaves_run_not_finished(self):
+        # Regression test: "pending" is non-terminal (backend's TestRun.completed()
+        # excludes both PENDING and EXECUTING), so it must not close the socket.
+        # A prior implementation used a negation check (`state != "executing"`)
+        # that misclassified any non-"executing" state, including "pending", as
+        # terminal.
+        s = _make_socket()
+
+        update = TestUpdate(test_type="test_run", body=TestRunUpdate(state="pending", test_run_execution_id=1))
+        with patch.object(s, "_TestRunSocket__log_test_run_update", new_callable=AsyncMock):
+            await s._TestRunSocket__handle_test_update(update=update)
+
+        assert s._run_finished is False
+
+    @pytest.mark.asyncio
+    async def test_handle_log_record_logs_every_record(self):
+        s = _make_socket()
+        records = [
+            TestLogRecord(level="INFO", timestamp=0.0, message=f"msg{i}") for i in range(3)
+        ]
+
+        with patch("th_cli.test_run.websocket.logger") as mock_logger:
+            await s._TestRunSocket__handle_log_record(records)
 
-        mock_socket.close.assert_called_once()
+        assert mock_logger.log.call_count == 3
+        for record in records:
+            mock_logger.log.assert_any_call(record.level, record.message)

th_cli/commands/run_tests.py

@@ -316,6 +316,7 @@ async def run_tests(
             execution_pics=execution_pics,
             project_id=project_id,
         )
+        test_logging.set_download_run_id(new_test_run.id)
         if _contains_webrtc_two_way_talk(selected_tests_dict):
             _webrtc_handler = TwoWayTalkHandler(port=8999)
             _webrtc_handler.start_waiting()

th_cli/test_run/log_stream_handler.py

@@ -29,7 +29,7 @@ class LogStreamHandler:
 
     def __init__(self, port: int = 8998):
         """Initialize the log stream handler.
-        
+
         Args:
             port: Port number for the HTTP server (default: 8998)
         """
@@ -42,15 +42,13 @@ def __init__(self, port: int = 8998):
         self.tree_state: dict = {}
         self.tree_lock = threading.Lock()
         self.is_running = False
-        self.log_file_path: Optional[str] = None
 
-    def start(self, test_run_title: str = "Test Execution", log_file_path: Optional[str] = None) -> str:
+    def start(self, test_run_title: str = "Test Execution") -> str:
         """Start the log streaming HTTP server.
-        
+
         Args:
             test_run_title: Title of the test run for display
-            log_file_path: Path to the log file for download functionality
-
+
         Returns:
             URL where logs can be viewed
         """
@@ -59,21 +57,17 @@ def start(self, test_run_title: str = "Test Execution", log_file_path: Optional[
             return self._get_log_viewer_url()
 
         try:
-            # Store log file path for download functionality
-            self.log_file_path = log_file_path
-
             # Get local IP address
             local_ip = self._get_local_ip()
-            
+
             # Start HTTP server
             self.http_server.start(
                 active_clients=self._clients,
                 clients_lock=self._clients_lock,
                 tree_state=self.tree_state,
                 test_run_title=test_run_title,
                 local_ip=local_ip,
-                log_file_path=log_file_path,
-                tree_lock=self.tree_lock
+                tree_lock=self.tree_lock,
             )
 
             self.is_running = True
@@ -87,6 +81,23 @@ def start(self, test_run_title: str = "Test Execution", log_file_path: Optional[
             logger.error(f"Failed to start log stream handler: {e}")
             raise
 
+    def set_run_id(self, run_id: int) -> None:
+        """Tell the HTTP server which run's log to link "Download Logs" to,
+        once the run has been created (its id isn't known when the server
+        starts).
+        """
+        if not self.is_running:
+            return
+
+        self.http_server.set_run_id(run_id)
+
+        # A viewer may already be connected (the run_id is typically set
+        # only *after* the viewer URL was printed and likely opened), so
+        # also push it through the existing SSE stream as a control message
+        # - a future/refreshed page load will pick it up from the HTTP
+        # server attribute above, but an already-open one only sees this.
+        self._broadcast({"type": "run_id", "run_id": run_id})
+
     def stop(self):
         """Stop the log streaming HTTP server."""
         if not self.is_running: