Merge pull request #3183 from perlpunk/schedule-reload

Prevent schedule from being reread unnecessarily
os-autoinst · Jun 18, 2020 · 560f446 · 560f446
2 parents 7390623 + 64caa7a
commit 560f446
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 8 deletions.
diff --git a/lib/OpenQA/Worker/Job.pm b/lib/OpenQA/Worker/Job.pm
@@ -729,24 +729,23 @@ sub _upload_results_step_0_prepare {
         test_execution_paused => 0,
     );
 
-    my $test_status         = -r $status_file ? decode_json(path($status_file)->slurp) : {};
-    my $running_or_finished = ($test_status->{status} || '') =~ m/^(?:running|finished)$/;
-    my $running_test        = $test_status->{current_test} || '';
+    my $test_status  = -r $status_file ? decode_json(path($status_file)->slurp) : {};
+    my $running      = ($test_status->{status} || '') eq 'running';
+    my $finished     = ($test_status->{status} || '') eq 'finished';
+    my $running_test = $test_status->{current_test} || '';
     $status{test_execution_paused} = $test_status->{test_execution_paused} // 0;
 
     # determine up to which module the results should be uploaded
     my $current_test_module = $self->current_test_module;
     my $upload_up_to;
-    if ($self->{_has_uploaded_logs_and_assets} || $running_or_finished) {
+    if ($self->{_has_uploaded_logs_and_assets} || $running || $finished) {
         my @file_info = stat $self->_result_file_path('test_order.json');
         my $test_order;
         my $changed_schedule = (
             $self->{_test_order_mtime} and ($file_info[9] != $self->{_test_order_mtime}
                 or $file_info[7] != $self->{_test_order_fsize}));
         if (not $current_test_module or $changed_schedule) {
             log_info('Test schedule has changed, reloading test_order.json') if $changed_schedule;
-            # We need to reread the schedule when $current_test_module is not set
-            # Not clear why
             $test_order                = $self->_read_json_file('test_order.json');
             $status{test_order}        = $test_order;
             $self->{_test_order}       = $test_order;
@@ -763,7 +762,11 @@ sub _upload_results_step_0_prepare {
         elsif ($current_test_module ne $running_test) {    # next test
             $upload_up_to = $current_test_module;
         }
-        $self->{_current_test_module} = $current_test_module = $running_test;
+        if ($running_test or $finished) {
+            # $running_test could be empty in between tests
+            # prevent $current_test_module from getting empty
+            $self->{_current_test_module} = $current_test_module = $running_test;
+        }
     }
 
     # adjust $upload_up_to to handle special cases

diff --git a/t/24-worker-jobs.t b/t/24-worker-jobs.t
@@ -1052,6 +1052,17 @@ $engine_mock->redefine(
     });
 
 subtest 'Dynamic schedule' => sub {
+    my $job_mock      = Test::MockModule->new('OpenQA::Worker::Job');
+    my $orig          = \&OpenQA::Worker::Job::_read_json_file;
+    my $read_schedule = 0;
+    $job_mock->redefine(
+        _read_json_file => sub {
+            my ($self, $name) = @_;
+            if ($name eq 'test_order.json') {
+                $read_schedule++;
+            }
+            return $orig->($self, $name);
+        });
     is_deeply $client->websocket_connection->sent_messages, [], 'no WebSocket calls yet';
     is_deeply $client->sent_messages, [], 'no REST-API calls yet';
 
@@ -1084,10 +1095,13 @@ subtest 'Dynamic schedule' => sub {
     $job->_upload_results_step_0_prepare(sub { });
     is_deeply $job->{_test_order}, $test_order, 'Initial test schedule';
 
-    # do not log that test_order.json has changed when it didn't
+    # do not reload test_order.json when it hasn't changed
     $autoinst_status->{current_test} = '';
     $status_file->spurt(encode_json($autoinst_status));
     $job->_upload_results_step_0_prepare(sub { });
+    $job->_upload_results_step_0_prepare(sub { });
+    $job->_upload_results_step_0_prepare(sub { });
+    is($read_schedule, 1, 'test_order.json has only been read once');
 
     combined_unlike {
         $job->_upload_results_step_0_prepare(sub { })