From 2de36c69ba47a3b0ebbd06abeeab02c97ffbb3b9 Mon Sep 17 00:00:00 2001
From: Oliver Gorwits <oliver@cpan.org>
Date: Sun, 12 Nov 2017 00:14:21 +0000
Subject: [PATCH] some reengineering to support proper testing

---
 lib/App/Netdisco/Backend/Job.pm               |  2 +-
 lib/App/Netdisco/Worker.pm                    | 10 ++-
 lib/App/Netdisco/Worker/Plugin.pm             |  6 +-
 lib/App/Netdisco/Worker/Plugin/Test.pm        | 27 --------
 lib/App/Netdisco/Worker/Plugin/Test/Core.pm   | 37 ----------
 xt/30-backend-workers.t                       | 67 +++++++++++++++++++
 xt/lib/App/NetdiscoX/Worker/Plugin/TestOne.pm | 17 +++++
 xt/lib/App/NetdiscoX/Worker/Plugin/TestTwo.pm | 17 +++++
 8 files changed, 110 insertions(+), 73 deletions(-)
 delete mode 100644 lib/App/Netdisco/Worker/Plugin/Test.pm
 delete mode 100644 lib/App/Netdisco/Worker/Plugin/Test/Core.pm
 create mode 100644 xt/30-backend-workers.t
 create mode 100644 xt/lib/App/NetdiscoX/Worker/Plugin/TestOne.pm
 create mode 100644 xt/lib/App/NetdiscoX/Worker/Plugin/TestTwo.pm

diff --git a/lib/App/Netdisco/Backend/Job.pm b/lib/App/Netdisco/Backend/Job.pm
index 61c9de8f..919471f7 100644
--- a/lib/App/Netdisco/Backend/Job.pm
+++ b/lib/App/Netdisco/Backend/Job.pm
@@ -78,7 +78,7 @@ sub finalise_status {
     next if $status->phase eq 'check'
       and $status->level eq Status->done()->level;
 
-    if ($status->level > $max_level) {
+    if ($status->level >= $max_level) {
       $job->status( $status->status );
       $job->log( $status->log );
       $max_level = $status->level;
diff --git a/lib/App/Netdisco/Worker.pm b/lib/App/Netdisco/Worker.pm
index c8afba16..5118d574 100644
--- a/lib/App/Netdisco/Worker.pm
+++ b/lib/App/Netdisco/Worker.pm
@@ -27,19 +27,17 @@ sub import {
   }
 
   # now vars->{workers} is populated, we set the dispatch order
-  my $store = Dancer::Factory::Hook->instance();
+  my $workers = vars->{'workers'}->{$action} || {};
   # use DDP; p vars->{'workers'};
 
   foreach my $phase (qw/check early main user/) {
-    $store->install_hooks("nd2_core_${phase}");
-
-    foreach my $namespace (sort keys %{ vars->{'workers'}->{$phase} }) {
+    foreach my $namespace (sort keys %{ $workers->{$phase} }) {
       foreach my $priority (sort {$b <=> $a}
-                            keys %{ vars->{'workers'}->{$phase}->{$namespace} }) {
+                            keys %{ $workers->{$phase}->{$namespace} }) {
 
         # D::Factory::Hook::register_hook() does not work?!
         hook "nd2_core_${phase}" => $_
-          for @{ vars->{'workers'}->{$phase}->{$namespace}->{$priority} };
+          for @{ $workers->{$phase}->{$namespace}->{$priority} };
       }
     }
   }
diff --git a/lib/App/Netdisco/Worker/Plugin.pm b/lib/App/Netdisco/Worker/Plugin.pm
index 9242b206..a537a02e 100644
--- a/lib/App/Netdisco/Worker/Plugin.pm
+++ b/lib/App/Netdisco/Worker/Plugin.pm
@@ -5,9 +5,11 @@ use Dancer::Plugin;
 
 use App::Netdisco::Util::Permission qw/check_acl_no check_acl_only/;
 use aliased 'App::Netdisco::Worker::Status';
-
 use Scope::Guard 'guard';
 
+Dancer::Factory::Hook->instance()->install_hooks("nd2_core_${_}")
+  for qw/check early main user/;
+
 register 'register_worker' => sub {
   my ($self, $first, $second) = plugin_args(@_);
 
@@ -76,7 +78,7 @@ register 'register_worker' => sub {
   };
 
   # store the built worker as Worker.pm will build the dispatch order later on
-  push @{ vars->{'workers'}
+  push @{ vars->{'workers'}->{$workerconf->{action}}
               ->{$workerconf->{phase}}
               ->{$workerconf->{namespace}}
               ->{$workerconf->{priority}} }, $worker;
diff --git a/lib/App/Netdisco/Worker/Plugin/Test.pm b/lib/App/Netdisco/Worker/Plugin/Test.pm
deleted file mode 100644
index 96a7c6f6..00000000
--- a/lib/App/Netdisco/Worker/Plugin/Test.pm
+++ /dev/null
@@ -1,27 +0,0 @@
-package App::Netdisco::Worker::Plugin::Test;
-
-use Dancer ':syntax';
-use App::Netdisco::Worker::Plugin;
-use aliased 'App::Netdisco::Worker::Status';
-
-register_worker({ phase => 'check', driver => 'snmp' }, sub {
-  my ($job, $workerconf) = @_;
-  return Status->done('Test (check) ran successfully.');
-});
-
-register_worker({ phase => 'check', priority => 100 }, sub {
-  my ($job, $workerconf) = @_;
-  return Status->done('Test (check 100) ran successfully.');
-});
-
-register_worker({ phase => 'check', priority => 120 }, sub {
-  my ($job, $workerconf) = @_;
-  return Status->done('Test (check 120) ran successfully.');
-});
-
-register_worker({ phase => 'check', driver => 'eapi' }, sub {
-  my ($job, $workerconf) = @_;
-  return Status->done('Test (check eapi) ran successfully.');
-});
-
-true;
diff --git a/lib/App/Netdisco/Worker/Plugin/Test/Core.pm b/lib/App/Netdisco/Worker/Plugin/Test/Core.pm
deleted file mode 100644
index 619ad4c6..00000000
--- a/lib/App/Netdisco/Worker/Plugin/Test/Core.pm
+++ /dev/null
@@ -1,37 +0,0 @@
-package App::Netdisco::Worker::Plugin::Test::Core;
-
-use Dancer ':syntax';
-use App::Netdisco::Worker::Plugin;
-use aliased 'App::Netdisco::Worker::Status';
-
-register_worker({ phase => 'early' }, sub {
-  my ($job, $workerconf) = @_;
-  return Status->error('Test (early) ran successfully.');
-});
-
-register_worker({ phase => 'early' }, sub {
-  my ($job, $workerconf) = @_;
-  return Status->done('Test (early) ran successfully.');
-});
-
-register_worker({ phase => 'main' }, sub {
-  my ($job, $workerconf) = @_;
-  return Status->done('Test (main) ran successfully (1).');
-});
-
-register_worker({ phase => 'main' }, sub {
-  my ($job, $workerconf) = @_;
-  return Status->done('Test (main) ran successfully (2).');
-});
-
-register_worker(sub {
-  my ($job, $workerconf) = @_;
-  return Status->noop('Test (user) ran successfully.');
-});
-
-register_worker(sub {
-  my ($job, $workerconf) = @_;
-  return Status->error('Test (user) ran successfully.');
-});
-
-true;
diff --git a/xt/30-backend-workers.t b/xt/30-backend-workers.t
new file mode 100644
index 00000000..76bdcdef
--- /dev/null
+++ b/xt/30-backend-workers.t
@@ -0,0 +1,67 @@
+#!/usr/bin/env perl
+
+use strict; use warnings FATAL => 'all';
+use Test::More 0.88;
+
+use lib 'xt/lib';
+
+use App::Netdisco;
+use App::Netdisco::Backend::Job;
+
+use Try::Tiny;
+use Dancer qw/:moose :script !pass/;
+
+# configure logging to force console output
+my $CONFIG = config();
+$CONFIG->{logger} = 'console';
+$CONFIG->{log} = 'error';
+Dancer::Logger->init('console', $CONFIG);
+
+{
+  package MyWorker;
+  use Moo;
+  with 'App::Netdisco::Worker::Runner';
+}
+
+# TESTS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+my $j1 = do_job('TestOne');
+is($j1->status, 'done', 'status is done');
+is($j1->log, 'OK: SNMP driver is successful.',
+  'workers are run in decreasing priority until done');
+
+my $j2 = do_job('TestTwo');
+is($j2->status, 'done', 'status is done');
+is($j2->log, 'OK: CLI driver is successful.',
+  'lower priority driver not run if higher is successful');
+
+# TESTS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+done_testing;
+
+sub do_job {
+  my $pkg = shift;
+
+  # include local plugins
+  config->{'extra_worker_plugins'} = ["X::${pkg}"];
+
+  # clear out any previous installed hooks
+  Dancer::Factory::Hook->init( Dancer::Factory::Hook->instance() );
+
+  my $job = App::Netdisco::Backend::Job->new({
+    job => 0,
+    action => lc($pkg),
+  });
+
+  try {
+    #info sprintf 'test: started at %s', scalar localtime;
+    MyWorker->new()->run($job);
+    #info sprintf 'test: %s: %s', $job->status, $job->log;
+  }
+  catch {
+    $job->status('error');
+    $job->log("error running job: $_");
+  };
+
+  return $job;
+}
diff --git a/xt/lib/App/NetdiscoX/Worker/Plugin/TestOne.pm b/xt/lib/App/NetdiscoX/Worker/Plugin/TestOne.pm
new file mode 100644
index 00000000..0dfeabfd
--- /dev/null
+++ b/xt/lib/App/NetdiscoX/Worker/Plugin/TestOne.pm
@@ -0,0 +1,17 @@
+package App::NetdiscoX::Worker::Plugin::TestOne;
+
+use Dancer ':syntax';
+use App::Netdisco::Worker::Plugin;
+use aliased 'App::Netdisco::Worker::Status';
+
+# info 'test: workers are run in decreasing priority until done';
+
+register_worker({ phase => 'main', driver => 'cli' }, sub {
+  return Status->noop('NOT OK: CLI driver is not the winner here.');
+});
+
+register_worker({ phase => 'main', driver => 'snmp' }, sub {
+  return Status->done('OK: SNMP driver is successful.');
+});
+
+true;
diff --git a/xt/lib/App/NetdiscoX/Worker/Plugin/TestTwo.pm b/xt/lib/App/NetdiscoX/Worker/Plugin/TestTwo.pm
new file mode 100644
index 00000000..79be4133
--- /dev/null
+++ b/xt/lib/App/NetdiscoX/Worker/Plugin/TestTwo.pm
@@ -0,0 +1,17 @@
+package App::NetdiscoX::Worker::Plugin::TestTwo;
+
+use Dancer ':syntax';
+use App::Netdisco::Worker::Plugin;
+use aliased 'App::Netdisco::Worker::Status';
+
+# info 'test: lower priority driver not run if higher is successful';
+
+register_worker({ phase => 'main', driver => 'snmp' }, sub {
+  return Status->done('NOT OK: SNMP driver should NOT be run.');
+});
+
+register_worker({ phase => 'main', driver => 'cli' }, sub {
+  return Status->done('OK: CLI driver is successful.');
+});
+
+true;