cmd/hiveview: Update UI to support line ranges in shared client logs

danceratopz · marioevz · commit 9102c1be782a · 2025-07-02T18:57:50.000Z
Updates the UI to properly display log links for shared clients with line range highlighting. Enhances client detection logic to work with multiple client instances of the same type, and improves the log viewer to properly highlight line ranges.
diff --git a/cmd/hiveview/assets/lib/app-suite.js b/cmd/hiveview/assets/lib/app-suite.js
@@ -317,21 +317,175 @@ function deselectTest(row, closeDetails) {
     history.replaceState(null, null, '#');
 }
 
-function testHasClients(testData) {
-    return testData.clientInfo && Object.getOwnPropertyNames(testData.clientInfo).length > 0;
+function testHasClients(testData, suiteData) {
+    if (testData.clientInfo && Object.getOwnPropertyNames(testData.clientInfo).length > 0) {
+        return true;
+    }
+    
+    if (testData.summaryResult && testData.summaryResult.clientLogs && 
+        Object.keys(testData.summaryResult.clientLogs).length > 0) {
+        return true;
+    }
+    
+    if (suiteData && suiteData.sharedClients) {
+        for (let clientID in suiteData.sharedClients) {
+            let clientName = suiteData.sharedClients[clientID].name;
+            if (testData.name.includes(clientName)) {
+                return true;
+            }
+        }
+    }
+    
+    return false;
 }
 
 // formatClientLogsList turns the clientInfo part of a test into a list of links.
 function formatClientLogsList(suiteData, testIndex, clientInfo) {
     let links = [];
-    for (let instanceID in clientInfo) {
-        let instanceInfo = clientInfo[instanceID];
-        let logfile = routes.resultsRoot + instanceInfo.logFile;
-        let url = routes.clientLog(suiteData.suiteID, suiteData.name, testIndex, logfile);
-        let link = html.makeLink(url, instanceInfo.name);
-        link.classList.add('log-link');
-        links.push(link.outerHTML);
+    let testCase = suiteData.testCases[testIndex];
+    let usedSharedClients = new Set(); // Track which shared clients were used in this test
+    
+    // First, check if the test has specific information about which shared clients it used
+    if (testCase && testCase.summaryResult && testCase.summaryResult.clientLogs) {
+        for (let clientID in testCase.summaryResult.clientLogs) {
+            usedSharedClients.add(clientID);
+        }
+    }
+    
+    // Handle clients listed directly in the test's clientInfo
+    if (clientInfo) {
+        for (let instanceID in clientInfo) {
+            let instanceInfo = clientInfo[instanceID];
+            
+            // Skip if no log file
+            if (!instanceInfo.logFile) {
+                continue;
+            }
+            
+            // If it's a shared client, mark it as used
+            if (instanceInfo.isShared) {
+                usedSharedClients.add(instanceID);
+            }
+            
+            let logfile = routes.resultsRoot + instanceInfo.logFile;
+            let url = routes.clientLog(suiteData.suiteID, suiteData.name, testIndex, logfile);
+            
+            // Check if this is a shared client with a log segment
+            let hasSegment = testCase && 
+                             testCase.summaryResult && 
+                             testCase.summaryResult.clientLogs && 
+                             testCase.summaryResult.clientLogs[instanceID];
+            
+            if (hasSegment) {
+                // If we have a log segment, update the URL to include the line numbers
+                const clientLogInfo = testCase.summaryResult.clientLogs[instanceID];
+                
+                // Use line numbers from the backend
+                url += `#L${clientLogInfo.startLine}-${clientLogInfo.endLine}`;
+            }
+            
+            // Add "(shared)" indicator for shared clients
+            let clientName = instanceInfo.name;
+            if (instanceInfo.isShared || hasSegment) {
+                clientName += " (shared)";
+            }
+            
+            let link = html.makeLink(url, clientName);
+            link.classList.add('log-link');
+            if (instanceInfo.isShared) {
+                link.classList.add('shared-client-log');
+            }
+            links.push(link.outerHTML);
+        }
+    }
+    
+    // For backward compatibility - if test name includes client name, add that client
+    // This handles the case where tests don't yet have clientInfo or clientLogs properly populated
+    if (suiteData.sharedClients) {
+        
+        // First try to match by existing client logs
+        if (usedSharedClients.size === 0) {
+            // Group clients by name to identify if there are multiple of the same type
+            let clientsByName = {};
+            for (let instanceID in suiteData.sharedClients) {
+                let sharedClient = suiteData.sharedClients[instanceID];
+                if (!sharedClient.logFile) continue; // Skip if no log file
+                
+                // Add to the clients by name map
+                if (!clientsByName[sharedClient.name]) {
+                    clientsByName[sharedClient.name] = [];
+                }
+                clientsByName[sharedClient.name].push({id: instanceID, client: sharedClient});
+            }
+            
+            // Now check test name for client match, but only if there's exactly one client of that type
+            for (let clientName in clientsByName) {
+                if (testCase.name.includes(clientName) && clientsByName[clientName].length === 1) {
+                    // If there's exactly one client of this type, it's safe to auto-register
+                    let instanceID = clientsByName[clientName][0].id;
+                    usedSharedClients.add(instanceID);
+                }
+            }
+        }
+        
+        // Now add all the used shared clients that haven't been handled yet
+        for (let instanceID in suiteData.sharedClients) {
+            // First check if this client is explicitly registered in the test's clientLogs
+            // This is the most reliable way to determine if a client was used in a test
+            const explicitlyRegistered = testCase && 
+                                       testCase.summaryResult && 
+                                       testCase.summaryResult.clientLogs && 
+                                       testCase.summaryResult.clientLogs[instanceID];
+            
+            if (explicitlyRegistered) {
+                usedSharedClients.add(instanceID);
+            }
+            
+            // Skip if not used by this test (based on explicit tracking or name matching)
+            if (!usedSharedClients.has(instanceID)) {
+                continue;
+            }
+            
+            // Skip clients already handled in clientInfo
+            if (clientInfo && instanceID in clientInfo) {
+                continue;
+            }
+            
+            let sharedClient = suiteData.sharedClients[instanceID];
+            
+            // Skip if no log file
+            if (!sharedClient.logFile) {
+                continue;
+            }
+            
+            // Create a link to the full log file for this shared client
+            let logfile = routes.resultsRoot + sharedClient.logFile;
+            let url = routes.clientLog(suiteData.suiteID, suiteData.name, testIndex, logfile);
+            
+            // Check if we have specific log segments for this client in this test
+            let hasSegment = testCase && 
+                             testCase.summaryResult && 
+                             testCase.summaryResult.clientLogs && 
+                             testCase.summaryResult.clientLogs[instanceID];
+            
+            if (hasSegment) {
+                // If we have a log segment, update the URL to include the line numbers
+                const clientLogInfo = testCase.summaryResult.clientLogs[instanceID];
+                
+                // Only add line range if we have valid line numbers (both > 0)
+                if (clientLogInfo.startLine > 0 && clientLogInfo.endLine > 0) {
+                    url += `#L${clientLogInfo.startLine}-${clientLogInfo.endLine}`;
+                }
+            }
+            
+            let clientName = sharedClient.name + " (shared)";
+            let link = html.makeLink(url, clientName);
+            
+            link.classList.add('log-link', 'shared-client-log');
+            links.push(link.outerHTML);
+        }
     }
+    
     return links.join(', ');
 }
 
@@ -360,7 +514,7 @@ function formatTestDetails(suiteData, row) {
         p.innerHTML = formatTestStatus(d.summaryResult);
         container.appendChild(p);
     }
-    if (!row.column('logs:name').responsiveHidden() && testHasClients(d)) {
+    if (!row.column('logs:name').responsiveHidden() && testHasClients(d, suiteData)) {
         let p = document.createElement('p');
         p.innerHTML = '<b>Clients:</b> ' + formatClientLogsList(suiteData, d.testIndex, d.clientInfo);
         container.appendChild(p);
diff --git a/cmd/hiveview/assets/lib/app-viewer.js b/cmd/hiveview/assets/lib/app-viewer.js
@@ -12,10 +12,16 @@ import { formatBytes, queryParam } from './utils.js';
 $(document).ready(function () {
     common.updateHeader();
 
-    // Check for line number in hash.
-    var line = null;
-    if (window.location.hash.substr(1, 1) == 'L') {
-        line = parseInt(window.location.hash.substr(2));
+    // Check for line number or line range in hash.
+    var startLine = null;
+    var endLine = null;
+    var hash = window.location.hash.substr(1);
+    if (hash.startsWith('L')) {
+        var range = hash.substr(1).split('-');
+        startLine = parseInt(range[0]);
+        if (range.length > 1) {
+            endLine = parseInt(range[1]);
+        }
     }
 
     // Get suite context.
@@ -33,7 +39,7 @@ $(document).ready(function () {
             showError('Invalid parameters! Missing \'suitefile\' or \'testid\' in URL.');
             return;
         }
-        fetchTestLog(routes.resultsRoot + suiteFile, testIndex, line);
+        fetchTestLog(routes.resultsRoot + suiteFile, testIndex, startLine, endLine);
         return;
     }
 
@@ -42,35 +48,61 @@ $(document).ready(function () {
     if (file) {
         $('#fileload').val(file);
         showText('Loading file...');
-        fetchFile(file, line);
+        fetchFile(file, startLine, endLine);
         return;
     }
 
     // Show default text because nothing was loaded.
     showText(document.getElementById('exampletext').innerHTML);
 });
 
-// setHL sets the highlight on a line number.
-function setHL(num, scroll) {
+// setHL sets the highlight on a line number or range of lines.
+function setHL(startLine, endLine, scroll) {
     // out with the old
     $('.highlighted').removeClass('highlighted');
-    if (!num) {
+    if (!startLine) {
         return;
     }
 
     let contentArea = document.getElementById('file-content');
     let gutter = document.getElementById('gutter');
-    let numElem = gutter.children[num - 1];
-    if (!numElem) {
-        console.error('invalid line number:', num);
-        return;
+    
+    // Calculate the end line if not provided
+    if (!endLine) {
+        endLine = startLine;
+    }
+    
+    // Calculate max available lines and adjust range if needed
+    const maxLines = gutter.children.length;
+    
+    // Check if the requested range is beyond the file size
+    if (startLine > maxLines) {
+        startLine = 1;
+    }
+    
+    if (endLine > maxLines) {
+        endLine = maxLines;
     }
-    // in with the new
-    let lineElem = contentArea.children[num - 1];
-    $(numElem).addClass('highlighted');
-    $(lineElem).addClass('highlighted');
+    
+    // Highlight all lines in the adjusted range
+    for (let num = startLine; num <= endLine; num++) {
+        let numElem = gutter.children[num - 1];
+        if (!numElem) {
+            // Skip invalid line numbers
+            continue;
+        }
+        
+        let lineElem = contentArea.children[num - 1];
+        $(numElem).addClass('highlighted');
+        $(lineElem).addClass('highlighted');
+    }
+    
+    // Scroll to the start of the highlighted range
     if (scroll) {
-        numElem.scrollIntoView();
+        let firstNumElem = gutter.children[startLine - 1];
+        if (firstNumElem) {
+            firstNumElem.scrollIntoView();
+        }
     }
 }
 
@@ -163,12 +195,12 @@ function appendLine(contentArea, gutter, number, text) {
 }
 
 function lineNumberClicked() {
-    setHL($(this).attr('line'), false);
+    setHL(parseInt($(this).attr('line')), null, false);
     history.replaceState(null, null, '#' + $(this).attr('id'));
 }
 
 // fetchFile loads up a new file to view
-async function fetchFile(url, line /* optional jump to line */ ) {
+async function fetchFile(url, startLine, endLine) {
     let resultsRE = new RegExp('^' + routes.resultsRoot);
     let text;
     try {
@@ -181,11 +213,11 @@ async function fetchFile(url, line /* optional jump to line */ ) {
     let title = url.replace(resultsRE, '');
     showTitle(null, title);
     showText(text);
-    setHL(line, true);
+    setHL(startLine, endLine, true);
 }
 
 // fetchTestLog loads the suite file and displays the output of a test.
-async function fetchTestLog(suiteFile, testIndex, line) {
+async function fetchTestLog(suiteFile, testIndex, startLine, endLine) {
     let data;
     try {
         data = await load(suiteFile, 'json');
@@ -221,7 +253,7 @@ async function fetchTestLog(suiteFile, testIndex, line) {
     }
     showTitle('Test:', name);
     showText(logtext);
-    setHL(line, true);
+    setHL(startLine, endLine, true);
 }
 
 async function load(url, dataType) {
diff --git a/hivesim/hive.go b/hivesim/hive.go
@@ -284,6 +284,33 @@ func (sim *Simulation) ExecSharedClient(testSuite SuiteID, clientID string, cmd
 	return resp, err
 }
 
+// RegisterNode registers a client with a test. This is normally handled
+// automatically by StartClient, but can be used directly to register a reference
+// to a shared client.
+func (sim *Simulation) RegisterNode(testSuite SuiteID, test TestID, clientID string, nodeInfo *simapi.NodeInfo) error {
+	if sim.docs != nil {
+		return errors.New("RegisterNode is not supported in docs mode")
+	}
+
+	// We'll use the startClient endpoint with a special parameter to register a shared client
+	var (
+		url  = fmt.Sprintf("%s/testsuite/%d/test/%d/node", sim.url, testSuite, test)
+		config = simapi.NodeConfig{
+			Client:        nodeInfo.Name,
+			SharedClientID: clientID,
+		}
+	)
+
+	// Set up a client setup object to post with files (even though we don't have any files)
+	setup := &clientSetup{
+		files: make(map[string]func() (io.ReadCloser, error)),
+		config: config,
+	}
+
+	var resp simapi.StartNodeResponse
+	return setup.postWithFiles(url, &resp)
+}
+
 // StopClient signals to the host that the node is no longer required.
 func (sim *Simulation) StopClient(testSuite SuiteID, test TestID, nodeid string) error {
 	if sim.docs != nil {
diff --git a/hivesim/testapi.go b/hivesim/testapi.go
