Merge pull request #998 from DeterminateSystems/notify-events

hydra-notify: abstract event handling, make testable

src/lib/Hydra/Event.pm

@@ -0,0 +1,41 @@
+package Hydra::Event;
+
+use strict;
+use Hydra::Event::BuildFinished;
+use Hydra::Event::BuildStarted;
+use Hydra::Event::StepFinished;
+
+my %channels_to_events = (
+  build_started => \&Hydra::Event::BuildStarted::parse,
+  step_finished => \&Hydra::Event::StepFinished::parse,
+  build_finished => \&Hydra::Event::BuildFinished::parse,
+);
+
+
+sub parse_payload :prototype($$) {
+    my ($channel_name, $payload) = @_;
+    my @payload = split /\t/, $payload;
+
+    my $parser = %channels_to_events{$channel_name};
+    unless (defined $parser) {
+      die "Invalid channel name: '$channel_name'";
+    }
+
+    return $parser->(@payload);
+}
+
+
+sub new_event {
+    my ($self, $channel_name, $payload) = @_;
+
+    return bless {
+        "channel_name" => $channel_name,
+        "payload" => $payload,
+        "event" => parse_payload($channel_name, $payload),
+    }, $self;
+}
+
+sub execute {
+    my ($self, $db, $plugin) = @_;
+    return $self->{"event"}->execute($db, $plugin);
+}

src/lib/Hydra/Event/BuildFinished.pm

@@ -0,0 +1,65 @@
+package Hydra::Event::BuildFinished;
+
+use strict;
+use warnings;
+
+sub parse :prototype(@) {
+    if (@_ == 0) {
+        die "build_finished: payload takes at least one argument, but ", scalar(@_), " were given";
+    }
+
+    my @failures = grep(!/^\d+$/, @_);
+    if (@failures > 0) {
+        die "build_finished: payload arguments should be integers, but we received the following non-integers:", @failures;
+    }
+
+    my ($build_id, @dependents) = map int, @_;
+    return Hydra::Event::BuildFinished->new($build_id, \@dependents);
+}
+
+sub new {
+    my ($self, $build_id, $dependent_ids) = @_;
+    return bless {
+        "build_id" => $build_id,
+        "dependent_ids" => $dependent_ids,
+        "build" => undef,
+        "dependents" => [],
+    }, $self;
+}
+
+sub load {
+    my ($self, $db) = @_;
+
+    if (!defined($self->{"build"})) {
+        $self->{"build"} = $db->resultset('Builds')->find($self->{"build_id"})
+            or die "build $self->{'build_id'} does not exist\n";
+
+        foreach my $id (@{$self->{"dependent_ids"}}) {
+            my $dep = $db->resultset('Builds')->find($id)
+                or die "dependent build $id does not exist\n";
+            push @{$self->{"dependents"}}, $dep;
+        }
+    }
+}
+
+sub execute {
+    my ($self, $db, $plugin) = @_;
+
+    $self->load($db);
+
+    $plugin->buildFinished($self->{"build"}, $self->{"dependents"});
+
+    # Mark the build and all dependents as having their notifications "finished".
+    #
+    # Otherwise, the dependent builds will remain with notificationpendingsince set
+    # until hydra-notify is started, as buildFinished is never emitted for them.
+    foreach my $b ($self->{"build"}, @{$self->{"dependents"}}) {
+        if ($b->finished && defined($b->notificationpendingsince)) {
+            $b->update({ notificationpendingsince => undef })
+        }
+    }
+
+    return 1;
+}
+
+1;

src/lib/Hydra/Event/BuildStarted.pm

@@ -0,0 +1,47 @@
+package Hydra::Event::BuildStarted;
+
+use strict;
+use warnings;
+
+sub parse :prototype(@) {
+    unless (@_ == 1) {
+        die "build_started: payload takes only one argument, but ", scalar(@_), " were given";
+    }
+
+    my ($build_id) = @_;
+
+    unless ($build_id =~ /^\d+$/) {
+        die "build_started: payload argument should be an integer, but '", $build_id, "' was given"
+    }
+
+    return Hydra::Event::BuildStarted->new(int($build_id));
+}
+
+sub new {
+    my ($self, $id) = @_;
+    return bless {
+        "build_id" => $id,
+        "build" => undef
+    }, $self;
+}
+
+sub load {
+    my ($self, $db) = @_;
+
+    if (!defined($self->{"build"})) {
+        $self->{"build"} = $db->resultset('Builds')->find($self->{"build_id"})
+            or die "build $self->{'build_id'} does not exist\n";
+    }
+}
+
+sub execute {
+    my ($self, $db, $plugin) = @_;
+
+    $self->load($db);
+
+    $plugin->buildStarted($self->{"build"});
+
+    return 1;
+}
+
+1;

src/lib/Hydra/Event/StepFinished.pm

@@ -0,0 +1,59 @@
+package Hydra::Event::StepFinished;
+
+use strict;
+use warnings;
+
+
+sub parse :prototype(@) {
+    unless (@_ == 3) {
+        die "step_finished: payload takes exactly three arguments, but ", scalar(@_), " were given";
+    }
+
+    my ($build_id, $step_number, $log_path) = @_;
+
+    unless ($build_id =~ /^\d+$/) {
+        die "step_finished: payload argument build_id should be an integer, but '", $build_id, "' was given"
+    }
+    unless ($step_number =~ /^\d+$/) {
+        die "step_finished: payload argument step_number should be an integer, but '", $step_number, "' was given"
+    }
+
+    return Hydra::Event::StepFinished->new(int($build_id), int($step_number), $log_path);
+}
+
+sub new :prototype($$$) {
+    my ($self, $build_id, $step_number, $log_path) = @_;
+
+    $log_path = undef if $log_path eq "-";
+
+    return bless {
+        "build_id" => $build_id,
+        "step_number" => $step_number,
+        "log_path" => $log_path,
+        "step" => undef,
+    }, $self;
+}
+
+sub load {
+    my ($self, $db) = @_;
+
+    if (!defined($self->{"step"})) {
+        my $build = $db->resultset('Builds')->find($self->{"build_id"})
+            or die "build $self->{'build_id'} does not exist\n";
+
+        $self->{"step"} = $build->buildsteps->find({stepnr => $self->{"step_number"}})
+            or die "step $self->{'step_number'} does not exist\n";
+    }
+}
+
+sub execute {
+    my ($self, $db, $plugin) = @_;
+
+    $self->load($db);
+
+    $plugin->stepFinished($self->{"step"}, $self->{"log_path"});
+
+    return 1;
+}
+
+1;

src/script/hydra-notify

@@ -2,10 +2,11 @@
 
 use strict;
 use utf8;
+use Hydra::Event::BuildFinished;
+use Hydra::Helper::AddBuilds;
+use Hydra::Helper::Nix;
 use Hydra::Plugin;
 use Hydra::PostgresListener;
-use Hydra::Helper::Nix;
-use Hydra::Helper::AddBuilds;
 use Getopt::Long;
 
 STDERR->autoflush(1);
@@ -31,70 +32,15 @@ $listener->subscribe("build_started");
 $listener->subscribe("build_finished");
 $listener->subscribe("step_finished");
 
-sub buildStarted {
-    my ($buildId) = @_;
-
-    my $build = $db->resultset('Builds')->find($buildId)
-        or die "build $buildId does not exist\n";
+sub runPluginsForEvent {
+    my ($event) = @_;
 
     foreach my $plugin (@plugins) {
         eval {
-            $plugin->buildStarted($build);
+            $event->execute($db, $plugin);
             1;
         } or do {
-            print STDERR "error with $plugin->buildStarted: $@\n";
-        }
-    }
-}
-
-sub buildFinished {
-    my ($buildId, @deps) = @_;
-
-    my $build = $db->resultset('Builds')->find($buildId)
-    or die "build $buildId does not exist\n";
-
-    my @dependents;
-    foreach my $id (@deps) {
-        my $dep = $db->resultset('Builds')->find($id)
-            or die "build $id does not exist\n";
-        push @dependents, $dep;
-    }
-
-    foreach my $plugin (@plugins) {
-        eval {
-            $plugin->buildFinished($build, [@dependents]);
-            1;
-        } or do {
-            print STDERR "error with $plugin->buildFinished: $@\n";
-        }
-    }
-
-    # We have to iterate through all dependents as well, and if they are finished
-    # to mark their notificationpendingsince.
-    # Otherwise, the dependent builds will remain with notificationpendingsince set
-    # until hydra-notify is started, as buildFinished is never emitted for them.
-    foreach my $b ($build, @dependents) {
-        $b->update({ notificationpendingsince => undef }) if $b->finished;
-    }
-}
-
-sub stepFinished {
-    my ($buildId, $stepNr, $logPath) = @_;
-
-    my $build = $db->resultset('Builds')->find($buildId)
-        or die "build $buildId does not exist\n";
-
-    my $step = $build->buildsteps->find({stepnr => $stepNr})
-        or die "step $stepNr does not exist\n";
-
-    $logPath = undef if $logPath eq "-";
-
-    foreach my $plugin (@plugins) {
-        eval {
-            $plugin->stepFinished($step, $logPath);
-            1;
-        } or do {
-            print STDERR "error with $plugin->stepFinished: $@\n";
+            print STDERR "error running $event->{'channel_name'} hooks: $@\n";
         }
     }
 }
@@ -103,9 +49,11 @@ sub stepFinished {
 for my $build ($db->resultset('Builds')->search(
                    { notificationpendingsince => { '!=', undef } }))
 {
-    my $buildId = $build->id;
-    print STDERR "sending notifications for build ${\$buildId}...\n";
-    buildFinished($build->id);
+    print STDERR "sending notifications for build $build->id...\n";
+
+
+    my $event = Hydra::Event::BuildFinished->new($build->id);
+    runPluginsForEvent($event);
 }
 
 
@@ -117,19 +65,11 @@ while (!$queued_only) {
         my $channelName = $message->{"channel"};
         my $pid = $message->{"pid"};
         my $payload = $message->{"payload"};
-        #print STDERR "got '$channelName' from $pid: $payload\n";
-
-        my @payload = split /\t/, $payload;
 
         eval {
-            if ($channelName eq "build_started") {
-                buildStarted(int($payload[0]));
-            } elsif ($channelName eq "build_finished") {
-                my $buildId = int($payload[0]);
-                buildFinished($buildId, @payload[1..$#payload]);
-            } elsif ($channelName eq "step_finished") {
-                stepFinished(int($payload[0]), int($payload[1]));
-            }
+            my $event = Hydra::Event::new_event($channelName, $message->{"payload"});
+            runPluginsForEvent($event);
+
             1;
         } or do {
             print STDERR "error processing message '$payload' on channel '$channelName': $@\n";

t/Event.t

@@ -0,0 +1,22 @@
+use strict;
+use Hydra::Event;
+
+use Test2::V0;
+use Test2::Tools::Exception;
+
+subtest "Event: new event" => sub {
+    my $event = Hydra::Event->new_event("build_started", "19");
+    is($event->{'payload'}, "19");
+    is($event->{'channel_name'}, "build_started");
+    is($event->{'event'}->{'build_id'}, 19);
+};
+
+subtest "Payload type: bogus" => sub {
+    like(
+        dies { Hydra::Event::parse_payload("bogus", "") },
+        qr/Invalid channel name/,
+        "bogus channel"
+    );
+};
+
+done_testing;

t/Event/BuildFinished.t

@@ -0,0 +1,92 @@
+use strict;
+use Setup;
+
+my %ctx = test_init();
+
+require Hydra::Schema;
+require Hydra::Model::DB;
+use Hydra::Event;
+use Hydra::Event::BuildFinished;
+
+use Test2::V0;
+use Test2::Tools::Exception;
+use Test2::Tools::Mock qw(mock_obj);
+
+my $db = Hydra::Model::DB->new;
+hydra_setup($db);
+
+subtest "Parsing" => sub {
+    like(
+        dies { Hydra::Event::parse_payload("build_finished", "") },
+        qr/at least one argument/,
+        "empty payload"
+    );
+    like(
+        dies { Hydra::Event::parse_payload("build_finished", "abc123") },
+        qr/should be integers/,
+        "build ID should be an integer"
+    );
+    like(
+        dies { Hydra::Event::parse_payload("build_finished", "123\tabc123") },
+        qr/should be integers/,
+        "dependent ID should be an integer"
+    );
+    is(
+        Hydra::Event::parse_payload("build_finished", "123"),
+        Hydra::Event::BuildFinished->new(123, []),
+        "no dependent builds"
+    );
+    is(
+        Hydra::Event::parse_payload("build_finished", "123\t456"),
+        Hydra::Event::BuildFinished->new(123, [456]),
+        "one dependent build"
+    );
+    is(
+        Hydra::Event::parse_payload("build_finished", "123\t456\t789\t012\t345"),
+        Hydra::Event::BuildFinished->new(123, [456, 789, 12, 345]),
+        "four dependent builds"
+    );
+};
+
+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");
+
+subtest "load" => sub {
+    my ($build, $dependent_a, $dependent_b) = $db->resultset('Builds')->search(
+      { },
+      { limit => 3 }
+    )->all;
+
+    my $event = Hydra::Event::BuildFinished->new($build->id, [$dependent_a->id, $dependent_b->id]);
+
+    $event->load($db);
+
+    is($event->{"build"}->id, $build->id, "The build record matches.");
+    is($event->{"dependents"}[0]->id, $dependent_a->id, "The dependent_a record matches.");
+    is($event->{"dependents"}[1]->id, $dependent_b->id, "The dependent_b record matches.");
+
+    # Create a fake "plugin" with a buildFinished sub, the sub sets this
+    # global passedBuild and passedDependents variables for verifying.
+    my $passedBuild;
+    my $passedDependents;
+    my $plugin = {};
+    my $mock = mock_obj $plugin => (
+        add => [
+            "buildFinished" => sub {
+                my ($self, $build, $dependents) = @_;
+                $passedBuild = $build;
+                $passedDependents = $dependents;
+            }
+        ]
+    );
+
+    $event->execute($db, $plugin);
+
+    is($passedBuild->id, $build->id, "The plugin's buildFinished hook is called with a matching build");
+    is($passedDependents->[0]->id, $dependent_a->id, "The plugin's buildFinished hook is called with a matching dependent_a");
+    is($passedDependents->[1]->id, $dependent_b->id, "The plugin's buildFinished hook is called with a matching dependent_b");
+};
+
+done_testing;

t/Event/BuildStarted.t

@@ -0,0 +1,77 @@
+use strict;
+use Setup;
+
+my %ctx = test_init();
+
+require Hydra::Schema;
+require Hydra::Model::DB;
+use Hydra::Event;
+use Hydra::Event::BuildStarted;
+
+use Test2::V0;
+use Test2::Tools::Exception;
+use Test2::Tools::Mock qw(mock_obj);
+
+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");
+
+subtest "Parsing build_started" => sub {
+    like(
+        dies { Hydra::Event::parse_payload("build_started", "") },
+        qr/one argument/,
+        "empty payload"
+    );
+    like(
+        dies { Hydra::Event::parse_payload("build_started", "abc123\tabc123") },
+        qr/only one argument/,
+        "two arguments"
+    );
+
+    like(
+        dies { Hydra::Event::parse_payload("build_started", "abc123") },
+        qr/should be an integer/,
+        "not an integer"
+    );
+    is(
+        Hydra::Event::parse_payload("build_started", "19"),
+        Hydra::Event::BuildStarted->new(19),
+        "Valid parse"
+    );
+};
+
+subtest "load" => sub {
+    my $build = $db->resultset('Builds')->search(
+      { },
+      { limit => 1 }
+    )->next;
+
+    my $event = Hydra::Event::BuildStarted->new($build->id);
+
+    $event->load($db);
+
+    is($event->{"build"}->id, $build->id, "The build record matches.");
+
+    # Create a fake "plugin" with a buildStarted sub, the sub sets this
+    # global passedBuild variable.
+    my $passedBuild;
+    my $plugin = {};
+    my $mock = mock_obj $plugin => (
+        add => [
+            "buildStarted" => sub {
+                my ($self, $build) = @_;
+                $passedBuild = $build;
+            }
+        ]
+    );
+
+    $event->execute($db, $plugin);
+
+    is($passedBuild->id, $build->id, "The plugin's buildStarted hook is called with the proper build");
+};
+
+done_testing;

t/Event/StepFinished.t

@@ -0,0 +1,101 @@
+use strict;
+use Setup;
+
+my %ctx = test_init();
+
+require Hydra::Schema;
+require Hydra::Model::DB;
+use Hydra::Event;
+use Hydra::Event::BuildStarted;
+
+use Test2::V0;
+use Test2::Tools::Exception;
+use Test2::Tools::Mock qw(mock_obj);
+
+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");
+
+for my $build (queuedBuildsForJobset($jobset)) {
+    ok(runBuild($build), "Build '".$build->job."' from jobs/basic.nix should exit with return code 0");
+}
+
+
+
+subtest "Parsing step_finished" => sub {
+    like(
+        dies { Hydra::Event::parse_payload("step_finished", "") },
+        qr/three arguments/,
+        "empty payload"
+    );
+    like(
+        dies { Hydra::Event::parse_payload("step_finished", "abc123") },
+        qr/three arguments/,
+        "one argument"
+    );
+    like(
+        dies { Hydra::Event::parse_payload("step_finished", "abc123\tabc123") },
+        qr/three arguments/,
+        "two arguments"
+    );
+    like(
+        dies { Hydra::Event::parse_payload("step_finished", "abc123\tabc123\tabc123\tabc123") },
+        qr/three arguments/,
+        "four arguments"
+    );
+    like(
+        dies { Hydra::Event::parse_payload("step_finished", "abc123\t123\t/path/to/log") },
+        qr/should be an integer/,
+        "not an integer: first position"
+    );
+    like(
+        dies { Hydra::Event::parse_payload("step_finished", "123\tabc123\t/path/to/log") },
+        qr/should be an integer/,
+        "not an integer: second argument"
+    );
+    is(
+        Hydra::Event::parse_payload("step_finished", "123\t456\t/path/to/logfile"),
+        Hydra::Event::StepFinished->new(123, 456, "/path/to/logfile")
+    );
+};
+
+subtest "load" => sub {
+
+    my $step = $db->resultset('BuildSteps')->search(
+      { },
+      { limit => 1 }
+    )->next;
+    my $build = $step->build;
+
+    my $event = Hydra::Event::StepFinished->new($build->id, $step->stepnr, "/foo/bar/baz");
+
+    $event->load($db);
+    is($event->{"step"}->get_column("build"), $build->id, "The build record matches.");
+
+    # Create a fake "plugin" with a stepFinished sub, the sub sets this
+    # "global" passedStep, passedLogPath variables.
+    my $passedStep;
+    my $passedLogPath;
+    my $plugin = {};
+    my $mock = mock_obj $plugin => (
+        add => [
+            "stepFinished" => sub {
+                my ($self, $step, $log_path) = @_;
+                $passedStep = $step;
+                $passedLogPath = $log_path;
+            }
+        ]
+    );
+
+    $event->execute($db, $plugin);
+
+    is($passedStep->get_column("build"), $build->id, "The plugin's stepFinished hook is called with a step from the expected build");
+    is($passedStep->stepnr, $step->stepnr, "The plugin's stepFinished hook is called with the proper step of the build");
+    is($passedLogPath, "/foo/bar/baz", "The plugin's stepFinished hook is called with the proper log path");
+};
+
+done_testing;