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@codeweavers.com --- 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 12e34d5c7..61deb50a6 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 e349dfe88..258aa2c19 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; }