Francois Gouget : testbot: Create a single Job per patch even for multiple modules.
Alexandre Julliard
julliard at winehq.org
Wed Jun 13 14:00:26 CDT 2018
Module: tools
Branch: master
Commit: 6fbd57b4f50c5a39e394dfbf0f97662e8a15a226
URL: https://source.winehq.org/git/tools.git/?a=commit;h=6fbd57b4f50c5a39e394dfbf0f97662e8a15a226
Author: Francois Gouget <fgouget at codeweavers.com>
Date: Wed Jun 13 10:23:42 2018 +0200
testbot: Create a single Job per patch even for multiple modules.
So far a build task could only provide the 32 and 64 bit test
executables for a single dll or program. Also, because of the lack of
proper dependency support between steps it was impossible to have
multiple build steps. So the TestBot used to create one job per module.
However the patch status site can only keep track of one job per patch.
This means some failures could be missed if they did not happen in the
one job tracked by the patch status site.
Now that Build.pl rebuilds all the impacted test executables the
Testbot systematically creates a single job and a single build task and
retrieves all the test executables in one go. This also has the
advantage of reducing the number of VM reverts.
Another side-effect of this change is that if the patch modifies the
tests of multiple modules and the compilation of one them fails, no
test will be run. Before only the tests impacted by the failed
compilation would have been skipped. However a compilation error is
grounds for resubmitting the patch, at which time the tests will be
rerun anyway.
Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard at winehq.org>
---
testbot/bin/WineRunBuild.pl | 65 ++++++-----------
testbot/lib/WineTestBot/Patches.pm | 143 +++++++++++++++++--------------------
2 files changed, 85 insertions(+), 123 deletions(-)
diff --git a/testbot/bin/WineRunBuild.pl b/testbot/bin/WineRunBuild.pl
index 12e34d5..61deb50 100755
--- a/testbot/bin/WineRunBuild.pl
+++ b/testbot/bin/WineRunBuild.pl
@@ -43,6 +43,7 @@ $Name0 =~ s+^.*/++;
use WineTestBot::Config;
use WineTestBot::Jobs;
+use WineTestBot::PatchUtils;
use WineTestBot::VMs;
use WineTestBot::Log;
use WineTestBot::Engine::Notify;
@@ -323,31 +324,15 @@ elsif ($Debug and !$VM->GetDomain()->IsPoweredOn())
# Figure out what to build
#
-my ($Run64, $BaseName);
-foreach my $OtherStep (@{$Job->Steps->GetItems()})
+my (%Bitnesses, %TestExes);
+foreach my $TestStep (@{$Job->Steps->GetItems()})
{
- next if ($OtherStep->No == $StepNo);
-
- $Run64 = 1 if ($OtherStep->FileType eq "exe64");
- my $OtherFileName = $OtherStep->FileName;
- if ($OtherFileName =~ m/^([\w_.]+)_test(?:64)?\.exe$/)
+ if ($TestStep->FileType =~ /^exe([0-9]+)$/)
{
- my $OtherBaseName = $1;
- if ($Step->FileType eq "patchprograms")
- {
- $OtherBaseName =~ s/\.exe$//;
- }
- if (defined $BaseName and $BaseName ne $OtherBaseName)
- {
- FatalError("$OtherBaseName doesn't match previously found $BaseName\n");
- }
- $BaseName = $OtherBaseName;
+ $Bitnesses{$1} = 1;
+ $TestExes{$TestStep->FileName} = 1;
}
}
-if (!defined $BaseName)
-{
- FatalError("Could not determine the test executable's base name\n");
-}
#
@@ -363,9 +348,8 @@ if (!$TA->SendFile($FileName, "staging/patch.diff", 0))
}
my $Script = "#!/bin/sh\n" .
"rm -f Build.log\n" .
- "../bin/build/Build.pl patch.diff 32";
-$Script .= ",64"if ($Run64);
-$Script .= " >>Build.log 2>&1\n";
+ "../bin/build/Build.pl patch.diff ". join(",", keys %Bitnesses) .
+ " >>Build.log 2>&1\n";
Debug(Elapsed($Start), " Sending the script: [$Script]\n");
if (!$TA->SendFileFromString($Script, "task", $TestAgent::SENDFILE_EXE))
{
@@ -456,34 +440,25 @@ FatalTAError(undef, $TAError) if (defined $TAError);
# Grab the executables for the next steps
#
-# Don't try copying the test executables if the build step failed
-if ($NewStatus eq "completed")
+my $Impacts = GetPatchImpact($FileName, "nounit");
+my $StepDir = $Step->CreateDir();
+foreach my $TestInfo (values %{$Impacts->{Tests}})
{
- my $StepDir = $Step->CreateDir();
- foreach my $OtherStep (@{$Job->Steps->GetItems()})
+ foreach my $Bits ("", "64")
{
- next if ($OtherStep->No == $StepNo);
-
- my $OtherFileName = $OtherStep->FileName;
- next if ($OtherFileName !~ /^[\w_.]+_test(?:64)?\.exe$/);
+ my $Local = "$TestInfo->{ExeBase}$Bits.exe";
+ next if (!$TestExes{$Local});
- my $Bits = $OtherStep->FileType eq "exe64" ? "64" : "32";
- my $TestExecutable;
- if ($Step->FileType ne "patchprograms")
- {
- $TestExecutable = "build-mingw$Bits/dlls/$BaseName/tests/${BaseName}_test.exe";
- }
- else
+ my $Remote = "build-mingw". ($Bits || "32") ."/$TestInfo->{Path}/$TestInfo->{ExeBase}.exe";
+ Debug(Elapsed($Start), " Retrieving '$Local'\n");
+ if ($TA->GetFile($Remote, "$StepDir/$Local"))
{
- $TestExecutable = "build-mingw$Bits/programs/$BaseName/tests/${BaseName}.exe_test.exe";
+ chmod 0664, "$StepDir/$Local";
}
-
- Debug(Elapsed($Start), " Retrieving '$OtherFileName'\n");
- if (!$TA->GetFile($TestExecutable, "$StepDir/$OtherFileName"))
+ elsif ($TA->GetLastError() !~ /: No such file or directory/)
{
- FatalTAError($TA, "Could not retrieve '$OtherFileName'");
+ FatalTAError($TA, "Could not retrieve '$Local'");
}
- chmod 0664, "$StepDir/$OtherFileName";
}
}
$TA->Disconnect();
diff --git a/testbot/lib/WineTestBot/Patches.pm b/testbot/lib/WineTestBot/Patches.pm
index e349dfe..258aa2c 100644
--- a/testbot/lib/WineTestBot/Patches.pm
+++ b/testbot/lib/WineTestBot/Patches.pm
@@ -157,59 +157,55 @@ sub Submit($$$)
$User = GetBatchUser();
}
- my $Disposition = "Submitted job ";
- my $First = 1;
- foreach my $Module (sort keys %{$Impacts->{Tests}})
- {
- my $TestInfo = $Impacts->{Tests}->{$Module};
- my $Jobs = CreateJobs();
-
- # Create a new job for this patch
- my $NewJob = $Jobs->Add();
- $NewJob->User($User);
- $NewJob->Priority(6);
- my $PropertyDescriptor = $Jobs->GetPropertyDescriptorByName("Remarks");
- my $Subject = $self->Subject;
- $Subject =~ s/\[PATCH[^\]]*]//i;
- $Subject =~ s/[[\(]?\d+\/\d+[\)\]]?//;
- $Subject =~ s/^\s*//;
- $NewJob->Remarks(substr("[wine-patches] " . $Subject, 0,
- $PropertyDescriptor->GetMaxLength()));
- $NewJob->Patch($self);
+ # Create a new job for this patch
+ my $Jobs = CreateJobs();
+ my $NewJob = $Jobs->Add();
+ $NewJob->User($User);
+ $NewJob->Priority(6);
+ my $PropertyDescriptor = $Jobs->GetPropertyDescriptorByName("Remarks");
+ my $Subject = $self->Subject;
+ $Subject =~ s/\[PATCH[^\]]*]//i;
+ $Subject =~ s/[[\(]?\d+\/\d+[\)\]]?//;
+ $Subject =~ s/^\s*//;
+ $NewJob->Remarks(substr("[wine-patches] " . $Subject, 0,
+ $PropertyDescriptor->GetMaxLength()));
+ $NewJob->Patch($self);
+
+ # Add build step to the job
+ my $BuildStep = $NewJob->Steps->Add();
+ $BuildStep->FileName("patch.diff");
+ $BuildStep->FileType("patchdlls"); # This is irrelevant now
+ $BuildStep->InStaging(!1);
+ $BuildStep->Type("build");
+ $BuildStep->DebugLevel(0);
- # Add build step to the job
- my $Steps = $NewJob->Steps;
- my $NewStep = $Steps->Add();
- $NewStep->FileName("patch.diff");
- $NewStep->FileType($TestInfo->{Type});
- $NewStep->InStaging(!1);
- $NewStep->Type("build");
- $NewStep->DebugLevel(0);
-
- # Add build task
- my $VMs = CreateVMs();
- $VMs->AddFilter("Type", ["build"]);
- $VMs->AddFilter("Role", ["base"]);
- my $BuildVM = ${$VMs->GetItems()}[0];
- my $Task = $NewStep->Tasks->Add();
- $Task->VM($BuildVM);
- $Task->Timeout($BuildTimeout);
-
- # Save the build step so other steps can reference it
- my ($ErrKey, $ErrProperty, $ErrMessage) = $Jobs->Save();
- if (defined($ErrMessage))
- {
- $self->Disposition("Failed to submit build step");
- return $ErrMessage;
- }
+ # Add build task
+ my $VMs = CreateVMs();
+ $VMs->AddFilter("Type", ["build"]);
+ $VMs->AddFilter("Role", ["base"]);
+ my $BuildVM = ${$VMs->GetItems()}[0];
+ my $Task = $BuildStep->Tasks->Add();
+ $Task->VM($BuildVM);
+ $Task->Timeout($BuildTimeout);
+
+ # Save the build step so the others can reference it.
+ my ($ErrKey, $ErrProperty, $ErrMessage) = $Jobs->Save();
+ if (defined($ErrMessage))
+ {
+ $self->Disposition("Failed to submit build step");
+ return $ErrMessage;
+ }
- # Stage the patch so it can be picked up by the job
- if (!link($PatchFileName, "$DataDir/staging/job". $NewJob->Id ."_patch.diff"))
- {
- $self->Disposition("Failed to prepare patch file");
- return $!;
- }
+ # Stage the patch so it can be picked up by the job
+ if (!link($PatchFileName, "$DataDir/staging/job". $NewJob->Id ."_patch.diff"))
+ {
+ $self->Disposition("Failed to stage the patch file");
+ return $!;
+ }
+ foreach my $Module (sort keys %{$Impacts->{Tests}})
+ {
+ my $TestInfo = $Impacts->{Tests}->{$Module};
foreach my $Unit (sort keys %{$TestInfo->{Units}})
{
# Add 32 and 64-bit tasks
@@ -221,8 +217,8 @@ sub Submit($$$)
if (@{$VMs->GetKeys()})
{
# Create the corresponding Step
- my $NewStep = $Steps->Add();
- $NewStep->PreviousNo(1);
+ my $NewStep = $NewJob->Steps->Add();
+ $NewStep->PreviousNo($BuildStep->No);
my $FileName = $TestInfo->{ExeBase};
$FileName .= "64" if ($Bits eq "64");
$NewStep->FileName("$FileName.exe");
@@ -243,35 +239,26 @@ sub Submit($$$)
}
}
}
+ }
- # Save it all
- ($ErrKey, $ErrProperty, $ErrMessage) = $Jobs->Save();
- if (defined $ErrMessage)
- {
- $self->Disposition("Failed to submit job");
- return $ErrMessage;
- }
-
- # Switch Status to staging to indicate we are done setting up the job
- $NewJob->Status("staging");
- ($ErrKey, $ErrProperty, $ErrMessage) = $Jobs->Save();
- if (defined $ErrMessage)
- {
- $self->Disposition("Failed to submit job (staging)");
- return $ErrMessage;
- }
+ # Save it all
+ ($ErrKey, $ErrProperty, $ErrMessage) = $Jobs->Save();
+ if (defined $ErrMessage)
+ {
+ $self->Disposition("Failed to submit job");
+ return $ErrMessage;
+ }
- if ($First)
- {
- $First = !1;
- }
- else
- {
- $Disposition .= ", ";
- }
- $Disposition .= $NewJob->Id;
+ # Switch Status to staging to indicate we are done setting up the job
+ $NewJob->Status("staging");
+ ($ErrKey, $ErrProperty, $ErrMessage) = $Jobs->Save();
+ if (defined $ErrMessage)
+ {
+ $self->Disposition("Failed to submit job (staging)");
+ return $ErrMessage;
}
- $self->Disposition($Disposition);
+
+ $self->Disposition("Submitted job ". $NewJob->Id);
return undef;
}
More information about the wine-cvs
mailing list