Cap the rendering to 2000 lines and changed log viewer download logs feature

Author: antonio-amjr

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,40 @@ 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_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

@@ -224,6 +224,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

@@ -28,55 +28,57 @@ class LogStreamHandler:
 
     def __init__(self, port: int = 8998):
         """Initialize the log stream handler.
-        
+
         Args:
             port: Port number for the HTTP server (default: 8998)
         """
         self.port = port
         self.http_server = LogsHTTPServer(port=port)
         self.log_queue: queue.Queue = queue.Queue(maxsize=1000)
         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
         """
         if self.is_running:
             logger.warning("Log stream handler already running")
             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(
                 log_queue=self.log_queue,
                 test_run_title=test_run_title,
                 local_ip=local_ip,
-                log_file_path=log_file_path,
             )
-            
+
             self.is_running = True
-            
+
             viewer_url = f"http://{local_ip}:{self.port}"
             logger.info(f"Log stream viewer started: {viewer_url}")
-            
+
             return viewer_url
-            
+
         except Exception as e:
             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 self.is_running:
+            self.http_server.set_run_id(run_id)
 
     def stop(self):
         """Stop the log streaming HTTP server."""

th_cli/test_run/log_viewer.html

@@ -184,6 +184,8 @@
             cursor: pointer;
             transition: all 0.2s;
             font-weight: 500;
+            display: inline-block;
+            text-decoration: none;
         }}
 
         .btn:hover {{
@@ -496,7 +498,7 @@
                     <div class="control-group">
                         <button id="clearBtn" class="btn" onclick="clearLogs()">Clear</button>
                         <button id="autoScrollBtn" class="btn active" onclick="toggleAutoScroll()">Auto-scroll</button>
-                        <button id="downloadBtn" class="btn download" onclick="downloadLogs()">Download Logs</button>
+                        <a id="downloadBtn" class="btn download" href="#" onclick="return false;" title="Available once the run starts" style="opacity:0.5;cursor:default;">Download Logs</a>
                         <input type="text" id="searchBox" class="search-box" placeholder="Search logs..." oninput="filterLogs()">
                         <span class="log-count">Logs: <span id="logCount">0</span></span>
                     </div>
@@ -522,6 +524,14 @@
     </div>
 
     <script>
+        // Injected server-side: the run's id (or null if the run hasn't been
+        // created yet) and the backend's configured hostname (or null if it
+        // was just "localhost"/"127.0.0.1" - meaningless once embedded in a
+        // page that may be opened from a different device than the one
+        // running the CLI). See updateDownloadLink() below.
+        const RUN_ID = {run_id};
+        const BACKEND_HOST = {backend_host};
+
         let eventSource = null;
         let autoScroll = true;
         let logCount = 0;
@@ -538,29 +548,43 @@
         // The "Download Logs" button reads the full file from disk directly,
         // so it isn't affected by this cap.
         const MAX_RETAINED_LOGS = 5000;
+        // Cap how many entries can sit unrendered in the queue. Without this,
+        // a fast burst (hundreds of thousands of lines) makes the queue grow
+        // into a multi-minute backlog: the view still eventually renders
+        // everything, but always shows stale content instead of what's
+        // happening right now - as bad as no live view at all for reading
+        // context to answer a prompt while a test is running. Dropping the
+        // oldest not-yet-rendered entries keeps the view within a few
+        // seconds of real-time, at the cost of some historical entries never
+        // appearing live during a burst (the full history is unaffected -
+        // it's still in the saved log file and via "Download Logs").
+        const MAX_PENDING_QUEUE = 2000;
 
         function connectToLogStream() {{
             const statusIndicator = document.getElementById('statusIndicator');
             const statusText = document.getElementById('statusText');
-            
+
             eventSource = new EventSource('/api/logs/stream');
-            
+
             eventSource.addEventListener('connected', function(e) {{
                 console.log('Connected to log stream');
                 statusIndicator.className = 'status-indicator connected';
                 statusText.textContent = 'Connected';
             }});
-            
+
             eventSource.addEventListener('log', function(e) {{
                 const logEntry = JSON.parse(e.data);
                 logBatchQueue.push(logEntry);
+                if (logBatchQueue.length > MAX_PENDING_QUEUE) {{
+                    logBatchQueue.splice(0, logBatchQueue.length - MAX_PENDING_QUEUE);
+                }}
                 scheduleBatchProcessing();
             }});
-            
+
             eventSource.addEventListener('keepalive', function(e) {{
                 // Silent keepalive
             }});
-            
+
             eventSource.addEventListener('end', function(e) {{
                 console.log('Log stream ended');
                 statusIndicator.className = 'status-indicator disconnected';
@@ -746,69 +770,6 @@
             }});
         }}
 
-        function downloadLogs() {{
-            // First try server download (if server is still running)
-            fetch('/download_logs')
-                .then(response => {{
-                    if (response.ok) {{
-                        return response.blob();
-                    }}
-                    throw new Error('Server not available');
-                }})
-                .then(blob => {{
-                    // Server download successful
-                    const url = window.URL.createObjectURL(blob);
-                    const a = document.createElement('a');
-                    a.href = url;
-                    a.download = 'test_run_logs.log';
-                    document.body.appendChild(a);
-                    a.click();
-                    window.URL.revokeObjectURL(url);
-                    document.body.removeChild(a);
-                }})
-                .catch(error => {{
-                    // Server not available, use client-side download
-                    console.log('Server unavailable, using client-side download');
-                    downloadLogsClientSide();
-                }});
-        }}
-
-        function downloadLogsClientSide() {{
-            // Build log content from stored logs
-            let logContent = '';
-            allLogs.forEach(entry => {{
-                const timestamp = entry.timestamp || new Date().toISOString();
-                const level = entry.level || 'INFO';
-                const message = entry.message || '';
-                // Replace literal \n strings with actual newlines, then add entry
-                const cleanMessage = message.replace(/\\n/g, '\n');
-                logContent += `${{timestamp}} [${{level}}] ${{cleanMessage}}\n`;
-            }});
-
-            if (logContent === '') {{
-                alert('No logs to download');
-                return;
-            }}
-
-            // Create blob and download
-            const blob = new Blob([logContent], {{ type: 'text/plain;charset=utf-8' }});
-            const url = window.URL.createObjectURL(blob);
-            const a = document.createElement('a');
-            a.href = url;
-            
-            // Generate filename with current timestamp
-            const now = new Date();
-            const dateStr = now.toISOString().slice(0,19).replace(/[T:]/g, '-');
-            a.download = `test_run_logs_${{dateStr}}.log`;
-            
-            document.body.appendChild(a);
-            a.click();
-            window.URL.revokeObjectURL(url);
-            document.body.removeChild(a);
-            
-            console.log(`Downloaded ${{allLogs.length}} log entries client-side`);
-        }}
-
         function escapeHtml(text) {{
             const div = document.createElement('div');
             div.textContent = text;
@@ -832,7 +793,27 @@
             }}
         }});
 
+        function updateDownloadLink() {{
+            if (RUN_ID === null) {{
+                return;
+            }}
+            // Prefer the backend's own configured hostname when it's a real
+            // address; otherwise fall back to whatever host this page was
+            // actually loaded from (correct whenever the CLI and backend
+            // run on the same machine, which is the common case - see the
+            // comment on BACKEND_HOST above).
+            const host = BACKEND_HOST || window.location.hostname;
+            const btn = document.getElementById('downloadBtn');
+            btn.href = `${{window.location.protocol}}//${{host}}/api/v1/test_run_executions/${{RUN_ID}}/log`;
+            btn.target = '_blank';
+            btn.removeAttribute('onclick');
+            btn.title = '';
+            btn.style.opacity = '';
+            btn.style.cursor = '';
+        }}
+
         window.addEventListener('load', function() {{
+            updateDownloadLink();
             connectToLogStream();
         }});
 

th_cli/test_run/logging.py

@@ -62,7 +62,7 @@ def configure_logger_for_run(title: str, enable_log_streaming: bool = False) ->
             from th_cli.test_run.log_stream_handler import LogStreamHandler
 
             _log_stream_handler = LogStreamHandler(port=8998)
-            viewer_url = _log_stream_handler.start(test_run_title=title, log_file_path=log_path)
+            viewer_url = _log_stream_handler.start(test_run_title=title)
             # Add custom sink that forwards logs to the stream handler
             def stream_sink(message):
                 """Custom sink that forwards logs to the HTTP stream."""
@@ -91,7 +91,7 @@ def stream_sink(message):
 def stop_log_streaming():
     """Stop the log streaming server if it's running."""
     global _log_stream_handler
-    
+
     if _log_stream_handler:
         try:
             _log_stream_handler.stop()
@@ -102,6 +102,15 @@ def stop_log_streaming():
             _log_stream_handler = None
 
 
+def set_download_run_id(run_id: int) -> None:
+    """Tell the log viewer which run's log to link the "Download Logs" button
+    to, once the run has been created and its id is known (the log-streaming
+    server starts before the run exists, so this can't be known up front).
+    """
+    if _log_stream_handler:
+        _log_stream_handler.set_run_id(run_id)
+
+
 def get_log_stream_url() -> Optional[str]:
     """Get the URL for the log viewer if streaming is enabled.