Merge pull request #1194 from Martchus/todo

Improve 'TODO' filter
os-autoinst · Feb 8, 2017 · bb33f7c · bb33f7c
2 parents 492e5a4 + 7984956
commit bb33f7c
Show file tree

Hide file tree

Showing 6 changed files with 64 additions and 29 deletions.
diff --git a/lib/OpenQA/BuildResults.pm b/lib/OpenQA/BuildResults.pm
@@ -47,7 +47,7 @@ sub count_job {
         }
         if ($job->result eq OpenQA::Schema::Result::Jobs::SOFTFAILED) {
             $jr->{softfailed}++;
-            if (@{$job->failed_modules}) {
+            if ($job->has_failed_modules) {
                 $jr->{softfailed_with_failed_modules}++;
                 $jr->{labeled_softfailed}++ if $labels->{$job->id};
             }

diff --git a/lib/OpenQA/Schema/Result/Jobs.pm b/lib/OpenQA/Schema/Result/Jobs.pm
@@ -1094,16 +1094,21 @@ sub create_asset {
     return $abs;
 }
 
+sub has_failed_modules {
+    my ($self) = @_;
+    return $self->modules->count({result => 'failed'}, {rows => 1});
+}
+
 sub failed_modules {
     my ($self) = @_;
 
     my $fails = $self->modules->search({result => 'failed'});
-    my $failedmodules = [];
+    my @failedmodules;
 
     while (my $module = $fails->next) {
-        push @$failedmodules, $module->name;
+        push(@failedmodules, $module->name);
     }
-    return $failedmodules;
+    return \@failedmodules;
 }
 
 sub update_status {

diff --git a/lib/OpenQA/WebAPI/Controller/Test.pm b/lib/OpenQA/WebAPI/Controller/Test.pm
@@ -344,10 +344,10 @@ sub _job_labels {
     my ($self, $jobs) = @_;
 
     my %labels;
-    my $c
-      = $self->db->resultset("Comments")->search({job_id => {in => [map { $_->id } @$jobs]}}, {order_by => 'me.id'});
+    my $comments
+      = $self->db->resultset('Comments')->search({job_id => {in => [map { $_->id } @$jobs]}}, {order_by => 'me.id'});
     # previous occurences of bug or label are overwritten here.
-    while (my $comment = $c->next()) {
+    while (my $comment = $comments->next()) {
         my $bugrefs = $comment->bugrefs;
         if (@$bugrefs) {
             push(@{$labels{$comment->job_id}{bugs} //= []}, @$bugrefs);
@@ -381,54 +381,61 @@ sub prepare_job_results {
 
     # prefetch descriptions from test suites
     my %desc_args = (name => {in => \@job_names});
-    my @descriptions = $self->db->resultset("TestSuites")->search(\%desc_args, {columns => [qw(name description)]});
+    my @descriptions = $self->db->resultset('TestSuites')->search(\%desc_args, {columns => [qw(name description)]});
     my %descriptions = map { $_->name => $_->description } @descriptions;
 
+    my $todo = $self->param('todo');
     foreach my $job (@$jobs) {
+        my $jobid  = $job->id;
         my $test   = $job->TEST;
         my $flavor = $job->FLAVOR || 'sweet';
         my $arch   = $job->ARCH || 'noarch';
 
         my $result;
-        if ($job->state eq 'done') {
-            my $result_stats = $all_result_stats->{$job->id};
+        if ($job->state eq OpenQA::Schema::Result::Jobs::DONE) {
+            my $result_stats = $all_result_stats->{$jobid};
             my $overall      = $job->result;
-            if ($job->result eq "passed") {
-                next if $self->param('todo');
-            }
-            if ($self->param('todo')) {
-                next if $job_labels->{$job->id}{bugs} || $job_labels->{$job->id}{label};
+
+            if ($todo) {
+                # skip all jobs NOT needed to be labeled for the black certificate icon to show up
+                next
+                  if $job->result eq OpenQA::Schema::Result::Jobs::PASSED
+                  || $job_labels->{$jobid}{bugs}
+                  || $job_labels->{$jobid}{label}
+                  || ( $job->result eq OpenQA::Schema::Result::Jobs::SOFTFAILED
+                    && $job->has_failed_modules);
             }
+
             $result = {
                 passed   => $result_stats->{passed},
                 unknown  => $result_stats->{unk},
                 failed   => $result_stats->{failed},
                 overall  => $overall,
-                jobid    => $job->id,
-                state    => 'done',
+                jobid    => $jobid,
+                state    => OpenQA::Schema::Result::Jobs::DONE,
                 failures => $job->failed_modules(),
-                bugs     => $job_labels->{$job->id}{bugs},
-                label    => $job_labels->{$job->id}{label},
-                comments => $job_labels->{$job->id}{comments},
+                bugs     => $job_labels->{$jobid}{bugs},
+                label    => $job_labels->{$jobid}{label},
+                comments => $job_labels->{$jobid}{comments},
             };
             $aggregated->{$overall}++;
         }
-        elsif ($job->state eq 'running') {
-            next if $self->param('todo');
+        elsif ($job->state eq OpenQA::Schema::Result::Jobs::RUNNING) {
+            next if $todo;
             $result = {
-                state => "running",
-                jobid => $job->id,
+                state => OpenQA::Schema::Result::Jobs::RUNNING,
+                jobid => $jobid,
             };
             $aggregated->{running}++;
         }
         else {
-            next if $self->param('todo');
+            next if $todo;
             $result = {
                 state    => $job->state,
-                jobid    => $job->id,
+                jobid    => $jobid,
                 priority => $job->priority,
             };
-            if ($job->state eq 'scheduled') {
+            if ($job->state eq OpenQA::Schema::Result::Jobs::SCHEDULED) {
                 $aggregated->{scheduled}++;
             }
             else {

diff --git a/lib/OpenQA/WebAPI/Plugin/Helpers.pm b/lib/OpenQA/WebAPI/Plugin/Helpers.pm
@@ -251,7 +251,9 @@ sub register {
             my $class = 'help_popover fa fa-question-circle';
             if ($details_url) {
                 $content
-                  .= '<p>See ' . $c->link_to($details_text ? $details_text : here => $details_url) . ' for details</p>';
+                  .= '<p>See '
+                  . $c->link_to($details_text ? $details_text : here => $details_url, target => 'blank')
+                  . ' for details</p>';
             }
             my $data = {toggle => 'popover', trigger => 'focus', title => $title, content => $content};
             $data->{placement} = $placement if $placement;

diff --git a/t/10-tests_overview.t b/t/10-tests_overview.t
@@ -154,6 +154,23 @@ $form = {distri => 'opensuse', version => 'Factory', build => '0048', todo => 1}
 $get = $t->get_ok('/tests/overview' => form => $form)->status_is(200);
 like(get_summary, qr/Passed: 0 Soft Failure: 2 Failed: 1/i, 'todo=1 shows all unlabeled failed');
 
+# add a failing module to one of the softfails to test 'TODO' option
+my $failing_module = $t->app->db->resultset('JobModules')->create(
+    {
+        script   => 'tests/x11/failing_module.pm',
+        job_id   => 99936,
+        category => 'x11',
+        name     => 'failing_module',
+        result   => 'failed'
+    });
+
+$get = $t->get_ok('/tests/overview' => form => {distri => 'opensuse', version => 'Factory', build => '0048', todo => 1})
+  ->status_is(200);
+like(get_summary, qr/Passed: 0 Soft Failure: 1 Failed: 1/i, 'todo=1 shows all unlabeled failed');
+$t->element_exists('#res-99939', 'softfailed without failing module present');
+$t->element_exists_not('#res-99936', 'softfailed with failing module filtered out');
+$failing_module->delete();
+
 # multiple groups can be shown at the same time
 $get = $t->get_ok('/tests/overview?distri=opensuse&version=13.1&groupid=1001&groupid=1002&build=0091')->status_is(200);
 $summary = get_summary;

diff --git a/templates/test/overview.html.ep b/templates/test/overview.html.ep
@@ -52,7 +52,7 @@
     </div>
     <div class="panel panel-default" id="filter-panel">
         <div class="panel-heading"><strong>Filter</strong>
-            <%= help_popover('Help for the <i>test overview</i>' => '
+            <%= help_popover('Help for the <em>test overview</em>' => '
                     <p>This page shows an overview of job results in a matrix
                     view. Only the latest job for each scenario is shown. The
                     view can be configured based on query parameters which can
@@ -91,6 +91,10 @@
                 <div class="form-group">
                     <strong>Misc</strong>
                     <label><input value="1" name="todo" type="checkbox" id="filter-todo"> TODO</label>
+                    <%= help_popover('Help for the <em>TODO</em>-filter' => '
+                        <p>Shows only jobs that need to be labeled for the black review badge to show up</p>',
+                        'http://open.qa/docs/#_review_badges' => 'documentation about review badges')
+                    %>
                 </div>
                 <button type="submit" class="btn btn-default">Apply</button>
             </form>