From 2ac47e8013bc0dc8fd72b6b0bda56ff8cb45a799 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Wed, 16 Jun 2021 11:20:06 -0400
Subject: [PATCH 1/5] Test that each page listing evals works

PR #860 caused a regression that broke some loads.
---
 t/Controller/Build/evals.t  | 33 +++++++++++++++++++++++++++++++++
 t/Controller/Jobset/evals.t | 34 ++++++++++++++++++++++++++++++++++
 t/Controller/Root/evals.t   | 29 +++++++++++++++++++++++++++++
 3 files changed, 96 insertions(+)
 create mode 100644 t/Controller/Build/evals.t
 create mode 100644 t/Controller/Jobset/evals.t
 create mode 100644 t/Controller/Root/evals.t

diff --git a/t/Controller/Build/evals.t b/t/Controller/Build/evals.t
new file mode 100644
index 00000000..f44d5733
--- /dev/null
+++ b/t/Controller/Build/evals.t
@@ -0,0 +1,33 @@
+use strict;
+use Setup;
+use Data::Dumper;
+my %ctx = test_init();
+
+require Hydra::Schema;
+require Hydra::Model::DB;
+require Hydra::Helper::Nix;
+
+use Test2::V0;
+require Catalyst::Test;
+use HTTP::Request::Common;
+Catalyst::Test->import('Hydra');
+
+my $db = Hydra::Model::DB->new;
+hydra_setup($db);
+
+my $project = $db->resultset('Projects')->create({name => "tests", displayname => "", owner => "root"});
+
+my $jobset = createBaseJobset("basic", "basic.nix", $ctx{jobsdir});
+
+ok(evalSucceeds($jobset),               "Evaluating jobs/basic.nix should exit with return code 0");
+is(nrQueuedBuildsForJobset($jobset), 3, "Evaluating jobs/basic.nix should result in 3 builds");
+my ($build, @builds) = queuedBuildsForJobset($jobset);
+
+ok(runBuild($build), "Build '".$build->job."' from jobs/basic.nix should exit with code 0");
+
+subtest "/build/ID/evals" => sub {
+    my $evals = request(GET '/build/' . $build->id . '/evals');
+    ok($evals->is_success, "The page listing evaluations this build is part of returns 200.");
+};
+
+done_testing;
diff --git a/t/Controller/Jobset/evals.t b/t/Controller/Jobset/evals.t
new file mode 100644
index 00000000..ba53124a
--- /dev/null
+++ b/t/Controller/Jobset/evals.t
@@ -0,0 +1,34 @@
+use strict;
+use Setup;
+use Data::Dumper;
+my %ctx = test_init();
+
+require Hydra::Schema;
+require Hydra::Model::DB;
+require Hydra::Helper::Nix;
+
+use Test2::V0;
+require Catalyst::Test;
+use HTTP::Request::Common;
+Catalyst::Test->import('Hydra');
+
+my $db = Hydra::Model::DB->new;
+hydra_setup($db);
+
+my $project = $db->resultset('Projects')->create({name => "tests", displayname => "", owner => "root"});
+
+my $jobset = createBaseJobset("basic", "basic.nix", $ctx{jobsdir});
+
+ok(evalSucceeds($jobset), "Evaluating jobs/basic.nix should exit with return code 0");
+
+subtest "/jobset/PROJECT/JOBSET" => sub {
+    my $jobset = request(GET '/jobset/' . $project->name . '/' . $jobset->name);
+    ok($jobset->is_success, "The page showing the jobset returns 200.");
+};
+
+subtest "/jobset/PROJECT/JOBSET/evals" => sub {
+    my $jobsetevals = request(GET '/jobset/' . $project->name . '/' . $jobset->name . '/evals');
+    ok($jobsetevals->is_success, "The page showing the jobset evals returns 200.");
+};
+
+done_testing;
diff --git a/t/Controller/Root/evals.t b/t/Controller/Root/evals.t
new file mode 100644
index 00000000..895ef424
--- /dev/null
+++ b/t/Controller/Root/evals.t
@@ -0,0 +1,29 @@
+use strict;
+use Setup;
+use Data::Dumper;
+my %ctx = test_init();
+
+require Hydra::Schema;
+require Hydra::Model::DB;
+require Hydra::Helper::Nix;
+
+use Test2::V0;
+require Catalyst::Test;
+use HTTP::Request::Common;
+Catalyst::Test->import('Hydra');
+
+my $db = Hydra::Model::DB->new;
+hydra_setup($db);
+
+my $project = $db->resultset('Projects')->create({name => "tests", displayname => "", owner => "root"});
+
+my $jobset = createBaseJobset("basic", "basic.nix", $ctx{jobsdir});
+
+ok(evalSucceeds($jobset), "Evaluating jobs/basic.nix should exit with return code 0");
+
+subtest "/evals" => sub {
+    my $global = request(GET '/evals');
+    ok($global->is_success, "The page showing the all evals returns 200.");
+};
+
+done_testing;

From bf5c76feb6103444c2dd2ceb20b60129a1542f12 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Wed, 16 Jun 2021 11:22:25 -0400
Subject: [PATCH 2/5] getEvals: order by the eval table's ID

I broke this when I added `me.` in f1e75c8bffec3f5f86122ee73e3c0f19e26ddb27

I added me. to disambiguate `id`, but:

* eval.id works on the per-build page
* me.id works on the other pages
* Just id works everywhere if I drop:

    , prefetch => { evaluationerror => [ ] },

  but this causes a query per row to collect the evaluationerror
  records later, this becomes significantly slow on non-trivial
  datasets.

Using evals->current_source_alias will use the correct alias
whether it is me or eval or something else.
---
 src/lib/Hydra/Helper/Nix.pm | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/lib/Hydra/Helper/Nix.pm b/src/lib/Hydra/Helper/Nix.pm
index e198a56c..01f65ad2 100644
--- a/src/lib/Hydra/Helper/Nix.pm
+++ b/src/lib/Hydra/Helper/Nix.pm
@@ -226,10 +226,12 @@ sub getEvalInfo {
 sub getEvals {
     my ($self, $c, $evals, $offset, $rows) = @_;
 
+    my $me = $evals->current_source_alias;
+
     my @evals = $evals->search(
         { hasnewbuilds => 1 },
-        { order_by => "me.id DESC", rows => $rows, offset => $offset
-        , prefetch => { evaluationerror => [  ] } });
+        { order_by => "$me.id DESC", rows => $rows, offset => $offset
+        , prefetch => { evaluationerror => [ ] } });
     my @res = ();
     my $cache = {};
 

From cb8929b7ed3d0c868bbe3c51758ca535302a9ddb Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Wed, 16 Jun 2021 11:48:49 -0400
Subject: [PATCH 3/5] Tighten up 'should exit with return code'

---
 t/Controller/Build/evals.t            | 2 +-
 t/Controller/Jobset/channel.t         | 2 +-
 t/build-products.t                    | 2 +-
 t/evaluate-basic.t                    | 2 +-
 t/evaluate-dependent-jobsets.t        | 4 ++--
 t/plugins/runcommand.t                | 2 +-
 t/queue-runner/default-machine-file.t | 2 +-
 t/queue-runner/notifications.t        | 4 ++--
 8 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/t/Controller/Build/evals.t b/t/Controller/Build/evals.t
index f44d5733..b12e00c6 100644
--- a/t/Controller/Build/evals.t
+++ b/t/Controller/Build/evals.t
@@ -23,7 +23,7 @@ ok(evalSucceeds($jobset),               "Evaluating jobs/basic.nix should exit w
 is(nrQueuedBuildsForJobset($jobset), 3, "Evaluating jobs/basic.nix should result in 3 builds");
 my ($build, @builds) = queuedBuildsForJobset($jobset);
 
-ok(runBuild($build), "Build '".$build->job."' from jobs/basic.nix should exit with code 0");
+ok(runBuild($build), "Build '".$build->job."' from jobs/basic.nix should exit with return code 0");
 
 subtest "/build/ID/evals" => sub {
     my $evals = request(GET '/build/' . $build->id . '/evals');
diff --git a/t/Controller/Jobset/channel.t b/t/Controller/Jobset/channel.t
index c632cf55..cb0a5952 100644
--- a/t/Controller/Jobset/channel.t
+++ b/t/Controller/Jobset/channel.t
@@ -29,7 +29,7 @@ ok(evalSucceeds($jobset));
 is(nrQueuedBuildsForJobset($jobset), 4);
 
 for my $build (queuedBuildsForJobset($jobset)) {
-    ok(runBuild($build), "Build '".$build->job."' should exit with code 0");
+    ok(runBuild($build), "Build '".$build->job."' should exit with return code 0");
     my $newbuild = $db->resultset('Builds')->find($build->id);
     is($newbuild->finished, 1, "Build '".$build->job."' should be finished.");
     is($newbuild->buildstatus, 0, "Build '".$build->job."' should have buildstatus 0.");
diff --git a/t/build-products.t b/t/build-products.t
index 0f558d86..6987b9df 100644
--- a/t/build-products.t
+++ b/t/build-products.t
@@ -21,7 +21,7 @@ is(nrQueuedBuildsForJobset($jobset), 2, "Evaluating jobs/build-products.nix shou
 
 for my $build (queuedBuildsForJobset($jobset)) {
     subtest "For the build job '" . $build->job . "'" => sub {
-        ok(runBuild($build), "Build should exit with code 0");
+        ok(runBuild($build), "Build should exit with return code 0");
         my $newbuild = $db->resultset('Builds')->find($build->id);
 
         is($newbuild->finished, 1, "Build should have finished");
diff --git a/t/evaluate-basic.t b/t/evaluate-basic.t
index 85d5547b..c12709b2 100644
--- a/t/evaluate-basic.t
+++ b/t/evaluate-basic.t
@@ -21,7 +21,7 @@ ok(evalSucceeds($jobset),               "Evaluating jobs/basic.nix should exit w
 is(nrQueuedBuildsForJobset($jobset), 3, "Evaluating jobs/basic.nix should result in 3 builds");
 
 for my $build (queuedBuildsForJobset($jobset)) {
-    ok(runBuild($build), "Build '".$build->job."' from jobs/basic.nix should exit with code 0");
+    ok(runBuild($build), "Build '".$build->job."' from jobs/basic.nix should exit with return code 0");
     my $newbuild = $db->resultset('Builds')->find($build->id);
     is($newbuild->finished, 1, "Build '".$build->job."' from jobs/basic.nix should be finished.");
     my $expected = $build->job eq "fails" ? 1 : $build->job =~ /with_failed/ ? 6 : 0;
diff --git a/t/evaluate-dependent-jobsets.t b/t/evaluate-dependent-jobsets.t
index 0bf3a2f0..c5a4085d 100644
--- a/t/evaluate-dependent-jobsets.t
+++ b/t/evaluate-dependent-jobsets.t
@@ -21,7 +21,7 @@ subtest "For the 'build1' job" => sub {
     my ($build) = queuedBuildsForJobset($jobset);
     is($build->job, "build1", "Verify the only job we got is for 'build1'");
 
-    ok(runBuild($build), "Build should exit with code 0");
+    ok(runBuild($build), "Build should exit with return code 0");
     my $newbuild = $db->resultset('Builds')->find($build->id);
     is($newbuild->finished, 1, "Build should be finished.");
     is($newbuild->buildstatus, 0, "Build should have buildstatus 0.");
@@ -33,7 +33,7 @@ subtest "For the 'build2' job" => sub {
     my ($build) = queuedBuildsForJobset($jobset);
     is($build->job, "build2", "Verify the only job we got is for 'build2'");
 
-    ok(runBuild($build), "Build should exit with code 0");
+    ok(runBuild($build), "Build should exit with return code 0");
     my $newbuild = $db->resultset('Builds')->find($build->id);
     is($newbuild->finished, 1, "Build should be finished.");
     is($newbuild->buildstatus, 0, "Build should have buildstatus 0.");
diff --git a/t/plugins/runcommand.t b/t/plugins/runcommand.t
index 7f5f4cb1..8a512b32 100644
--- a/t/plugins/runcommand.t
+++ b/t/plugins/runcommand.t
@@ -30,7 +30,7 @@ is(nrQueuedBuildsForJobset($jobset), 1, "Evaluating jobs/runcommand.nix should r
 (my $build) = queuedBuildsForJobset($jobset);
 
 is($build->job, "metrics", "The only job should be metrics");
-ok(runBuild($build), "Build should exit with code 0");
+ok(runBuild($build), "Build should exit with return code 0");
 my $newbuild = $db->resultset('Builds')->find($build->id);
 is($newbuild->finished, 1, "Build should be finished.");
 is($newbuild->buildstatus, 0, "Build should have buildstatus 0.");
diff --git a/t/queue-runner/default-machine-file.t b/t/queue-runner/default-machine-file.t
index d4d3f3df..d57f1816 100644
--- a/t/queue-runner/default-machine-file.t
+++ b/t/queue-runner/default-machine-file.t
@@ -24,7 +24,7 @@ ok(evalSucceeds($jobset),               "Evaluating jobs/default-machine-file.ni
 is(nrQueuedBuildsForJobset($jobset), 1, "Evaluating jobs/default-machine-file.nix should result in 1 build");
 
 for my $build (queuedBuildsForJobset($jobset)) {
-    ok(runBuild($build), "Build '".$build->job."' from jobs/default-machine-file.nix should exit with code 0");
+    ok(runBuild($build), "Build '".$build->job."' from jobs/default-machine-file.nix should exit with return code 0");
     my $newbuild = $db->resultset('Builds')->find($build->id);
     is($newbuild->finished, 1, "Build '".$build->job."' from jobs/default-machine-file.nix should be finished.");
     my $expected = $build->job eq "fails" ? 1 : $build->job =~ /with_failed/ ? 6 : 0;
diff --git a/t/queue-runner/notifications.t b/t/queue-runner/notifications.t
index 3c43bceb..f9d36361 100644
--- a/t/queue-runner/notifications.t
+++ b/t/queue-runner/notifications.t
@@ -88,7 +88,7 @@ my @builds = queuedBuildsForJobset($jobset);
 
 subtest "Build: substitutable, canbesubstituted" => sub {
     my ($build) = grep { $_->nixname eq "can-be-substituted" } @builds;
-    ok(runBuild($build), "Build should exit with code 0");
+    ok(runBuild($build), "Build should exit with return code 0");
 
     my $newbuild = $db->resultset('Builds')->find($build->id);
     is($newbuild->finished, 1, "Build should be finished.");
@@ -107,7 +107,7 @@ subtest "Build: substitutable, canbesubstituted" => sub {
 
 subtest "Build: not substitutable, unsubstitutable" => sub {
     my ($build) = grep { $_->nixname eq "unsubstitutable" } @builds;
-    ok(runBuild($build), "Build should exit with code 0");
+    ok(runBuild($build), "Build should exit with return code 0");
 
     my $newbuild = $db->resultset('Builds')->find($build->id);
     is($newbuild->finished, 1, "Build should be finished.");

From 5d95abf54026ad1901da30fde52d614d91b9800f Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Wed, 16 Jun 2021 11:53:04 -0400
Subject: [PATCH 4/5] getBuilds: clarify the names of evals vs. the query
 builder

---
 src/lib/Hydra/Helper/Nix.pm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/lib/Hydra/Helper/Nix.pm b/src/lib/Hydra/Helper/Nix.pm
index 01f65ad2..29df1c42 100644
--- a/src/lib/Hydra/Helper/Nix.pm
+++ b/src/lib/Hydra/Helper/Nix.pm
@@ -224,11 +224,11 @@ sub getEvalInfo {
 
 
 sub getEvals {
-    my ($self, $c, $evals, $offset, $rows) = @_;
+    my ($self, $c, $evals_query_builder, $offset, $rows) = @_;
 
-    my $me = $evals->current_source_alias;
+    my $me = $evals_query_builder->current_source_alias;
 
-    my @evals = $evals->search(
+    my @evals = $evals_query_builder->search(
         { hasnewbuilds => 1 },
         { order_by => "$me.id DESC", rows => $rows, offset => $offset
         , prefetch => { evaluationerror => [ ] } });

From 09ad52ab60bc0ee3a13bcd354a525e65828d886b Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Wed, 16 Jun 2021 12:42:25 -0400
Subject: [PATCH 5/5] Document getEvals a bit

Also drop the $self parameter.

Co-authored-by: gustavderdrache
---
 src/lib/Hydra/Controller/Build.pm  |  2 +-
 src/lib/Hydra/Controller/Jobset.pm |  4 ++--
 src/lib/Hydra/Controller/Root.pm   |  2 +-
 src/lib/Hydra/Helper/Nix.pm        | 33 +++++++++++++++++++++++++++---
 4 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/src/lib/Hydra/Controller/Build.pm b/src/lib/Hydra/Controller/Build.pm
index c769edef..765dcbd7 100644
--- a/src/lib/Hydra/Controller/Build.pm
+++ b/src/lib/Hydra/Controller/Build.pm
@@ -558,7 +558,7 @@ sub evals : Chained('buildChain') PathPart('evals') Args(0) {
     $c->stash->{page} = $page;
     $c->stash->{resultsPerPage} = $resultsPerPage;
     $c->stash->{total} = $evals->search({hasnewbuilds => 1})->count;
-    $c->stash->{evals} = getEvals($self, $c, $evals, ($page - 1) * $resultsPerPage, $resultsPerPage)
+    $c->stash->{evals} = getEvals($c, $evals, ($page - 1) * $resultsPerPage, $resultsPerPage)
 }
 
 
diff --git a/src/lib/Hydra/Controller/Jobset.pm b/src/lib/Hydra/Controller/Jobset.pm
index ae0bb018..a5b6cfac 100644
--- a/src/lib/Hydra/Controller/Jobset.pm
+++ b/src/lib/Hydra/Controller/Jobset.pm
@@ -41,7 +41,7 @@ sub jobset_GET {
 
     $c->stash->{template} = 'jobset.tt';
 
-    $c->stash->{evals} = getEvals($self, $c, scalar $c->stash->{jobset}->jobsetevals, 0, 10);
+    $c->stash->{evals} = getEvals($c, scalar $c->stash->{jobset}->jobsetevals, 0, 10);
 
     $c->stash->{latestEval} = $c->stash->{jobset}->jobsetevals->search({ hasnewbuilds => 1 }, { rows => 1, order_by => ["id desc"] })->single;
 
@@ -337,7 +337,7 @@ sub evals_GET {
     $c->stash->{resultsPerPage} = $resultsPerPage;
     $c->stash->{total} = $evals->search({hasnewbuilds => 1})->count;
     my $offset = ($page - 1) * $resultsPerPage;
-    $c->stash->{evals} = getEvals($self, $c, $evals, $offset, $resultsPerPage);
+    $c->stash->{evals} = getEvals($c, $evals, $offset, $resultsPerPage);
     my %entity = (
         evals => [ map { $_->{eval} } @{$c->stash->{evals}} ],
         first => "?page=1",
diff --git a/src/lib/Hydra/Controller/Root.pm b/src/lib/Hydra/Controller/Root.pm
index 62e793e1..df178df4 100644
--- a/src/lib/Hydra/Controller/Root.pm
+++ b/src/lib/Hydra/Controller/Root.pm
@@ -398,7 +398,7 @@ sub evals :Local Args(0) {
     $c->stash->{page} = $page;
     $c->stash->{resultsPerPage} = $resultsPerPage;
     $c->stash->{total} = $evals->search({hasnewbuilds => 1})->count;
-    $c->stash->{evals} = getEvals($self, $c, $evals, ($page - 1) * $resultsPerPage, $resultsPerPage);
+    $c->stash->{evals} = getEvals($c, $evals, ($page - 1) * $resultsPerPage, $resultsPerPage);
 
     $self->status_ok($c, entity => $c->stash->{evals});
 }
diff --git a/src/lib/Hydra/Helper/Nix.pm b/src/lib/Hydra/Helper/Nix.pm
index 29df1c42..51509c73 100644
--- a/src/lib/Hydra/Helper/Nix.pm
+++ b/src/lib/Hydra/Helper/Nix.pm
@@ -223,12 +223,39 @@ sub getEvalInfo {
 }
 
 
+=head2 getEvals
+
+This method returns a list of evaluations with details about what changed,
+intended to be used with `eval.tt`.
+
+Arguments:
+
+=over 4
+
+=item C<$c>
+L<Hydra> - the entire application.
+
+=item C<$evals_result_set>
+
+A L<DBIx::Class::ResultSet> for the result class of L<Hydra::Model::DB::JobsetEvals>
+
+=item C<$offset>
+
+Integer offset when selecting evaluations
+
+=item C<$rows>
+
+Integer rows to fetch
+
+=back
+
+=cut
 sub getEvals {
-    my ($self, $c, $evals_query_builder, $offset, $rows) = @_;
+    my ($c, $evals_result_set, $offset, $rows) = @_;
 
-    my $me = $evals_query_builder->current_source_alias;
+    my $me = $evals_result_set->current_source_alias;
 
-    my @evals = $evals_query_builder->search(
+    my @evals = $evals_result_set->search(
         { hasnewbuilds => 1 },
         { order_by => "$me.id DESC", rows => $rows, offset => $offset
         , prefetch => { evaluationerror => [ ] } });