Merge pull request #5873 from Martchus/log-lines

Add line numbers with anchoring in log viewer

assets/javascripts/openqa.js

@@ -83,7 +83,8 @@ function updateQueryParams(params) {
   if (!history.replaceState) {
     return; // skip if not supported
   }
-  var search = [];
+  const search = [];
+  const hash = document.location.hash;
   $.each(params, function (key, values) {
     $.each(values, function (index, value) {
       if (value === undefined) {
@@ -93,7 +94,7 @@ function updateQueryParams(params) {
       }
     });
   });
-  history.replaceState({}, document.title, window.location.pathname + '?' + search.join('&'));
+  history.replaceState({}, document.title, `?${search.join('&')}${hash}`);
 }
 
 function renderDataSize(sizeInByte) {

assets/javascripts/test_result.js

@@ -358,7 +358,7 @@ function handleKeyDownOnTestDetails(e) {
 function setPageHashAccordingToCurrentTab(tabNameOrHash, replace) {
   // don't mess with #step hashes within details tab
   const currentHash = window.location.hash;
-  if (tabNameOrHash === 'details' && currentHash.search('#step/') === 0) {
+  if (tabNameOrHash === 'details' && (currentHash.startsWith('#step/') || currentHash.startsWith('#line-'))) {
     return;
   }
 
@@ -459,15 +459,18 @@ function activateTabAccordingToHashChange() {
   }
 
   // check for tabs, steps or comments matching the hash
-  var link = $("[href='" + hash + "'], [data-href='" + hash + "']");
-  var tabName = hash.substr(1);
-  if (hash.search('#step/') === 0) {
-    setCurrentPreviewFromStepLinkIfPossible(link);
+  let link = $(`[href='${hash}'], [data-href='${hash}']`);
+  let tabName = hash.substr(1);
+  let isStep = hash.startsWith('#step/');
+  if (hash.startsWith('#line-') || isStep) {
+    if (isStep) {
+      setCurrentPreviewFromStepLinkIfPossible(link);
+      // note: It is not a problem if the details haven't been loaded so far. Once the details become available the hash
+      //       is checked again and the exact step preview will be shown.
+    }
     link = $("[href='#details']");
     tabName = 'details';
-    // note: It is not a problem if the details haven't been loaded so far. Once the details become available the hash
-    //       is checked again and the exact step preview will be shown.
-  } else if (hash.search('#comment-') === 0) {
+  } else if (hash.startsWith('#comment-')) {
     link = $("[href='#comments']");
     tabName = 'comments';
   } else if (link.attr('role') !== 'tab' || link.prop('aria-expanded')) {
@@ -586,74 +589,117 @@ function delay(callback, ms) {
   };
 }
 
-function filterLogLines(input) {
+function filterLogLines(input, viaSearchBox = true) {
   if (input === undefined) {
     return;
   }
   const string = input.value;
-  let regex = undefined;
-  const match = string.match(/^\/(.*)\/([i]*)$/);
-  if (match) {
-    regex = new RegExp(match[1], match[2]);
+  if (string === input.dataset.lastString) {
+    // abort if the value does not change which can happen because there are multiple event handlers calling this function
+    return;
   }
+  const match = string.match(/^\/(.*)\/([i]*)$/);
+  const regex = match ? new RegExp(match[1], match[2]) : undefined;
+  input.dataset.lastString = string;
   displaySearchInfo('Searching…');
   $('.embedded-logfile').each(function (index, logFileElement) {
-    let content = logFileElement.dataset.content;
+    const content = logFileElement.content;
     if (content === undefined) {
       return;
     }
+    const lines = Array.from(content);
+    let lineNumber = 0;
+    let matchingLines = 0;
     if (string.length > 0) {
-      const lines = content.split(/\r?\n/);
-      const wanted = [];
       for (const line of lines) {
-        if (regex) {
-          // For searching for /^something/ we need to remove ansi control characters
-          const text = ansiToText(line);
-          if (text.match(regex)) {
-            wanted.push(line);
-          }
-          continue;
-        }
-        if (line.includes(string)) {
-          wanted.push(line);
+        const lineAsText = ansiToText(line);
+        if (regex ? lineAsText.match(regex) : lineAsText.includes(string)) {
+          ++matchingLines;
+        } else {
+          lines[lineNumber] = undefined;
         }
+        ++lineNumber;
       }
-      content = wanted.join('\n');
-      displaySearchInfo(`Showing ${wanted.length} / ${lines.length} lines`);
+      displaySearchInfo(`Showing ${matchingLines} / ${lineNumber} lines`);
     } else {
       displaySearchInfo('');
     }
-    logFileElement.innerHTML = ansiToHtml(content);
+    showLogLines(logFileElement, lines, viaSearchBox);
   });
-  const fullCurrentUrl = window.location.href;
-  const urlParts = fullCurrentUrl.split('#');
-  const currentUrl = urlParts[0];
-  const fragment = urlParts[1];
-  if (string.length > 0) {
-    window.location.href = `${currentUrl}#filter=${encodeURIComponent(string)}`;
-  } else if (fragment) {
-    // leaving off the # here would reload the page
-    window.location.href = currentUrl + '#';
-  }
+  const params = parseQueryParams();
+  string.length > 0 ? (params.filter = [string]) : delete params.filter;
+  updateQueryParams(params);
 }
 
 function filterEmbeddedLogFiles() {
   const searchBox = document.getElementById('filter-log-file');
   if (searchBox) {
-    const currentUrl = window.location.href;
-    const fragment = currentUrl.split('#')[1];
-    if (fragment) {
-      const params = fragment.split('&');
-      for (let i = 0; i < params.length; i++) {
-        const keyval = params[i].split('=');
-        if (keyval[0] === 'filter') {
-          searchBox.value = decodeURIComponent(keyval[1]);
-        }
+    const filterParam = parseQueryParams().filter?.[0];
+    if (filterParam !== undefined) {
+      searchBox.value = filterParam;
+    }
+  }
+  loadEmbeddedLogFiles(filterLogLines.bind(null, searchBox, false));
+}
+
+function showLogLines(logFileElement, lines, viaSearchBox = false) {
+  const tableElement = document.createElement('table');
+  const currentHash = document.location.hash;
+  let lineNumber = 0;
+  let currentLineElement = undefined;
+  logFileElement.innerHTML = '';
+  for (const line of lines) {
+    ++lineNumber;
+    if (line === undefined) {
+      continue;
+    }
+    const lineElement = document.createElement('tr');
+    const lineNumberElement = document.createElement('td');
+    const lineNumberLinkElement = document.createElement('a');
+    const lineContentElement = document.createElement('td');
+    const hash = '#' + (lineElement.id = 'line-' + lineNumber);
+    lineNumberLinkElement.href = hash;
+    lineNumberLinkElement.onclick = () => {
+      if (currentLineElement !== undefined) {
+        currentLineElement.classList.remove('line-current');
       }
+      lineContentElement.classList.add('line-current');
+      currentLineElement = lineContentElement;
+    };
+    lineNumberLinkElement.append(lineNumber);
+    lineNumberElement.className = 'line-number';
+    lineContentElement.className = 'line-content';
+    if (hash === currentHash) {
+      lineNumberLinkElement.onclick();
     }
+    lineContentElement.innerHTML = ansiToHtml(line);
+    lineNumberElement.appendChild(lineNumberLinkElement);
+    lineElement.append(lineNumberElement, lineContentElement);
+    tableElement.appendChild(lineElement);
   }
-  const filter = filterLogLines.bind(null, searchBox);
-  loadEmbeddedLogFiles(filter);
+  logFileElement.appendChild(tableElement);
+
+  // trigger the current hash again or delete it if no longer valid
+  if (currentLineElement && !viaSearchBox) {
+    currentLineElement.scrollIntoView();
+  } else if (currentHash.startsWith('line-') && !currentLineElement) {
+    document.location.hash = '';
+  }
+
+  // setup event handler to update the current line when the hash changes
+  if (window.hasHandlerForUpdatingCurrentLine) {
+    return;
+  }
+  addEventListener('hashchange', event => {
+    const hash = document.location.hash;
+    if (hash.startsWith('#line-')) {
+      const lineNumberLinkElement = document.querySelector(hash + ' .line-number a');
+      if (lineNumberLinkElement) {
+        lineNumberLinkElement.onclick();
+      }
+    }
+  });
+  window.hasHandlerForUpdatingCurrentLine = true;
 }
 
 function loadEmbeddedLogFiles(filter) {
@@ -663,12 +709,8 @@ function loadEmbeddedLogFiles(filter) {
     }
     $.ajax(logFileElement.dataset.src)
       .done(function (response) {
-        logFileElement.dataset.content = response;
-        if (filter) {
-          filter();
-        } else {
-          logFileElement.innerHTML = ansiToHtml(response);
-        }
+        const lines = (logFileElement.content = response.split(/\r?\n/));
+        filter ? filter() : showLogLines(logFileElement, lines, false);
         logFileElement.dataset.contentsLoaded = true;
       })
       .fail(function (jqXHR, textStatus, errorThrown) {
@@ -682,7 +724,7 @@ window.onload = function () {
   if (!searchBox) {
     return;
   }
-  const filter = filterLogLines.bind(null, searchBox);
+  const filter = filterLogLines.bind(null, searchBox, true);
   searchBox.addEventListener('keyup', delay(filter), 1000);
   searchBox.addEventListener('change', filter, false);
   searchBox.addEventListener('search', filter, false);

assets/stylesheets/test-details.scss

@@ -289,10 +289,30 @@ span.resborder_na {
     max-height: none;
     box-shadow: inset 0 2px 2px rgba(0, 0, 0, 0.2);
 }
-pre.embedded-logfile {
+
+.embedded-logfile {
     white-space: pre-wrap;
+    font-family: var(--bs-font-monospace);
+    font-size: 0.875em;
+    overflow: auto;
+
+    .line-number {
+        background-color: rgba(var(--dt-row-stripe), 0.05);
+        border-right: 1px solid rgba(var(--dt-row-stripe), 0.15);
+        text-align: right;
+        vertical-align: top;
+        padding-left: 3px;
+        padding-right: 3px;
+        user-select: none;
+    }
+    .line-content {
+        padding-left: 10px;
+    }
+    .line-current {
+        background-color: var(--bs-warning-bg-subtle);
+    }
 }
-[data-bs-theme="dark"] pre.embedded-logfile {
+[data-bs-theme="dark"] .embedded-logfile {
     color: $default-font-color-dark;
 }
 

t/ui/18-tests-details.t

@@ -13,6 +13,7 @@ use Test::Warnings qw(:all :report_warnings);
 use Mojo::JSON qw(decode_json encode_json);
 use Mojo::File qw(path);
 use Mojo::IOLoop;
+use Mojo::URL;
 use OpenQA::Test::TimeLimit '40';
 use OpenQA::Test::Case;
 use OpenQA::Test::Utils qw(prepare_clean_needles_dir prepare_default_needle);
@@ -277,7 +278,8 @@ subtest 'reason and log details on incomplete jobs' => sub {
     is(current_tab, 'Details', 'starting on Details tab also for incomplete jobs');
     like($driver->find_element('#info_box')->get_text(), qr/Reason: just a test/, 'reason shown');
     wait_for_ajax(msg => 'test details tab for job 99926 loaded');
-    my $log_element = $driver->find_element_by_xpath('//*[@id="details"]//pre[string-length(text()) > 0]');
+    my $log_element_path = '//*[@id="details"]//*[contains(@class, "embedded-logfile")][table]';
+    my $log_element = $driver->find_element_by_xpath($log_element_path);
     like($log_element->get_attribute('data-src'), qr/autoinst-log.txt/, 'log file embedded');
     like($log_element->get_text(), qr/Crashed\?/, 'log contents loaded');
     $driver->find_element_by_link_text('Investigation')->click;
@@ -495,6 +497,20 @@ subtest 'misc details: title, favicon, go back, go to source view, go to log vie
     $driver->find_element('#filter-log-file')->send_keys('/kate-[12]/');
     like $driver->find_element('#filter-info')->get_text, qr{Showing 2 / 1292 lines},
       'Showing filter result info for regex';
+
+    my $url = Mojo::URL->new($driver->execute_script('return document.location.toString()'));
+    is $url->query->param('filename'), 'autoinst-log.txt', 'URL still contains filename after filtering';
+    is $url->query->param('filter'), '/kate-[12]/', 'URL contains filter';
+    is $url->fragment, undef, 'URL contains no fragment';
+    wait_for_element
+      selector => '#line-68 .line-current',
+      is_displayed => 1,
+      description => 'clicked line highlighted',
+      trigger_function => sub () { $driver->find_element('#line-68 a')->click };
+    $url = Mojo::URL->new($driver->execute_script('return document.location.toString()'));
+    is $url->query->param('filename'), 'autoinst-log.txt', 'URL still contains filename after clicking on line number';
+    is $url->query->param('filter'), '/kate-[12]/', 'URL still contains filter after clicking on line number';
+    is $url->fragment, 'line-68', 'URL fragment points to clicked line';
 };
 
 my $t = Test::Mojo->new('OpenQA::WebAPI');
@@ -731,7 +747,7 @@ subtest 'additional investigation notes provided on new failed' => sub {
     wait_for_ajax(msg => 'details tab for job 99947 loaded to test investigation');
     $driver->find_element('#clones a')->click;    # navigates to 99982
     $driver->find_element_by_link_text('Investigation')->click;
-    $driver->find_element('table#investigation_status_entry')
+    $driver->find_element('#investigation_status_entry')
       ->text_like(qr/No result dir/, 'investigation status content shown as table');
 };
 
@@ -748,7 +764,7 @@ subtest 'alert box shown if not already on first bad' => sub {
 
     $driver->find_element_by_xpath("//div[\@class='alert alert-info']/a[\@class='alert-link']")->click;
     wait_for_ajax(msg => 'details tab for job 99938 loaded to test investigation');
-    $driver->find_element('table#investigation_status_entry')
+    $driver->find_element('#investigation_status_entry')
       ->text_like(qr/error\nNo previous job in this scenario, cannot provide hints/,
         'linked to investigation tab directly');
     $driver->find_element_by_xpath("//div[\@class='tab-content']")

templates/webapi/test/autoinst_log_within_details.html.ep

@@ -1,4 +1,4 @@
 <div>
     Likely error from autoinst-log.txt:<br>
-    <pre class="embedded-logfile" data-src="<%= url_for('test_file', testid => $testid, filename => 'autoinst-log.txt') %>"></pre>
+    <div class="embedded-logfile" data-src="<%= url_for('test_file', testid => $testid, filename => 'autoinst-log.txt') %>"></div>
 </div>

templates/webapi/test/logfile.html.ep

@@ -21,6 +21,6 @@
 </div>
 <p><%= $filename %></p>
 <hr>
-<pre class="embedded-logfile" data-src="<%= $url %>">
+<div class="embedded-logfile" data-src="<%= $url %>">
 Loading log contents …
-</pre>
+</div>