From ba9ced238f826313adeac9f0f286ef682134b3cb Mon Sep 17 00:00:00 2001 From: Marius Kittler Date: Thu, 16 Nov 2023 17:28:57 +0100 Subject: [PATCH 1/2] Do not associate skipped child jobs with new parent when restarting jobs See https://progress.opensuse.org/issues/150917 --- lib/OpenQA/Schema/Result/Jobs.pm | 1 + t/05-scheduler-dependencies.t | 26 ++++++++++---------------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/lib/OpenQA/Schema/Result/Jobs.pm b/lib/OpenQA/Schema/Result/Jobs.pm index 3b9ff347ab8..420ce80bffa 100644 --- a/lib/OpenQA/Schema/Result/Jobs.pm +++ b/lib/OpenQA/Schema/Result/Jobs.pm @@ -643,6 +643,7 @@ sub _create_clone_with_parent ($res, $clones, $p, $dependency) { } sub _create_clone_with_child ($res, $clones, $c, $dependency) { + return undef unless exists $clones->{$c}; $c = $clones->{$c}->id if defined $clones->{$c}; $res->children->find_or_create({child_job_id => $c, dependency => $dependency}); } diff --git a/t/05-scheduler-dependencies.t b/t/05-scheduler-dependencies.t index 87114dae39b..5719a3b9d84 100644 --- a/t/05-scheduler-dependencies.t +++ b/t/05-scheduler-dependencies.t @@ -1121,22 +1121,16 @@ subtest 'skip "ok" children' => sub { my $new_job_cluster = $clone->cluster_jobs; subtest 'dependencies of cloned jobs' => sub { - my @expected_job_ids = ((map { $_->clone_id } ($parent, $child_2, $child_2_child_1, $child_3)), $child_1->id); - is(ref $new_job_cluster->{$_}, 'HASH', "new cluster contains job $_") for @expected_job_ids; - is_deeply( - [sort @{$new_job_cluster->{$clone->id}{directly_chained_children}}], - [$child_1->id, $child_2->clone_id, $child_3->clone_id], - 'parent contains all children, including the not restarted one' - ); - is_deeply( - [sort @{$new_job_cluster->{$child_1->id}{directly_chained_parents}}], - [$parent->id, $clone->id], - 'skipped job has nevertheless new parent (besides old one) to appear in the new dependency tree as well' - ); - is_deeply([sort @{$new_job_cluster->{$child_2_child_1->clone_id}{directly_chained_parents}}], - [$child_2->clone_id], 'parent for child of child assigned'); - # note: It is not exactly clear whether creating a dependency between the skipped job and the new - # parent is the best behavior but let's assert it for now. + my @expected_job_ids = map { $_->clone_id } ($parent, $child_2, $child_2_child_1, $child_3); + is ref $new_job_cluster->{$_}, 'HASH', "new cluster contains job $_" for @expected_job_ids; + is_deeply [sort @{$new_job_cluster->{$clone->id}{directly_chained_children}}], + [$child_2->clone_id, $child_3->clone_id], + 'parent contains all children, except the skipped one (as per poo#150917)'; + my @child_1_parents = map { $_->parent_job_id } $child_1->parents->all; + is_deeply \@child_1_parents, [$parent->id], + 'skipped job has only the old parent and not also the new parent (as per poo#150917)'; + is_deeply [sort @{$new_job_cluster->{$child_2_child_1->clone_id}{directly_chained_parents}}], + [$child_2->clone_id], 'parent for child of child assigned'; } or $log_jobs->() or diag explain $new_job_cluster; }; From ea9bf5cbd94352269c247781503aa091914ab7aa Mon Sep 17 00:00:00 2001 From: Marius Kittler Date: Thu, 16 Nov 2023 18:05:38 +0100 Subject: [PATCH 2/2] Improve comment in test code for dependency handling --- t/05-scheduler-dependencies.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/05-scheduler-dependencies.t b/t/05-scheduler-dependencies.t index 5719a3b9d84..dfe33a70a0a 100644 --- a/t/05-scheduler-dependencies.t +++ b/t/05-scheduler-dependencies.t @@ -843,7 +843,7 @@ subtest 'clone chained child with siblings; then clone chained parent' => # |- C # \- D - # B failed, auto clone it + # auto duplicate the incomplete job B my $jobBc = $jobB->auto_duplicate({dup_type_auto => 1}); ok($jobBc, 'jobB duplicated');