diff --git a/lib/OpenQA/Schema/Result/Jobs.pm b/lib/OpenQA/Schema/Result/Jobs.pm index 5981460927f..b0941979177 100644 --- a/lib/OpenQA/Schema/Result/Jobs.pm +++ b/lib/OpenQA/Schema/Result/Jobs.pm @@ -1783,27 +1783,26 @@ sub bugref { return undef; } -# extend to finish sub store_column { - my ($self, %args) = @_; - if ($args{state} && grep { $args{state} eq $_ } FINAL_STATES) { - if (!$self->t_finished) { - # make sure we do not overwrite a t_finished from fixtures - # in normal operation it should be impossible to finish - # twice - $self->t_finished(now()); - } + my ($self, $columnname, $value) = @_; + + # handle transition to the final state + if ($columnname eq 'state' && grep { $value eq $_ } FINAL_STATES) { + # make sure we do not overwrite a t_finished from fixtures + # note: In normal operation it should be impossible to finish twice. + $self->t_finished(now()) unless $self->t_finished; # make sure no modules are left running $self->modules->search({result => RUNNING})->update({result => NONE}); - # This function gets executed even when unit tests are setting up - # fixtures. $app and $self->id may not be set in that case. Additionally, - # the app might not be setup to have the "gru" method. - if (my $id = $self->id) { - my $gru = eval { OpenQA::App->singleton->gru }; - $gru->enqueue(finalize_job_results => [$id]) if $gru; - } } - return $self->SUPER::store_column(%args); + + return $self->SUPER::store_column($columnname, $value); +} + +sub enqueue_finalize_job_results { + my ($self) = @_; + + my $gru = eval { OpenQA::App->singleton->gru }; # gru might not be present within tests + $gru->enqueue(finalize_job_results => [$self->id]) if $gru; } # used to stop jobs with some kind of dependency relationship to another @@ -1992,6 +1991,7 @@ sub done { $new_val{reason} = $reason; } $self->update(\%new_val); + $self->enqueue_finalize_job_results; # stop other jobs in the cluster if (defined $new_val{result} && !grep { $result eq $_ } OK_RESULTS) { @@ -2013,6 +2013,7 @@ sub cancel { return undef if $self->result ne NONE; $self->release_networks; $self->update({state => CANCELLED, result => ($obsoleted ? OBSOLETED : USER_CANCELLED)}); + $self->enqueue_finalize_job_results; my $count = 1; if (my $worker = $self->assigned_worker) { $worker->send_command(command => WORKER_COMMAND_CANCEL, job_id => $self->id); diff --git a/t/14-grutasks.t b/t/14-grutasks.t index f2ad051fce9..5bc0f18c2b6 100644 --- a/t/14-grutasks.t +++ b/t/14-grutasks.t @@ -20,6 +20,7 @@ use FindBin; use lib "$FindBin::Bin/lib"; use OpenQA::Utils; use OpenQA::Jobs::Constants; +use OpenQA::JobDependencies::Constants; use OpenQA::Schema::Result::Jobs; use File::Copy; use OpenQA::Test::Database; @@ -72,13 +73,14 @@ $assets_result_mock->redefine( $assets_result_mock->redefine(refresh_size => sub { }); # setup test config and database -my $test_case = OpenQA::Test::Case->new(config_directory => "$FindBin::Bin/data/41-audit-log"); -my $schema = $test_case->init_data(fixtures_glob => '01-jobs.pl 04-products.pl'); -my $jobs = $schema->resultset('Jobs'); -my $job_groups = $schema->resultset('JobGroups'); -my $parent_groups = $schema->resultset('JobGroupParents'); -my $assets = $schema->resultset('Assets'); -my $job_assets = $schema->resultset('JobsAssets'); +my $test_case = OpenQA::Test::Case->new(config_directory => "$FindBin::Bin/data/41-audit-log"); +my $schema = $test_case->init_data(fixtures_glob => '01-jobs.pl 04-products.pl'); +my $jobs = $schema->resultset('Jobs'); +my $job_groups = $schema->resultset('JobGroups'); +my $job_dependencies = $schema->resultset('JobDependencies'); +my $parent_groups = $schema->resultset('JobGroupParents'); +my $assets = $schema->resultset('Assets'); +my $job_assets = $schema->resultset('JobsAssets'); # move group 1002 into a parent group # note: This shouldn't change much because 1002 will be the only child and the same limit applies. @@ -642,7 +644,10 @@ subtest 'finalize job results' => sub { ARCH => 'x86_64', TEST => 'minion', ); - my $job = $schema->resultset('Jobs')->create_from_settings(\%settings); + my $job = $jobs->create_from_settings(\%settings); + my $child_job = $jobs->create_from_settings(\%settings); + my @chained = (dependency => OpenQA::JobDependencies::Constants::CHAINED); + $job_dependencies->create({child_job_id => $child_job->id, parent_job_id => $job->id, @chained}); $job->discard_changes; $job->insert_module({name => 'a', category => 'a', script => 'a', flags => {}}); $job->update_module('a', {result => 'ok', details => [{title => 'wait_serial', text => 'a-0.txt'}]}); @@ -661,18 +666,20 @@ subtest 'finalize job results' => sub { my $a_txt = path($job->result_dir, 'a-0.txt')->spurt('Foo'); my $b_txt = path('t/data/14-module-b.txt')->copy_to($job->result_dir . '/b-0.txt'); $job->done; - $job->discard_changes; - is($job->result, OpenQA::Jobs::Constants::FAILED, 'job result is failed'); + $_->discard_changes for ($job, $child_job); + is($job->result, FAILED, 'job result is failed'); + is($child_job->state, CANCELLED, 'child job cancelled'); + is($child_job->result, SKIPPED, 'child job skipped'); $minion->perform_jobs; my $minion_jobs = $minion->jobs({tasks => ['finalize_job_results']}); - is($minion_jobs->total, 1, 'one minion job executed') + is($minion_jobs->total, 1, 'one minion job created; no minion job for skipped child job created') and is($minion_jobs->next->{state}, 'finished', 'the minion job succeeded'); - ok(!-e $a_txt); - ok(!-e $b_txt); + ok(!-e $a_txt, 'extra txt file for module a actually gone'); + ok(!-e $b_txt, 'extra txt file for module b actually gone'); my @modlist = $job->modules; - is($modlist[0]->results->{details}->[0]->{text_data}, 'Foo'); - is($modlist[1]->results->{details}->[0]->{text_data}, "正解\n"); - is($a_details->stat->mode & 0644, 0644, 'details JSON globally readable'); + is($modlist[0]->results->{details}->[0]->{text_data}, 'Foo', 'text data for module a still present'); + is($modlist[1]->results->{details}->[0]->{text_data}, "正解\n", 'text data for module b still present'); + is($a_details->stat->mode & 0644, 0644, 'details JSON globally readable'); }; subtest 'enqueue finalize_job_results without job or job which (no longer) exists' => sub {