There is no need to allow for a full rebuild if the patch only touches a few modules. This is particularly significant for Wine builds since they take longer. Reducing the timeouts ensures the TestBot will not be blocked longer than necessary should a task get stuck either due to an issue in the patch, or to a network issue. This also centralizes all build timeout calcultations in GetBuildTimeout(). Finally this slightly changes a couple of timeouts in the scheduler and activity modules because the $BuildTimeout and $ReconfigTimeout parameters are going away.
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/bin/CheckForWinetestUpdate.pl | 16 ++++-- testbot/lib/WineTestBot/Activity.pm | 2 +- testbot/lib/WineTestBot/Config.pm | 53 ++++++++++-------- testbot/lib/WineTestBot/Engine/Scheduler.pm | 2 +- testbot/lib/WineTestBot/PatchUtils.pm | 62 +++++++++++++++++---- testbot/lib/WineTestBot/Patches.pm | 19 ++++--- testbot/web/Submit.pl | 20 ++++++- 7 files changed, 122 insertions(+), 52 deletions(-)
diff --git a/testbot/bin/CheckForWinetestUpdate.pl b/testbot/bin/CheckForWinetestUpdate.pl index a7d77f02f..991cdeb0f 100755 --- a/testbot/bin/CheckForWinetestUpdate.pl +++ b/testbot/bin/CheckForWinetestUpdate.pl @@ -50,12 +50,13 @@ use HTTP::Response; use HTTP::Status;
use WineTestBot::Config; +use WineTestBot::Engine::Notify; use WineTestBot::Jobs; -use WineTestBot::Users; use WineTestBot::Log; +use WineTestBot::PatchUtils; +use WineTestBot::Users; use WineTestBot::Utils; use WineTestBot::VMs; -use WineTestBot::Engine::Notify;
my %WineTestUrls = ( @@ -245,6 +246,9 @@ sub AddJob($$$) return 1; }
+my @ExeBuilds = qw(exe32 exe64); +my @WineBuilds = qw(win32 wow32 wow64); + sub AddReconfigJob($) { my ($VMType) = @_; @@ -283,9 +287,9 @@ sub AddReconfigJob($) Debug(" $VMKey $VMType reconfig\n"); my $Task = $BuildStep->Tasks->Add(); $Task->VM($VM); - $Task->Timeout($VMType eq "wine" ? - 3 * $WineReconfigTimeout : # 3 full Wine builds - $ReconfigTimeout); # 1 overall timeout + my $Builds; + map { $Builds->{$_} = 1 } ($VMType eq "wine" ? @WineBuilds : @ExeBuilds); + $Task->Timeout(GetBuildTimeout(undef, $Builds)); }
# Save the build step so the others can reference it. @@ -299,7 +303,7 @@ sub AddReconfigJob($) if ($VMType eq "wine") { # Add steps to run WineTest on Wine - foreach my $Build ("win32", "wow32", "wow64") + foreach my $Build (@WineBuilds) { # Add a step to the job my $NewStep = $Steps->Add(); diff --git a/testbot/lib/WineTestBot/Activity.pm b/testbot/lib/WineTestBot/Activity.pm index a8885a4e1..eba45a039 100644 --- a/testbot/lib/WineTestBot/Activity.pm +++ b/testbot/lib/WineTestBot/Activity.pm @@ -438,7 +438,7 @@ sub GetStatistics($;$) # Of course this only works for statistics about VM operations and running # tasks (so running.time, reverting.time, etc) not for those about idle or # off VMs (idle.time, etc.). - $ActivitySeconds = $Seconds + 60 + max($SuiteTimeout, $ReconfigTimeout); + $ActivitySeconds = $Seconds + 60 + max($SuiteTimeout, 2 * $WineBuildTimeout); } my ($Activity, $Counters) = GetActivity($VMs, $ActivitySeconds); $GlobalStats->{"recordgroups.count"} = $Counters->{recordgroups}; diff --git a/testbot/lib/WineTestBot/Config.pm b/testbot/lib/WineTestBot/Config.pm index 1ffff262f..a83085d44 100644 --- a/testbot/lib/WineTestBot/Config.pm +++ b/testbot/lib/WineTestBot/Config.pm @@ -30,10 +30,11 @@ use vars qw (@ISA @EXPORT @EXPORT_OK $UseSSL $LogDir $DataDir $BinDir $MaxRevertsWhileRunningVMs $MaxActiveVMs $MaxRunningVMs $MaxVMsWhenIdle $SleepAfterRevert $WaitForToolsInVM $VMToolTimeout $MaxVMErrors $MaxTaskTries $AdminEMail $RobotEMail - $WinePatchToOverride $WinePatchCc $SuiteTimeout $SingleTimeout - $BuildTimeout $ReconfigTimeout $WineReconfigTimeout $TimeoutMargin - $TagPrefix - $MaxUnitSize $ProjectName $PatchesMailingList $LDAPServer + $WinePatchToOverride $WinePatchCc + $ExeBuildNativeTimeout $ExeBuildTestTimeout $ExeModuleTimeout + $WineBuildTimeout $WineModuleTimeout $TimeoutMargin + $SuiteTimeout $SingleTimeout $MaxUnitSize + $TagPrefix $ProjectName $PatchesMailingList $LDAPServer $LDAPBindDN $LDAPSearchBase $LDAPSearchFilter $LDAPRealNameAttribute $LDAPEMailAttribute $AgentPort $Tunnel $TunnelDefaults $PrettyHostNames $JobPurgeDays @@ -46,9 +47,10 @@ require Exporter; $MaxRunningVMs $MaxVMsWhenIdle $SleepAfterRevert $WaitForToolsInVM $VMToolTimeout $MaxVMErrors $MaxTaskTries $AdminEMail $RobotEMail $WinePatchToOverride $WinePatchCc $SuiteTimeout - $SingleTimeout $BuildTimeout $ReconfigTimeout $WineReconfigTimeout - $TimeoutMargin - $TagPrefix $MaxUnitSize $ProjectName $PatchesMailingList + $ExeBuildNativeTimeout $ExeBuildTestTimeout $ExeModuleTimeout + $WineBuildTimeout $WineModuleTimeout $TimeoutMargin + $SuiteTimeout $SingleTimeout $MaxUnitSize + $TagPrefix $ProjectName $PatchesMailingList $LDAPServer $LDAPBindDN $LDAPSearchBase $LDAPSearchFilter $LDAPRealNameAttribute $LDAPEMailAttribute $AgentPort $Tunnel $TunnelDefaults $PrettyHostNames $JobPurgeDays @@ -90,24 +92,29 @@ $MaxVMErrors = 3; # How many times to run a test that fails before giving up. $MaxTaskTries = 3;
-# How long to let a test suite run before forcibly shutting it down -# (in seconds). -$SuiteTimeout = 30 * 60; -# How long to let a regular test run before forcibly shutting it down -# (in seconds). -$SingleTimeout = 2 * 60; -# How long to let a regular build run before forcibly shutting it down -# (in seconds). -$BuildTimeout = 5 * 60; -# How long to let a reconfig task run before forcibly shutting it down -# (in seconds). Note that this includes building the native Wine build tools, -# and the 32 and 64 bit test executables. -$ReconfigTimeout = (1 + 2 * 5) * 60; -# How long to let a full Wine recompilation run before forcibly shutting it -# down (in seconds). -$WineReconfigTimeout = 20 * 60; +# Exe build timeouts (in seconds) +# - For a full build +$ExeBuildNativeTimeout = 60; +$ExeBuildTestTimeout = 2 * 60; +# - For a single module +$ExeModuleTimeout = 30; + +# Wine build timeouts (in seconds) +# - For a full build +$WineBuildTimeout = 20 * 60; +# - For a single module +$WineModuleTimeout = 60; + # How much to add to the task timeout to account for file transfers, etc. +# (in seconds) $TimeoutMargin = 2 * 60; + +# Test timeouts (in seconds) +# - For the whole test suite +$SuiteTimeout = 30 * 60; +# - For a single test +$SingleTimeout = 2 * 60; + # Maximum amount of traces for a test unit. $MaxUnitSize = 32 * 1024;
diff --git a/testbot/lib/WineTestBot/Engine/Scheduler.pm b/testbot/lib/WineTestBot/Engine/Scheduler.pm index 4c49f5a1a..ffcc3cdbd 100644 --- a/testbot/lib/WineTestBot/Engine/Scheduler.pm +++ b/testbot/lib/WineTestBot/Engine/Scheduler.pm @@ -640,7 +640,7 @@ sub _ScheduleTasks($) }
# It's not worth preparing the next step for tasks that take so long - $StepVMs{$Step} = undef if ($Task->Timeout > $BuildTimeout); + $StepVMs{$Step} = undef if ($Task->Timeout > $VMToolTimeout);
my $VMKey = $VM->GetKey(); if ($VM->Status eq "idle") diff --git a/testbot/lib/WineTestBot/PatchUtils.pm b/testbot/lib/WineTestBot/PatchUtils.pm index d399ced54..7049694aa 100644 --- a/testbot/lib/WineTestBot/PatchUtils.pm +++ b/testbot/lib/WineTestBot/PatchUtils.pm @@ -31,7 +31,9 @@ the Wine builds. =cut
use Exporter 'import'; -our @EXPORT = qw(GetPatchImpact UpdateWineData); +our @EXPORT = qw(GetPatchImpact UpdateWineData GetBuildTimeout); + +use List::Util qw(min max);
use WineTestBot::Config;
@@ -166,11 +168,12 @@ sub _HandleFile($$$) if ($FilePath =~ m~^(dlls|programs)/([^/]+)/tests/([^/\s]+)$~) { my ($Root, $Dir, $File) = ($1, $2, $3); - $Impacts->{IsWinePatch} = 1; + my $Module = ($Root eq "programs") ? "$Dir.exe" : $Dir; + $Impacts->{BuildModules}->{$Module} = 1; $Impacts->{TestBuild} = 1; + $Impacts->{IsWinePatch} = 1;
my $Tests = $Impacts->{Tests}; - my $Module = ($Root eq "programs") ? "$Dir.exe" : $Dir; if (!$Tests->{$Module}) { $Tests->{$Module} = { @@ -205,8 +208,9 @@ sub _HandleFile($$$) { my ($Root, $Dir, $File) = ($1, $2, $3); my $Module = ($Root eq "programs") ? "$Dir.exe" : $Dir; - $Impacts->{IsWinePatch} = 1; + $Impacts->{BuildModules}->{$Module} = 1; $Impacts->{ModuleBuild} = 1; + $Impacts->{IsWinePatch} = 1;
if ($File eq "Makefile.in" and $Change ne "modify") { @@ -263,6 +267,11 @@ sub GetPatchImpact($;$$)
my $Impacts = { NoUnits => $NoUnits, + # Number of patched test units. + UnitCount => 0, + # The modules that need a rebuild, even if only for the tests. + BuildModules => {}, + # Information about 'tests' directories. Tests => {}, }; _LoadWineFiles(); @@ -277,6 +286,9 @@ sub GetPatchImpact($;$$) map { $Impacts->{WineFiles}->{$_} = 1 } keys %{$WineFiles}; map { $Impacts->{WineFiles}->{$_} = 1 } keys %{$PastImpacts->{NewFiles}}; map { delete $Impacts->{WineFiles}->{$_} } keys %{$PastImpacts->{DeletedFiles}}; + # Modules impacted by previous parts of a patchset still need to be + # rebuilt. + $Impacts->{BuildModules} = { %{$PastImpacts->{BuildModules}} }; } else { @@ -341,8 +353,6 @@ sub GetPatchImpact($;$$) } close($fh);
- $Impacts->{ModuleCount} = 0; - $Impacts->{UnitCount} = 0; foreach my $TestInfo (values %{$Impacts->{Tests}}) { # For each module, identify modifications to non-C files and helper dlls @@ -390,14 +400,44 @@ sub GetPatchImpact($;$$) }
$TestInfo->{UnitCount} = scalar(keys %{$TestInfo->{Units}}); - if ($TestInfo->{UnitCount}) - { - $Impacts->{ModuleCount}++; - $Impacts->{UnitCount} += $TestInfo->{UnitCount}; - } + $Impacts->{UnitCount} += $TestInfo->{UnitCount}; }
return $Impacts; }
+ +# +# Compute task timeouts based on the patch data +# + +sub GetBuildTimeout($$) +{ + my ($Impacts, $Builds) = @_; + + my ($ExeCount, $WineCount); + map {$_ =~ /^exe/ ? $ExeCount++ : $WineCount++ } keys %$Builds; + + # Set $ModuleCount to 0 if a full rebuild is needed + my $ModuleCount = (!$Impacts or $Impacts->{WineBuild}) ? 0 : + scalar(keys %{$Impacts->{BuildModules}}); + + my ($ExeTimeout, $WineTimeout) = (0, 0); + if ($ExeCount) + { + my $OneBuild = $ModuleCount ? $ModuleCount * $ExeModuleTimeout : + $ExeBuildTestTimeout; + $ExeTimeout = ($ModuleCount ? 0 : $ExeBuildNativeTimeout) + + $ExeCount * min($ExeBuildTestTimeout, $OneBuild); + } + if ($WineCount) + { + my $OneBuild = $ModuleCount ? $ModuleCount * $WineModuleTimeout : + $WineBuildTimeout; + $WineTimeout = $WineCount * min($WineBuildTimeout, $OneBuild); + } + + return $ExeTimeout + $WineTimeout; +} + 1; diff --git a/testbot/lib/WineTestBot/Patches.pm b/testbot/lib/WineTestBot/Patches.pm index f5ef431eb..390b1611c 100644 --- a/testbot/lib/WineTestBot/Patches.pm +++ b/testbot/lib/WineTestBot/Patches.pm @@ -185,12 +185,6 @@ sub Submit($$$) $BuildStep->Type("build"); $BuildStep->DebugLevel(0);
- # Add build task - my $BuildVM = ${$BuildVMs->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)) @@ -200,6 +194,7 @@ sub Submit($$$) }
# Create steps for the Windows tests + my $Builds; foreach my $Module (sort keys %{$Impacts->{Tests}}) { my $TestInfo = $Impacts->{Tests}->{$Module}; @@ -219,6 +214,7 @@ sub Submit($$$) $FileName .= "64" if ($Bits eq "64"); $NewStep->FileName("$FileName.exe"); $NewStep->FileType("exe$Bits"); + $Builds->{"exe$Bits"} = 1;
# And a task for each VM my $Tasks = $NewStep->Tasks; @@ -235,6 +231,12 @@ sub Submit($$$) } } } + + # Add the build task + my $BuildVM = ${$BuildVMs->GetItems()}[0]; + my $BuildTask = $BuildStep->Tasks->Add(); + $BuildTask->VM($BuildVM); + $BuildTask->Timeout(GetBuildTimeout($Impacts, $Builds)); }
my $WineVMs = CreateVMs(); @@ -258,8 +260,9 @@ sub Submit($$$) my $Task = $Tasks->Add(); $Task->VM($VM); # Only verify that the win32 version compiles - $Task->Timeout($WineReconfigTimeout); - $Task->CmdLineArg("win32"); + my $Builds = { "win32" => 1 }; + $Task->Timeout(GetBuildTimeout($Impacts, $Builds)); + $Task->CmdLineArg(join(",", keys %$Builds)); } }
diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl index 440bd8e5b..e755b5758 100644 --- a/testbot/web/Submit.pl +++ b/testbot/web/Submit.pl @@ -777,6 +777,13 @@ sub OnSubmit($)
# Add steps and tasks for the 32 and 64-bit tests my $FileType = $self->GetParam("FileType"); + my $Impacts; + if ($FileType eq "patch") + { + my $TmpStagingFullPath = $self->GetTmpStagingFullPath($BaseName); + $Impacts = GetPatchImpact($TmpStagingFullPath); + } + my $BuildStep; foreach my $Bits ("32", "64") { @@ -812,7 +819,10 @@ sub OnSubmit($) my $BuildVM = ${$VMs->GetItems()}[0]; my $Task = $BuildStep->Tasks->Add(); $Task->VM($BuildVM); - $Task->Timeout($BuildTimeout); + + my $Builds = { "exe32" => 1 }; + $Builds->{"exe64"} = 1 if (defined $self->GetParam("Run64"); + $Task->Timeout(GetBuildTimeout($Impacts, $Builds));
# Save the build step so the others can reference it my ($ErrKey, $ErrProperty, $ErrMessage) = $Jobs->Save(); @@ -861,6 +871,7 @@ sub OnSubmit($) { next if ($Build eq "wow64" and !defined($self->GetParam("Run64")));
+ my $Timeout; foreach my $VMKey (@$SortedKeys) { my $VM = $VMs->GetItem($VMKey); @@ -878,12 +889,17 @@ sub OnSubmit($) $WineStep->ReportSuccessfulTests(defined($self->GetParam("ReportSuccessfulTests"))); $Tasks = $WineStep->Tasks; } + if (!defined $Timeout) + { + my $Builds = { $Build => 1 }; + $Timeout = GetBuildTimeout($Impacts, $Builds); + }
# Then add a task for this VM my $Task = $Tasks->Add(); $Task->VM($VM); $Task->CmdLineArg($Build); - $Task->Timeout($WineReconfigTimeout); + $Task->Timeout($Timeout); } } }