From 26f2b94d4b6d9b500dcdade55b1e33caaaa87e30 Mon Sep 17 00:00:00 2001 From: Sebastian Riedel Date: Mon, 15 Feb 2021 12:07:46 +0100 Subject: [PATCH 1/4] Fix Mojolicious 9.0 compatibility 1. Use of reserved stash values as placeholders in route patterns is forbidden. We used "/:status". 2. Log messages are now joined with a whitespace character instead of a newline by Mojo::Log. We were using a custom formatter that handled it wrong. 3. Missing controllers are now exceptions. We were relying on "->to('search#search')" to render a template without controller/action. 4. Mojo::IOLoop::Delay no longer exists. Mojo::Promise should be used instead. --- lib/OpenQA/Log.pm | 6 +++--- lib/OpenQA/WebAPI.pm | 4 ++-- lib/OpenQA/WebAPI/Controller/API/V1/Mm.pm | 14 +++++++------- t/26-controllerrunning.t | 12 ++++++------ 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/lib/OpenQA/Log.pm b/lib/OpenQA/Log.pm index 0d51790166e..022ef22fd7c 100644 --- a/lib/OpenQA/Log.pm +++ b/lib/OpenQA/Log.pm @@ -181,7 +181,7 @@ sub log_format_callback { $time = gettimeofday; return sprintf(strftime("[%FT%T.%%04d %Z] [$level] ", localtime($time)), 1000 * ($time - int($time))) - . join("\n", @lines, ''); + . join(' ', @lines) . "\n"; } # Removes a channel from defaults. @@ -243,8 +243,8 @@ sub setup_log { $log = $log_module->new(%settings, handle => \*STDOUT); $log->format( sub { - my ($time, $level, @lines) = @_; - return "[$level] " . join "\n", @lines, ''; + my ($time, $level, @parts) = @_; + return "[$level] " . join(' ', @parts) . "\n"; }); } diff --git a/lib/OpenQA/WebAPI.pm b/lib/OpenQA/WebAPI.pm index e40d77428e0..12e865c0687 100644 --- a/lib/OpenQA/WebAPI.pm +++ b/lib/OpenQA/WebAPI.pm @@ -128,7 +128,7 @@ sub startup ($self) { $apik_auth->post('/')->to('api_key#create'); $apik_auth->delete('/:apikeyid')->name('api_key')->to('api_key#destroy'); - $r->get('/search')->name('search')->to('search#search'); + $r->get('/search')->name('search')->to(template => 'search/search'); $r->get('/tests')->name('tests')->to('test#list'); $r->get('/tests/overview')->name('tests_overview')->to('test#overview'); @@ -358,7 +358,7 @@ sub startup ($self) { # api/v1/mm my $mm_api = $api_r_job->any('/mm'); push @api_routes, $mm_api; - $mm_api->get('/children/:status' => [status => [qw(running scheduled done)]])->name('apiv1_mm_running_children') + $mm_api->get('/children/:state' => [state => [qw(running scheduled done)]])->name('apiv1_mm_running_children') ->to('mm#get_children_status'); $mm_api->get('/children')->name('apiv1_mm_children')->to('mm#get_children'); $mm_api->get('/parents')->name('apiv1_mm_parents')->to('mm#get_parents'); diff --git a/lib/OpenQA/WebAPI/Controller/API/V1/Mm.pm b/lib/OpenQA/WebAPI/Controller/API/V1/Mm.pm index bd2968c4e53..6dedb093483 100644 --- a/lib/OpenQA/WebAPI/Controller/API/V1/Mm.pm +++ b/lib/OpenQA/WebAPI/Controller/API/V1/Mm.pm @@ -52,20 +52,20 @@ JSON block with the list. # IMHO it should be replaced with get_children and removed sub get_children_status { my ($self) = @_; - my $status = $self->stash('status'); - if ($status eq 'running') { - $status = OpenQA::Jobs::Constants::RUNNING; + my $state = $self->stash('state'); + if ($state eq 'running') { + $state = OpenQA::Jobs::Constants::RUNNING; } - elsif ($status eq 'scheduled') { - $status = OpenQA::Jobs::Constants::SCHEDULED; + elsif ($state eq 'scheduled') { + $state = OpenQA::Jobs::Constants::SCHEDULED; } else { - $status = OpenQA::Jobs::Constants::DONE; + $state = OpenQA::Jobs::Constants::DONE; } my $jobid = $self->stash('job_id'); my @res = $self->schema->resultset('Jobs') - ->search({'parents.parent_job_id' => $jobid, state => $status}, {columns => ['id'], join => 'parents'}); + ->search({'parents.parent_job_id' => $jobid, state => $state}, {columns => ['id'], join => 'parents'}); my @res_ids = map { $_->id } @res; return $self->render(json => {jobs => \@res_ids}, status => 200); } diff --git a/t/26-controllerrunning.t b/t/26-controllerrunning.t index e850f4303b5..c545fddf0e6 100644 --- a/t/26-controllerrunning.t +++ b/t/26-controllerrunning.t @@ -26,6 +26,7 @@ use OpenQA::Jobs::Constants; use Mojolicious; use Mojo::File 'path'; use Mojo::IOLoop; +use Mojo::Promise; my $log_messages = ''; @@ -41,17 +42,16 @@ subtest streamtext => sub { $buffer .= $chunk; # uncoverable statement }); }); - my $port = Mojo::IOLoop->acceptor($id)->port; - my $delay = Mojo::IOLoop->delay; - my $end = $delay->begin; - my $handle = undef; + my $port = Mojo::IOLoop->acceptor($id)->port; + my $promise = Mojo::Promise->new; + my $handle = undef; Mojo::IOLoop->client( {port => $port} => sub { my ($loop, $err, $stream) = @_; $handle = $stream->steal_handle; - $end->(); + $promise->resolve; }); - $delay->wait; + $promise->wait; my $stream = Mojo::IOLoop::Stream->new($handle); $id = Mojo::IOLoop->stream($stream); From 15a7785b09b8f9c2d7250aecbebfde87803f257a Mon Sep 17 00:00:00 2001 From: Marius Kittler Date: Wed, 17 Feb 2021 14:56:28 +0100 Subject: [PATCH 2/4] Remove dependency to Mojo::Log::Colored as it is not Mojolicious 9 compatible It will not be necessary with Mojolicious 9.91 anyways --- lib/OpenQA/Log.pm | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/lib/OpenQA/Log.pm b/lib/OpenQA/Log.pm index 022ef22fd7c..dde587c7961 100644 --- a/lib/OpenQA/Log.pm +++ b/lib/OpenQA/Log.pm @@ -44,9 +44,6 @@ our @EXPORT_OK = qw( my %CHANNELS; my %LOG_DEFAULTS = (LOG_TO_STANDARD_CHANNEL => 1, CHANNELS => []); -my $log_module = "Mojo::Log"; -eval 'use Mojo::Log::Colored; $log_module = "Mojo::Log::Colored"'; - # logging helpers - _log_msg wrappers # log_debug("message"[, param1=>val1, param2=>val2]); @@ -170,7 +167,7 @@ sub add_log_channel { } delete $options{default}; } - $CHANNELS{$channel} = $log_module->new(%options); + $CHANNELS{$channel} = Mojo::Log->new(%options); $CHANNELS{$channel}->format(\&log_format_callback); } @@ -219,28 +216,18 @@ sub setup_log { $level //= $app->config->{logging}->{level} // 'info'; $logfile = $ENV{OPENQA_LOGFILE} || $app->config->{logging}->{file}; - # select a color selection that is compatible with reverse video terminals - # as well as standard - my %settings = ( - level => $level, - colors => { - debug => "white", - info => "yellow", - warn => "red", - error => "magenta", - fatal => "yellow on_red", - }); + my %settings = (level => $level); if ($logfile || $logdir) { $logfile = catfile($logdir, $logfile) if $logfile && $logdir; # So each worker from each host get its own log (as the folder can be shared). # Hopefully the machine hostname is already sanitized. Otherwise we need to check $logfile //= catfile($logdir, hostname() . (defined $app->instance ? "-${\$app->instance}" : '') . ".log"); - $log = $log_module->new(%settings, handle => path($logfile)->open('>>')); + $log = Mojo::Log->new(%settings, handle => path($logfile)->open('>>')); $log->format(\&log_format_callback); } else { - $log = $log_module->new(%settings, handle => \*STDOUT); + $log = Mojo::Log->new(%settings, handle => \*STDOUT); $log->format( sub { my ($time, $level, @parts) = @_; From 6ff2141ab3568eb9549004d875b5c60fa59bc0f5 Mon Sep 17 00:00:00 2001 From: Sebastian Riedel Date: Wed, 17 Feb 2021 16:07:11 +0100 Subject: [PATCH 3/4] The CI package has been updated to 9.01 already --- .circleci/ci-packages.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/ci-packages.txt b/.circleci/ci-packages.txt index dd0dc60f43d..789a927ef61 100644 --- a/.circleci/ci-packages.txt +++ b/.circleci/ci-packages.txt @@ -140,7 +140,7 @@ perl-Module-Pluggable-5.2 perl-Module-Runtime-0.016 perl-Module-Runtime-Conflicts-0.003 perl-Mojo-IOLoop-ReadWriteProcess-0.28 -perl-Mojolicious-8.73 +perl-Mojolicious-9.01 perl-Mojolicious-Plugin-AssetPack-2.10 perl-Mojolicious-Plugin-OAuth2-1.58 perl-Mojo-Pg-4.24 From 20d219db6b59207769cb5e0076e44396892339f3 Mon Sep 17 00:00:00 2001 From: Marius Kittler Date: Wed, 17 Feb 2021 16:37:51 +0100 Subject: [PATCH 4/4] Apply tidy to t/ui/27-plugin_obs_rsync_status_details.t --- t/ui/27-plugin_obs_rsync_status_details.t | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/t/ui/27-plugin_obs_rsync_status_details.t b/t/ui/27-plugin_obs_rsync_status_details.t index 33ac61b87ac..3bca6207f5c 100644 --- a/t/ui/27-plugin_obs_rsync_status_details.t +++ b/t/ui/27-plugin_obs_rsync_status_details.t @@ -134,13 +134,14 @@ sub _wait_helper { sleep .1; # uncoverable statement } # sometimes gru is not fast enough, so let's refresh the page and see if that helped - if($refresh) { # uncoverable statement + if ($refresh) { # uncoverable statement $refresh->(); # uncoverable statement - } else { + } + else { $driver->refresh(); # uncoverable statement } } - return $driver->find_element($element)->get_text(); # uncoverable statement + return $driver->find_element($element)->get_text(); # uncoverable statement } foreach my $proj (sort keys %params) { @@ -176,12 +177,14 @@ foreach my $proj (sort keys %params) { $builds_text = ($builds_text ? $builds_text : 'No data'); # now request fetching builds from obs $driver->find_element("tr#folder_$ident .obsbuildsupdate")->click(); - my $obsbuilds = _wait_helper("tr#folder_$ident .obsbuilds", sub { + my $obsbuilds = _wait_helper( + "tr#folder_$ident .obsbuilds", + sub { shift eq $builds_text; - }, sub { + }, + sub { $driver->find_element("tr#folder_$ident .obsbuildsupdate")->click(); - } - ); + }); is($obsbuilds, $builds_text, "$proj obs builds"); if ($dt ne 'no data') {