From 25f6bae84776538aef68235dbd1ba537fe6efce0 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Sat, 19 Mar 2022 14:34:43 -0400
Subject: [PATCH 1/4] HydraTestContext: make it easy to create a jobset without
 evaluating

---
 t/lib/HydraTestContext.pm | 61 ++++++++++++++++++++++++++++-----------
 1 file changed, 44 insertions(+), 17 deletions(-)

diff --git a/t/lib/HydraTestContext.pm b/t/lib/HydraTestContext.pm
index ade12280..ce05b581 100644
--- a/t/lib/HydraTestContext.pm
+++ b/t/lib/HydraTestContext.pm
@@ -145,10 +145,47 @@ sub nix_state_dir {
 sub makeAndEvaluateJobset {
     my ($self, %opts) = @_;
 
-    my $expression = $opts{'expression'} || die "Mandatory 'expression' option not passed to makeAndEValuateJobset.";
-    my $should_build = $opts{'build'} // 0;
+    my $expression = $opts{'expression'} || die "Mandatory 'expression' option not passed to makeAndEvaluateJobset.";
     my $jobsdir = $opts{'jobsdir'} // $self->jobsdir;
+    my $should_build = $opts{'build'} // 0;
 
+    my $jobsetCtx = $self->makeJobset(
+        expression => $expression,
+        jobsdir => $jobsdir,
+    );
+    my $jobset = $jobsetCtx->{"jobset"};
+
+    evalSucceeds($jobset) or die "Evaluating jobs/$expression should exit with return code 0";
+
+    my $builds = {};
+
+    for my $build ($jobset->builds) {
+        if ($should_build) {
+            runBuild($build) or die "Build '".$build->job."' from jobs/$expression should exit with return code 0";
+            $build->discard_changes();
+        }
+
+        $builds->{$build->job} = $build;
+    }
+
+    return $builds;
+}
+
+# Create a jobset.
+#
+# In return, you get a hash of the user, project, and jobset records.
+#
+# This always uses an `expression` from the `jobsdir` directory.
+#
+# Hash Parameters:
+#
+#  * expression: The file in the jobsdir directory to evaluate
+#  * jobsdir: An alternative jobsdir to source the expression from
+sub makeJobset {
+    my ($self, %opts) = @_;
+
+    my $expression = $opts{'expression'} || die "Mandatory 'expression' option not passed to makeJobset.";
+    my $jobsdir = $opts{'jobsdir'} // $self->jobsdir;
 
     # Create a new user for this test
     my $user = $self->db()->resultset('Users')->create({
@@ -174,23 +211,13 @@ sub makeAndEvaluateJobset {
     my $jobsetinput = $jobset->jobsetinputs->create({name => "jobs", type => "path"});
     $jobsetinput->jobsetinputalts->create({altnr => 0, value => $jobsdir});
 
-    evalSucceeds($jobset) or die "Evaluating jobs/$expression should exit with return code 0";
-
-    my $builds = {};
-
-    for my $build ($jobset->builds) {
-        if ($should_build) {
-            runBuild($build) or die "Build '".$build->job."' from jobs/$expression should exit with return code 0";
-            $build->discard_changes();
-        }
-
-        $builds->{$build->job} = $build;
-    }
-
-    return $builds;
+    return {
+        user => $user,
+        project => $project,
+        jobset => $jobset,
+    };
 }
 
-
 sub DESTROY
 {
     my ($self) = @_;

From 0c51de6334196683a039689af934344facadf8e2 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Sat, 19 Mar 2022 14:35:30 -0400
Subject: [PATCH 2/4] hydra-evaluate-jobset: assert it logs errored
 constituents properly

---
 t/evaluator/evaluate-constituents-broken.t | 32 ++++++++++++++++++++++
 t/jobs/constituents-broken.nix             | 19 +++++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 t/evaluator/evaluate-constituents-broken.t
 create mode 100644 t/jobs/constituents-broken.nix

diff --git a/t/evaluator/evaluate-constituents-broken.t b/t/evaluator/evaluate-constituents-broken.t
new file mode 100644
index 00000000..ed25d192
--- /dev/null
+++ b/t/evaluator/evaluate-constituents-broken.t
@@ -0,0 +1,32 @@
+use strict;
+use warnings;
+use Setup;
+use Test2::V0;
+use Hydra::Helper::Exec;
+
+my $ctx = test_context();
+
+my $jobsetCtx = $ctx->makeJobset(
+    expression => 'constituents-broken.nix',
+);
+my $jobset = $jobsetCtx->{"jobset"};
+
+my ($res, $stdout, $stderr) = captureStdoutStderr(60,
+    ("hydra-eval-jobset", $jobsetCtx->{"project"}->name, $jobset->name)
+);
+isnt($res, 0, "hydra-eval-jobset exits non-zero");
+ok(utf8::decode($stderr), "Stderr output is UTF8-clean");
+like(
+    $stderr,
+    qr/aggregate job ‘mixed_aggregate’ failed with the error: constituentA: does not exist/,
+    "The stderr record includes a relevant error message"
+);
+
+$jobset->discard_changes;  # refresh from DB
+like(
+    $jobset->errormsg,
+    qr/aggregate job ‘mixed_aggregate’ failed with the error: constituentA: does not exist/,
+    "The jobset records a relevant error message"
+);
+
+done_testing;
diff --git a/t/jobs/constituents-broken.nix b/t/jobs/constituents-broken.nix
new file mode 100644
index 00000000..0445a990
--- /dev/null
+++ b/t/jobs/constituents-broken.nix
@@ -0,0 +1,19 @@
+with import ./config.nix;
+rec {
+  constituentA = null;
+
+  constituentB = mkDerivation {
+    name = "empty-dir-B";
+    builder = ./empty-dir-builder.sh;
+  };
+
+  mixed_aggregate = mkDerivation {
+    name = "mixed_aggregate";
+    _hydraAggregate = true;
+    constituents = [
+      "constituentA"
+      constituentB
+    ];
+    builder = ./empty-dir-builder.sh;
+  };
+}

From 074a2f96bf381b60c75c156db67836031f069b80 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Sat, 19 Mar 2022 14:37:12 -0400
Subject: [PATCH 3/4] hydra-eval-jobset: emit a useful error if constituents
 errored

---
 src/script/hydra-eval-jobset | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/script/hydra-eval-jobset b/src/script/hydra-eval-jobset
index 108c59c8..99277fd4 100755
--- a/src/script/hydra-eval-jobset
+++ b/src/script/hydra-eval-jobset
@@ -799,7 +799,13 @@ sub checkJobsetWrapped {
 
             foreach my $job (values %{$jobs}) {
                 next unless $job->{constituents};
-                my $x = $drvPathToId{$job->{drvPath}} or die;
+
+                if (defined $job->{error}) {
+                    die "aggregate job ‘$job->{jobName}’ failed with the error: $job->{error}";
+                }
+
+                my $x = $drvPathToId{$job->{drvPath}} or
+                    die "aggregate job ‘$job->{jobName}’ has no corresponding build record.\n";
                 foreach my $drvPath (@{$job->{constituents}}) {
                     my $constituent = $drvPathToId{$drvPath};
                     if (defined $constituent) {

From a582e4c485977d62c02b65e94d25eb5d2b283037 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Sat, 19 Mar 2022 14:46:53 -0400
Subject: [PATCH 4/4] HydraTestContext: add \n's to various dies

---
 src/script/hydra-eval-jobset |  2 +-
 t/lib/HydraTestContext.pm    | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/script/hydra-eval-jobset b/src/script/hydra-eval-jobset
index 99277fd4..8bafe07c 100755
--- a/src/script/hydra-eval-jobset
+++ b/src/script/hydra-eval-jobset
@@ -801,7 +801,7 @@ sub checkJobsetWrapped {
                 next unless $job->{constituents};
 
                 if (defined $job->{error}) {
-                    die "aggregate job ‘$job->{jobName}’ failed with the error: $job->{error}";
+                    die "aggregate job ‘$job->{jobName}’ failed with the error: $job->{error}\n";
                 }
 
                 my $x = $drvPathToId{$job->{drvPath}} or
diff --git a/t/lib/HydraTestContext.pm b/t/lib/HydraTestContext.pm
index ce05b581..237fcbe4 100644
--- a/t/lib/HydraTestContext.pm
+++ b/t/lib/HydraTestContext.pm
@@ -145,7 +145,7 @@ sub nix_state_dir {
 sub makeAndEvaluateJobset {
     my ($self, %opts) = @_;
 
-    my $expression = $opts{'expression'} || die "Mandatory 'expression' option not passed to makeAndEvaluateJobset.";
+    my $expression = $opts{'expression'} || die "Mandatory 'expression' option not passed to makeAndEvaluateJobset.\n";
     my $jobsdir = $opts{'jobsdir'} // $self->jobsdir;
     my $should_build = $opts{'build'} // 0;
 
@@ -155,13 +155,13 @@ sub makeAndEvaluateJobset {
     );
     my $jobset = $jobsetCtx->{"jobset"};
 
-    evalSucceeds($jobset) or die "Evaluating jobs/$expression should exit with return code 0";
+    evalSucceeds($jobset) or die "Evaluating jobs/$expression should exit with return code 0.\n";
 
     my $builds = {};
 
     for my $build ($jobset->builds) {
         if ($should_build) {
-            runBuild($build) or die "Build '".$build->job."' from jobs/$expression should exit with return code 0";
+            runBuild($build) or die "Build '".$build->job."' from jobs/$expression should exit with return code 0.\n";
             $build->discard_changes();
         }
 
@@ -184,7 +184,7 @@ sub makeAndEvaluateJobset {
 sub makeJobset {
     my ($self, %opts) = @_;
 
-    my $expression = $opts{'expression'} || die "Mandatory 'expression' option not passed to makeJobset.";
+    my $expression = $opts{'expression'} || die "Mandatory 'expression' option not passed to makeJobset.\n";
     my $jobsdir = $opts{'jobsdir'} // $self->jobsdir;
 
     # Create a new user for this test
@@ -227,7 +227,7 @@ sub DESTROY
 
 sub write_file {
     my ($path, $text) = @_;
-    open(my $fh, '>', $path) or die "Could not open file '$path' $!";
+    open(my $fh, '>', $path) or die "Could not open file '$path' $!\n.";
     print $fh $text || "";
     close $fh;
 }