Francois Gouget : testbot/TestWTBS: Allow checking the patches disposition.

[Alexandre Julliard]

Module: tools
Branch: master
Commit: 18965f76c01c7cb6bb2729fea49ca4a24120b704
URL:    https://source.winehq.org/git/tools.git/?a=commit;h=18965f76c01c7cb6bb2729fea49ca4a24120b704

Author: Francois Gouget <fgouget at codeweavers.com>
Date:   Mon Jan 25 14:52:34 2021 +0100

testbot/TestWTBS: Allow checking the patches disposition.

This allows verifying that patches that should not result in a TestBot
job are indeed ignored for the right reason (aka disposition).

For instance:
----- TestWTBS -----
p patch.Disposition Does not impact the Wine build
----- TestWTBS -----

Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard at winehq.org>

---

 testbot/tests/TestWTBS | 115 +++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 106 insertions(+), 9 deletions(-)

diff --git a/testbot/tests/TestWTBS b/testbot/tests/TestWTBS
index 58bf8a7..5686428 100755
--- a/testbot/tests/TestWTBS
+++ b/testbot/tests/TestWTBS
@@ -48,6 +48,7 @@ use WineTestBot::Config; # For $PatchesMailingList
 use WineTestBot::Jobs;
 use WineTestBot::Log;
 use WineTestBot::LogUtils;
+use WineTestBot::Patches;
 use WineTestBot::VMs;
 
 
@@ -161,9 +162,10 @@ if (defined $Usage)
   print "\n";
   print "Where:\n";
   print "  --jobs RANGES Only check the jobs in one of the specified comma-separated\n";
-  print "                job id ranges. Each range is either a single job id, or of the\n";
-  print "                form 'FIRST..LAST' where FIRST and LAST are either the empty\n";
-  print "                string or a job id.\n";
+  print "                job id ranges. This also causes patches not related to one of\n";
+  print "                these jobs to be ignored. Each range is either a single job id,\n";
+  print "                or of the form 'FIRST..LAST' where FIRST and LAST are either the\n";
+  print "                empty string or a job id.\n";
   print "  --help        Shows this usage message.\n";
   exit 0;
 }
@@ -318,9 +320,12 @@ sub LoadTestInfo($)
   return undef if (!$HasTestInfo);
 
   # Fill in some useful defaults
-  SetDefault($TestInfo, "job", "Status", "completed");
-  SetDefault($TestInfo, "tasks", "Status", "completed");
-  SetDefault($TestInfo, "tasks", "HasTask", 1);
+  if (!defined $TestInfo->{patch}->{Disposition})
+  {
+    SetDefault($TestInfo, "job", "Status", "completed");
+    SetDefault($TestInfo, "tasks", "Status", "completed");
+    SetDefault($TestInfo, "tasks", "HasTask", 1);
+  }
 
   # Then propagate the defaults
   foreach my $Pair (["tasks", ["build", "tests"]],
@@ -666,6 +671,10 @@ sub CheckJob($$)
   {
     is($Job->Status, $JobInfo->{Status}, "Check Status for job ". $Job->Id);
   }
+  elsif (CheckValue($TestInfo->{patch}->{Disposition}))
+  {
+    fail("The '$TestInfo->{patch}->{Disposition}' patch disposition is incompatible with job ". $Job->Id);
+  }
 }
 
 sub IsJobInRange($)
@@ -681,6 +690,8 @@ sub IsJobInRange($)
   return 0;
 }
 
+my %CheckedJobs;
+
 =pod
 
 =item <tasks.HasTask>
@@ -705,12 +716,15 @@ p win.TestUnits cabinet:extract cabinet:fdi
 
 =cut
 
-sub CheckJobTree($)
+sub CheckJobTree($;$)
 {
-  my ($Job) = @_;
+  my ($Job, $TestInfo) = @_;
   return if (!IsJobInRange($Job));
 
-  my ($TestInfo, $HasTask);
+  return if ($CheckedJobs{$Job->Id});
+  $CheckedJobs{$Job->Id} = 1;
+
+  my $HasTask;
   my $TestUnits = { wine => {} };
 
   my $Steps = $Job->Steps;
@@ -783,6 +797,85 @@ sub CheckJobs()
 }
 
 
+#
+# Verify the Patches and the patches site
+#
+
+=pod
+
+=item <patch.Disposition>
+
+Checks the patch's disposition.
+By default the disposition is supposed to point to the relevant job, in which
+case default values are provided for the job.Status, tasks.Status and
+tasks.HasTask properties. If a specific disposition is specified, then these
+defaults are not provided since no job should be created in that case.
+
+For instance:
+p patch.Disposition Does not impact the Wine build
+
+=cut
+
+sub CheckPatch($$$)
+{
+  my ($Jobs, $Patch, $TestInfo) = @_;
+
+  my $Job;
+  if ($Patch->Disposition =~ /^Submitted job (\d+)$/)
+  {
+    $Job = $Jobs->GetItem($1);
+  }
+  # The job range restriction extends to their associated patches...
+  return if ($Job and !IsJobInRange($Job));
+  # ..and to patches with no job
+  return if (!$Job and @JobRanges);
+
+  my $PatchInfo = $TestInfo->{patch};
+  if (CheckValue($PatchInfo->{Disposition}))
+  {
+    is($Patch->Disposition, $PatchInfo->{Disposition}, "Check Disposition for patch ". $Patch->Id .": ". $Patch->Subject);
+  }
+  elsif ($Patch->Disposition =~ /^Submitted job (.*)$/)
+  {
+    isnt($Job, undef, "Check job $1 for patch ". $Patch->Id .": ". $Patch->Subject);
+    if (!$Job or $Job->Status eq "canceled")
+    {
+      ; # Nothing to check
+    }
+    elsif ($Job->Status =~ /^(?:queued|running)$/)
+    {
+      print "...skipping job ". $Job->Id ." because it is not complete yet (". $Job->Status .")\n";
+    }
+    else
+    {
+      # Provide defaults for the job checks
+      my $Remarks = $Patch->Subject;
+      $Remarks =~ s~^\[\d+/\d+\]\s~~; # In case submitted as a patch series
+      $Remarks =~ s~\s+$~~;
+      SetDefault($TestInfo, "job", "Remarks", $Remarks);
+      CheckJobTree($Job, $TestInfo);
+    }
+  }
+  else
+  {
+    fail("Patch ". $Patch->Id ."(". $Patch->Subject .") should have an associated job but Disposition is: ". $Patch->Disposition);
+  }
+}
+
+sub CheckPatches()
+{
+  my $Jobs = CreateJobs();
+  my $Patches = CreatePatches();
+  foreach my $Patch (sort { $b->Id <=> $a->Id } @{$Patches->GetItems()})
+  {
+    my $PatchFileName = "$DataDir/patches/". $Patch->Id;
+    next if (!-f $PatchFileName);
+    my $TestInfo = LoadTestInfo($PatchFileName);
+    CheckPatch($Jobs, $Patch, $TestInfo) if ($TestInfo);
+  }
+}
+
+
 #
 # Run the tests
 #
@@ -799,6 +892,10 @@ if (!$HasBaseVM->{build})
   delete $HasBaseVM->{win64};
 }
 
+print "***** Checking Patches *****\n";
+CheckPatches();
+
+print "\n***** Checking Jobs *****\n";
 CheckJobs();
 
 done_testing();