Add self-deduced git repo construction

As part of the changes for the investigation tab with links to the commits, it was neccessity to find a way to make the repo url self-agnostic, for the test repo as well as the needles. This is what the purpose of `gitrepodir()` in *Utils.pm*. Running that, it will return something like "https://github.com/b10n1k/os-autoinst-distri-opensuse/commit/" constructed by the of the local **.git/config** and the info under the `origin` branch. `origin` might not be the one that is set so this may need to be configurable, but i leave it for later. The reason of this approach is because: - avoid static variables. In this case i could set the variables in the js and ask to use each one correspondingly. - avoid openqa setup for every instance. Similar with the previous but the variables would have to adjust for every instance and finally passed to somehow to the js. Also i stash the variables (*needlesgiturl* and *testgiturl*) inside `_show`. i think this will give access to the variables also for other utulities. However i come up with it because i strungled to stash the variables from `investigation()`. Signed-off-by: ybonatakis <[email protected]>
asdil12 · Nov 15, 2021 · 95f8002 · 95f8002
1 parent a407bd4
commit 95f8002
Show file tree

Hide file tree

Showing 8 changed files with 120 additions and 57 deletions.
diff --git a/assets/javascripts/test_result.js b/assets/javascripts/test_result.js
@@ -72,6 +72,9 @@ const tabConfiguration = {
   next_previous: {}
 };
 
+const DISPLAY_LOG_LIMIT = 5;
+const DISPLAY_LINE_LIMIT = 10;
+
 function checkPreviewVisible(stepPreviewContainer, preview) {
   // scroll the element to the top if the preview is not in view
   if (stepPreviewContainer.offset().top + preview.height() > $(window).scrollTop() + $(window).height()) {
@@ -596,18 +599,15 @@ function setCurrentPreviewFromStepLinkIfPossible(stepLink) {
   }
 }
 
-function githashToLink(value) {
+function githashToLink(value, repo) {
   var hashes_regex = /(\b[0-9a-f]{9}\b)/gms;
   var githashes = value.match(hashes_regex);
-  var repo = 'https://github.com/os-autoinst/os-autoinst-distri-opensuse/commit/';
-  var stat_regex = /(\<a.+?)(?=(\<a))/gms;
-  if (githashes == null) {
-    return;
-  }
+  var statRegex = /(\<a.+?)(?=(\<a))/gms;
+  if (!Array.isArray(githashes)) return;
   for (let i = 0; i < githashes.length; i++) {
     value = value.replace(githashes[i], githashes[i].link(repo + githashes[i]));
   }
-  return value.match(stat_regex);
+  return value.match(statRegex);
 }
 
 function renderTestModules(response) {
@@ -842,50 +842,52 @@ function renderInvestigationTab(response) {
         preElement.appendChild(document.createTextNode(value));
       }
       if (['test_log', 'needles_log'].indexOf(key) >= 0) {
-        var gitstats = githashToLink(value);
-        // assume string 'No test changes..'
-        if (gitstats == null) {
-          preElement.appendChild(document.createTextNode(value));
-        } else {
-          for (let i = 0; i < gitstats.length; i++) {
-            var statItem = document.createElement('div');
-            var collapseSign = document.createElement('a');
-            collapseSign.className = 'collapsed';
-            collapseSign.setAttribute('href', '#collapseEntry' + i);
-            collapseSign.setAttribute('data-toggle', 'collapse');
-            collapseSign.setAttribute('aria-expanded', 'false');
-            collapseSign.setAttribute('aria-controls', 'collapseEntry');
-            collapseSign.innerHTML = '+ ';
-            collapseSign.setAttribute('onclick', 'toggleSign(this)');
-            var spanElem = document.createElement('span');
-            var logDetailsDiv = document.createElement('div');
-            logDetailsDiv.id = 'collapseEntry' + i;
-            logDetailsDiv.className = 'collapse';
-            stats = gitstats[i].split('\n')[0];
-            spanElem.innerHTML = stats;
-            logDetailsDiv.innerHTML = gitstats[i]
-              .split('\n')
-              .slice(1, gitstats.length - 1)
-              .join('\n');
-            statItem.append(collapseSign, spanElem, logDetailsDiv);
-
-            if (i < 5) {
-              preElement.appendChild(statItem);
-            } else {
-              enable_more = true;
-              preElementMore.appendChild(statItem);
+        var repoUrl = getInvestigationDataAttr(key);
+        if (repoUrl) {
+          var gitstats = githashToLink(value, repoUrl);
+          // assume string 'No test changes..'
+          if (gitstats == null) {
+            preElement.appendChild(document.createTextNode(value));
+          } else {
+            for (let i = 0; i < gitstats.length; i++) {
+              var statItem = document.createElement('div');
+              var collapseSign = document.createElement('a');
+              collapseSign.className = 'collapsed';
+              collapseSign.setAttribute('href', '#collapseEntry' + i);
+              collapseSign.setAttribute('data-toggle', 'collapse');
+              collapseSign.setAttribute('aria-expanded', 'false');
+              collapseSign.setAttribute('aria-controls', 'collapseEntry');
+              collapseSign.innerHTML = '+ ';
+              collapseSign.setAttribute('onclick', 'toggleSign(this)');
+              var spanElem = document.createElement('span');
+              var logDetailsDiv = document.createElement('div');
+              logDetailsDiv.id = 'collapseEntry' + i;
+              logDetailsDiv.className = 'collapse';
+              stats = gitstats[i].split('\n')[0];
+              spanElem.innerHTML = stats;
+              logDetailsDiv.innerHTML = gitstats[i]
+                .split('\n')
+                .slice(1, gitstats.length - 1)
+                .join('\n');
+              statItem.append(collapseSign, spanElem, logDetailsDiv);
+
+              if (i < DISPLAY_LOG_LIMIT) {
+                preElement.appendChild(statItem);
+              } else {
+                enable_more = true;
+                preElementMore.appendChild(statItem);
+              }
             }
           }
-        }
-      } else {
-        var textLines = typeof value === 'string' ? value.split('\n') : [value];
-        var textLinesRest;
-
-        var lineLimit = 10;
-        if (textLines.length > lineLimit) {
-          textLinesRest = textLines.slice(lineLimit, textLines.length);
-          textLines = textLines.slice(0, lineLimit);
-          preElement.appendChild(document.createTextNode(textLines.join('\n')));
+        } else {
+          var textLines = typeof value === 'string' ? value.split('\n') : [value];
+          var textLinesRest;
+
+          if (textLines.length > DISPLAY_LINE_LIMIT) {
+            textLinesRest = textLines.slice(DISPLAY_LINE_LIMIT, textLines.length);
+            textLines = textLines.slice(0, DISPLAY_LINE_LIMIT);
+            preElement.appendChild(document.createTextNode(textLines.join('\n')));
+          }
         }
       }
 
@@ -932,11 +934,12 @@ function renderInvestigationTab(response) {
 }
 
 function toggleSign(elem) {
-  if (elem.className === 'collapsed' && elem.innerHTML === '+ ') {
-    elem.innerHTML = '- ';
-  } else {
-    elem.innerHTML = '+ ';
-  }
+  elem.innerHTML = elem.className === 'collapsed' && elem.innerHTML === '+ ' ? '- ' : '+ ';
+}
+
+function getInvestigationDataAttr(key) {
+  var attrs = {test_log: 'data-testgiturl', needles_log: 'data-needlegiturl'};
+  return document.getElementById('investigation').getAttribute(attrs[key]);
 }
 
 function renderDependencyTab(response) {

diff --git a/dependencies.yaml b/dependencies.yaml
@@ -44,6 +44,7 @@ common_requires:
   perl(Storable):
   perl(Time::Moment):
   perl(Try::Tiny):
+  perl(Config::Tiny);
 
 cover_requires:
   perl(Devel::Cover):

diff --git a/lib/OpenQA/Utils.pm b/lib/OpenQA/Utils.pm
@@ -26,6 +26,7 @@ use Exporter 'import';
 use OpenQA::App;
 use OpenQA::Constants qw(VIDEO_FILE_NAME_START VIDEO_FILE_NAME_REGEX FRAGMENT_REGEX);
 use OpenQA::Log qw(log_info log_debug log_warning log_error);
+use Config::Tiny;
 
 # avoid boilerplate "$VAR1 = " in dumper output
 $Data::Dumper::Terse = 1;
@@ -38,6 +39,7 @@ our @EXPORT = qw(
   needledir
   productdir
   testcasedir
+  gitrepodir
   is_in_tests
   save_base64_png
   run_cmd_with_log
@@ -155,6 +157,28 @@ sub testcasedir {
     return $dir;
 }
 
+sub gitrepodir {
+    my %args = (
+        distri  => '',
+        version => '',
+        repo    => 'tests',
+        @_,
+    );
+    my $path
+      = $args{needles}
+      ? needledir($args{distri}, $args{version})
+      : testcasedir($args{distri}, $args{version});
+    my $filename = (-e path($path, '.git')) ? path($path, '.git', 'config') : '';
+    log_warning("$path is not a git directory") if $filename eq '';
+    my $config = Config::Tiny->read($filename, 'utf8');
+    return '' unless defined $config;
+    return $config->{'remote "origin"'}{url} if ($config->{'remote "origin"'}{url} =~ /^http(s?)/);
+    my @url_tokenized = split(':', $config->{'remote "origin"'}{url});
+    $url_tokenized[1] =~ s{\.git$}{/commit/};
+    my @githost = split('@', $url_tokenized[0]);
+    return 'https://' . $githost[1] . '/' . $url_tokenized[1];
+}
+
 sub is_in_tests {
     my ($file) = @_;
 

diff --git a/lib/OpenQA/WebAPI/Controller/Test.pm b/lib/OpenQA/WebAPI/Controller/Test.pm
@@ -405,6 +405,8 @@ sub show {
 sub _show {
     my ($self, $job) = @_;
     return $self->reply->not_found unless $job;
+    my $test_giturl   = gitrepodir(distri => $job->DISTRI);
+    my $needle_giturl = gitrepodir(distri => $job->DISTRI, needles => 1);
 
     $self->stash(
         {
@@ -421,6 +423,8 @@ sub _show {
             show_autoinst_log => $job->should_show_autoinst_log,
             show_investigation => $job->should_show_investigation,
             show_live_tab => $job->state ne DONE,
+            testgiturl => $test_giturl,
+            needlegiturl => $needle_giturl,
         });
     $self->render('test/result');
 }

diff --git a/package.json b/package.json
@@ -12,7 +12,7 @@
   },
   "homepage": "https://github.com/os-autoinst/openQA#readme",
   "devDependencies": {
-    "eslint": "^7.28.0",
+    "eslint": "^7.31.0",
     "eslint-config-prettier": "^8.3.0",
     "eslint-plugin-prettier": "^3.4.0",
     "prettier": "2.3.2"

diff --git a/t/10-jobs.t b/t/10-jobs.t
@@ -476,6 +476,25 @@ subtest 'carry over, including soft-fails' => sub {
         is($inv->{test_log}, 'No test changes recorded, test regression unlikely', 'git log with no test changes');
     };
 
+    subtest 'investigation provides test_log with git stats' => sub {
+        path('t/data/last_good.json')->copy_to(path(($job->_previous_scenario_jobs)[1]->result_dir(), 'vars.json'));
+        path('t/data/first_bad.json')->copy_to(path($job->result_dir(),                               'vars.json'));
+        $job->done;
+        is($job->result, OpenQA::Jobs::Constants::FAILED, 'job result is failed');
+        ok(my $inv = $job->investigate, 'job can provide investigation details');
+        ok($inv,                        'job provides failure investigation');
+        $fake_git_log
+          = "\nqwertyuio0 test0\n mylogfile0 | 1 +\n 1 file changed, 1 insertion(+)\nqwertyuio1 test1\n mylogfile1 | 1 +\n 1 file changed, 1 insertion(+)\n";
+        ok($inv = $job->investigate, 'job investigation ok with test changes');
+        my $actual_lines   = split(/\n/, $inv->{test_log});
+        my $expected_lines = 7;
+        is($actual_lines, $expected_lines, 'test_log have correct number of lines');
+        like($inv->{test_log}, qr/^.*file changed/m, 'git log with test changes');
+        $fake_git_log = '';
+        ok($inv = $job->investigate, 'job investigation ok for no test changes');
+        is($inv->{test_log}, 'No test changes recorded, test regression unlikely', 'git log with no test changes');
+    };
+
     subtest 'external hook is called on done job if specified' => sub {
         my $task_mock = Test::MockModule->new('OpenQA::Task::Job::FinalizeResults', no_auto => 1);
         $task_mock->redefine(

diff --git a/t/16-utils.t b/t/16-utils.t
@@ -334,20 +334,29 @@ subtest 'project directory functions' => sub {
     is resultdir(), '/var/lib/openqa/testresults', 'right directory';
     is assetdir(), '/var/lib/openqa/share/factory', 'right directory';
     is imagesdir(), '/var/lib/openqa/images', 'right directory';
+    is gitrepodir(), '', 'not url when distri is not defined';
 
     local $ENV{OPENQA_BASEDIR} = '/tmp/test';
     is prjdir(), '/tmp/test/openqa', 'right directory';
     is sharedir(), '/tmp/test/openqa/share', 'right directory';
     is resultdir(), '/tmp/test/openqa/testresults', 'right directory';
     is assetdir(), '/tmp/test/openqa/share/factory', 'right directory';
     is imagesdir(), '/tmp/test/openqa/images', 'right directory';
+    my $distri = 'opensuse';
+    is gitrepodir(distri => $distri), '', 'empty when .git is missing';
+    my $mocked_git = path(sharedir . "/tests/opensuse/.git")->make_path;
+    $mocked_git->child('config')
+      ->spurt(qq{[remote "origin"]\n        url = git\@github.com:b10n1k/os-autoinst-distri-opensuse.git});
+    is gitrepodir(distri => $distri) =~ /github\.com.+os-autoinst-distri-opensuse\/commit/, 1, 'correct git url';
 
     local $ENV{OPENQA_SHAREDIR} = '/tmp/share';
     is prjdir(), '/tmp/test/openqa', 'right directory';
     is sharedir(), '/tmp/share', 'right directory';
     is resultdir(), '/tmp/test/openqa/testresults', 'right directory';
     is assetdir(), '/tmp/share/factory', 'right directory';
     is imagesdir(), '/tmp/test/openqa/images', 'right directory';
+    is gitrepodir(), '', 'not url when distri is not defined';
+    is gitrepodir(distri => $distri) =~ /github\.com.+os-autoinst-distri-opensuse\/commit/, 1, 'correct git url';
 };
 
 subtest 'change_sec_to_word' => sub {

diff --git a/templates/webapi/test/result.html.ep b/templates/webapi/test/result.html.ep
@@ -89,8 +89,11 @@
                     <div role="tabpanel" class="tab-pane" id="dependencies" data-src="<%= url_for('test_dependencies', testid => $testid) %>" data-current-job-id="<%= $testid %>"></div>
                 % }
                 % if ($show_investigation) {
-                    <div role="tabpanel" class="tab-pane" id="investigation" data-src="<%= url_for('test_investigation', testid => $testid) %>"></div>
-                % }
+                    <div role="tabpanel" class="tab-pane" id="investigation"
+                data-src="<%= url_for('test_investigation', testid => $testid)
+                %>" data-needlegiturl="<%= stash('needlegiturl') %>"
+                data-testgiturl="<%= stash('testgiturl') %>" ></div>
+                    % }
                 <div role="tabpanel" class="tab-pane" id="comments" data-src="<%= url_for('test_comments', testid => $testid) %>"></div>
                 <div role="tabpanel" class="tab-pane" id="next_previous">
                     %= include 'test/job_next_previous'