load_templates: better error handling

- The unit test also runs a webui with fixtures to verify how templates are loaded and how errors are handled. - SeleniumTest::start_app is re-based onto OpenQA::TEST::Utils::create_webapi since both do the same except the latter was missing the schema hook. - Fail cleanly in JobTemplate::update if not template was specified. - Make Selenium start helpers internal
os-autoinst · Feb 5, 2020 · a6b0414 · a6b0414
1 parent 0289375
commit a6b0414
Show file tree

Hide file tree

Showing 9 changed files with 111 additions and 67 deletions.
diff --git a/lib/OpenQA/WebAPI/Controller/API/V1/JobTemplate.pm b/lib/OpenQA/WebAPI/Controller/API/V1/JobTemplate.pm
@@ -218,9 +218,11 @@ sub update {
     my $yaml   = {};
     my $errors = [];
     try {
+        my $template = $self->param('template') or die "No template specified\n";
+
         # No objects (aka SafeYAML)
         $YAML::XS::LoadBlessed = 0;
-        $yaml                  = YAML::XS::Load($self->param('template'));
+        $yaml                  = YAML::XS::Load($template);
         $errors = $self->app->validate_yaml($yaml, $self->param('schema'), $self->app->log->level eq 'debug');
     }
     catch {

diff --git a/script/load_templates b/script/load_templates
@@ -75,6 +75,7 @@ use OpenQA::Client;
 use Mojo::JSON;    # booleans
 use Cpanel::JSON::XS ();
 use YAML::XS;
+use Scalar::Util qw(reftype);
 
 use Getopt::Long;
 
@@ -130,11 +131,17 @@ my @tables = (qw(Machines TestSuites Products JobTemplates JobGroups));
 
 sub print_error {
     my ($res) = @_;
-    die 'unknown error code - host ' . $url->host . ' unreachable?' unless $res->code;
-    printf STDERR "ERROR: %s - %s\n", $res->code, $res->message // 'no error message';
-    if ($res->body) {
-        dd($res->json || $res->body);
+    if (my $err = $res->error) {
+        if (my $json = $res->json) {
+            if (my $json_err = $json->{error}) {
+                die "$err->{message}: " . join("\n", @{$json_err})
+                  if reftype $json_err eq 'ARRAY';
+                die "$err->{message}: $json_err";
+            }
+        }
+        die "ERROR: $err->{code} - $err->{message}" if $err->{code};
     }
+    die "unknown error code - host $url->{host} unreachable?";
 }
 
 sub post_entry {

diff --git a/t/05-scheduler-full.t b/t/05-scheduler-full.t
@@ -65,7 +65,7 @@ my $schema = OpenQA::Test::Database->new->create(skip_schema => 1);
 # Create webapi and websocket server services
 my $mojoport = Mojo::IOLoop::Server->generate_port();
 my $wspid    = create_websocket_server($mojoport + 1, 0, 1, 1);
-my $webapi   = create_webapi($mojoport);
+my $webapi   = create_webapi($mojoport, sub { });
 
 # Setup needed files for workers.
 

diff --git a/t/14-grutasks.t b/t/14-grutasks.t
@@ -24,8 +24,8 @@ use OpenQA::Utils;
 use OpenQA::Jobs::Constants;
 use OpenQA::Schema::Result::Jobs;
 use File::Copy;
-use OpenQA::SeleniumTest;
 use OpenQA::Test::Database;
+use OpenQA::Test::Utils;
 use Test::Output 'combined_like';
 use Test::MockModule;
 use Test::More;
@@ -40,6 +40,7 @@ use Fcntl ':mode';
 use Mojo::File 'tempdir';
 use Mojo::Log;
 use Storable qw(store retrieve);
+use Mojo::IOLoop;
 
 # these are used to track assets being 'removed from disk' and 'deleted'
 # by mock methods (so we don't *actually* lose them)
@@ -95,7 +96,8 @@ $assets_mock->mock(refresh_assets            => sub { });
 my $t = Test::Mojo->new('OpenQA::WebAPI');
 
 # launch an additional app to serve some file for testing blocking downloads
-my $mojo_port = OpenQA::SeleniumTest::start_app(sub { });
+my $mojo_port = Mojo::IOLoop::Server->generate_port;
+my $pid       = OpenQA::Test::Utils::create_webapi($mojo_port, sub { });
 
 # define a fix asset_size_limit configuration for this test to be independent of the default value
 # we possibly want to adjust without going into the details of this test
@@ -576,6 +578,8 @@ subtest 'download assets with correct permissions' => sub {
     }
 };
 
+kill TERM => $pid;
+
 done_testing();
 
 # clear gru task queue at end of execution so no 'dangling' tasks

diff --git a/t/40-script_load_templates.t b/t/40-script_load_templates.t
@@ -18,6 +18,8 @@ use Mojo::Base -strict;
 
 use File::Temp qw(tempfile);
 use Mojo::File qw(path curfile);
+use OpenQA::Test::Database;
+use OpenQA::Test::Utils;
 use Test::More;
 use Test::Output;
 use Test::Warnings;
@@ -42,4 +44,41 @@ $args = "--host $host $filename";
 combined_like sub { $ret = run_once($args); }, qr/unknown error code - host $host unreachable?/, 'invalid host error';
 is $ret, 22, 'error because host is invalid';
 
+$ENV{MOJO_LOG_LEVEL} = 'fatal';
+my $mojoport = Mojo::IOLoop::Server->generate_port;
+$host = "localhost:$mojoport";
+my $pid = OpenQA::Test::Utils::create_webapi($mojoport, sub { OpenQA::Test::Database->new->create; });
+# Note: See t/fixtures/03-users.pl for test user credentials
+my $apikey    = 'PERCIVALKEY02';
+my $apisecret = 'PERCIVALSECRET02';
+$args = "--host $host --apikey $apikey --apisecret $apisecret $filename";
+combined_like sub { $ret = run_once($args); }, qr/Administrator level required/, 'Operator is not allowed';
+is $ret, 255, 'error because of insufficient permissions';
+
+$apikey    = 'ARTHURKEY01';
+$apisecret = 'EXCALIBUR';
+$args      = "--host $host --apikey $apikey --apisecret $apisecret $filename";
+combined_like sub { $ret = run_once($args); }, qr/Job group.+not found/, 'Invalid job group';
+is $ret, 255, 'failed to load templates with invalid job group';
+kill TERM => $pid;
+
+sub schema_hook {
+    my $schema     = OpenQA::Test::Database->new->create;
+    my $job_groups = $schema->resultset('JobGroups');
+    $job_groups->create({name => 'openSUSE Leap 42.3 Updates'});
+}
+$mojoport = Mojo::IOLoop::Server->generate_port;
+$host     = "localhost:$mojoport";
+$pid      = OpenQA::Test::Utils::create_webapi($mojoport, \&schema_hook);
+$args     = "--host $host --apikey $apikey --apisecret $apisecret $filename";
+stdout_like sub { $ret = run_once($args); }, qr/JobGroups.+=> \{ added => 1, of => 1 \}/, 'Admin may load templates';
+is $ret, 0, 'successfully loaded templates';
+
+$filename = 't/data/40-templates.json';
+$args     = "--host $host --apikey $apikey --apisecret $apisecret $filename";
+combined_like sub { $ret = run_once($args); }, qr/Bad Request: No template specified/,
+  'YAML template is mandatory (JSON)';
+is $ret, 255, 'failed to load templates without YAML (JSON)';
+kill TERM => $pid;
+
 done_testing;
diff --git a/t/data/40-templates.json b/t/data/40-templates.json
@@ -0,0 +1,35 @@
+{
+   "Machines" : [
+      {
+         "name" : "32bit",
+         "backend" : "qemu"
+      }
+   ],
+   "JobGroups" : [
+      {
+         "group_name" : "openSUSE Leap 42.3 Updates"
+      }
+   ],
+   "Products" : [
+      {
+         "arch" : "x86_64",
+         "version" : "42.2",
+         "flavor" : "DVD",
+         "distri" : "opensuse"
+      }
+   ],
+   "JobTemplates" : [
+   ],
+   "TestSuites" : [
+      {
+         "description" : "Maintainer: averagea\n\nThat's some test right there",
+         "name" : "uefi",
+         "settings" : [
+            {
+               "key" : "DESKTOP",
+               "value" : "gnome"
+            }
+         ]
+      }
+   ]
+}
diff --git a/t/lib/OpenQA/SeleniumTest.pm b/t/lib/OpenQA/SeleniumTest.pm
@@ -23,6 +23,7 @@ use Try::Tiny;
 use Time::HiRes 'time';
 use OpenQA::WebAPI;
 use OpenQA::Utils;
+use OpenQA::Test::Utils;
 use POSIX '_exit';
 
 our $_driver;
@@ -32,68 +33,20 @@ our $mojoport;
 our $startingpid   = 0;
 our $drivermissing = 'Install Selenium::Remote::Driver and Selenium::Chrome to run these tests';
 
-=head2 start_app
-
-  start_app([$schema_hook]);
-
-Fork a server instance with database creation and return the server port.
-
-By default the database is created based on the fixture set.
-
-The optional parameter C<$schema_hook> allows to provide a custom way of creating a database, e.g.
-
-    sub schema_hook {
-        my $schema = OpenQA::Test::Database->new->create;
-        # delete unused job id 1234
-        $schema->resultset('Jobs')->find(1234)->delete;
-    }
-    start_app(\&schema_hook);
-=cut
-
-sub start_app {
+sub _start_app {
     my ($schema_hook, $args) = @_;
+    $schema_hook = sub { OpenQA::Test::Database->new->create }
+      unless $schema_hook;
     $mojoport = $args->{mojoport} // $ENV{MOJO_PORT} // Mojo::IOLoop::Server->generate_port;
 
     $startingpid = $$;
-    $mojopid     = fork();
-    if ($mojopid == 0) {
-        log_info("inserting fixtures into database\n");
-        if ($schema_hook) {
-            $schema_hook->();
-        }
-        else {
-            OpenQA::Test::Database->new->create;
-        }
-
-        log_info("starting web UI\n");
-        $ENV{MOJO_MODE} = 'test';
-        my $daemon = Mojo::Server::Daemon->new(
-            listen => ["http://127.0.0.1:$mojoport"],
-            silent => 1,
-        );
-        $daemon->build_app('OpenQA::WebAPI');
-        $daemon->run;
-        Devel::Cover::report() if Devel::Cover->can('report');
-        _exit(0);
-    }
+    $mojopid     = OpenQA::Test::Utils::create_webapi($mojoport, $schema_hook);
 
-    # as this might download assets on first test, we need to wait a while
-    my $wait = time + 50;
-    while (time < $wait) {
-        my $t      = time;
-        my $socket = IO::Socket::INET->new(
-            PeerHost => '127.0.0.1',
-            PeerPort => $mojoport,
-            Proto    => 'tcp',
-        );
-        last    if $socket;
-        sleep 1 if time - $t < 1;
-    }
-    start_gru() if ($args->{with_gru});
+    _start_gru() if ($args->{with_gru});
     return $mojoport;
 }
 
-sub start_gru {
+sub _start_gru {
     $gru_pid = fork();
     if ($gru_pid == 0) {
         log_info("starting gru\n");
@@ -220,7 +173,7 @@ sub call_driver {
     # are available, otherwise return undef
     return undef unless check_driver_modules;
     my ($schema_hook, $args) = @_;
-    my $mojoport = start_app($schema_hook, $args);
+    my $mojoport = _start_app($schema_hook, $args);
     return start_driver($mojoport);
 }
 

diff --git a/t/lib/OpenQA/Test/Utils.pm b/t/lib/OpenQA/Test/Utils.pm
@@ -173,12 +173,16 @@ sub wait_for_worker {
 }
 
 sub create_webapi {
-    my $mojoport = shift;
+    my ($mojoport, $schema_hook) = @_;
+    die 'No port specified '       unless $mojoport;
+    die 'No schema hook specified' unless $schema_hook;
     note("Starting WebUI service. Port: $mojoport");
 
-    my $startingpid = $$;
-    my $mojopid     = fork();
+    my $mojopid = fork();
     if ($mojopid == 0) {
+        log_info("inserting fixtures into database\n");
+        $schema_hook->();
+
         local $ENV{MOJO_MODE} = 'test';
         my $daemon = Mojo::Server::Daemon->new(listen => ["http://127.0.0.1:$mojoport"], silent => 1);
         $daemon->build_app('OpenQA::WebAPI');

diff --git a/t/ui/27-plugin_obs_rsync_status_details.t b/t/ui/27-plugin_obs_rsync_status_details.t
@@ -158,7 +158,7 @@ foreach my $proj (sort { $b cmp $a } keys %params) {
 
     # refresh page and make sure that last sync is gone
     $driver->get("/admin/obs_rsync/$parent");
-    is($driver->find_element("tr#folder_$proj .lastsync")->get_text(), 'no data', "$proj last sync forgotten");
+    is($driver->find_element("tr#folder_$proj .lastsync")->get_text(), 'no data', "$proj last sync absent from web UI");
 
     # Update project status
     $driver->find_element("tr#folder_$proj .dirtystatusupdate")->click();