Module: tools Branch: master Commit: 73c62ab7dc4bafb83bfc676b018b5db55e5edd3a URL: http://source.winehq.org/git/tools.git/?a=commit;h=73c62ab7dc4bafb83bfc676b0...
Author: Francois Gouget fgouget@codeweavers.com Date: Fri Nov 2 00:11:12 2012 +0100
testbot/lib: Turn the Jobs::Schedule() and Jobs::Check() methods into functions.
Their typical usage pattern shows there is no reason for them to be methods. This even more true for Check() since it modifies the object it is called on.
---
testbot/bin/Engine.pl | 15 ++++------- testbot/lib/WineTestBot/Jobs.pm | 46 ++++++++++++++++------------------- testbot/lib/WineTestBot/Patches.pm | 6 ++-- 3 files changed, 30 insertions(+), 37 deletions(-)
diff --git a/testbot/bin/Engine.pl b/testbot/bin/Engine.pl index 7bd14a1..1cc8a9b 100755 --- a/testbot/bin/Engine.pl +++ b/testbot/bin/Engine.pl @@ -81,7 +81,7 @@ sub HandleJobSubmit } }
- $ErrMessage = $Jobs->Schedule(); + $ErrMessage = ScheduleJobs(); if (defined($ErrMessage)) { LogMsg "Scheduling problem in HandleJobSubmit: $ErrMessage\n"; @@ -158,7 +158,7 @@ sub HandleJobCancel
sub HandleTaskComplete { - my $ErrMessage = CreateJobs()->Schedule(); + my $ErrMessage = ScheduleJobs(); if (defined($ErrMessage)) { LogMsg "Scheduling problem in HandleTaskComplete: $ErrMessage\n"; @@ -180,7 +180,7 @@ sub HandleVMStatusChange if ($OldStatus eq "reverting" || $OldStatus eq "running" || $NewStatus eq "idle" || $NewStatus eq "dirty") { - my $ErrMessage = CreateJobs()->Schedule(); + my $ErrMessage = ScheduleJobs(); if (defined($ErrMessage)) { LogMsg "Scheduling problem in HandleVMStatusChange: $ErrMessage\n"; @@ -266,7 +266,7 @@ sub HandleFoundWinetestUpdate DeleteEvent("GiveUpOnWinetestUpdate"); }
- my $ErrMessage = CreateJobs()->Schedule(); + my $ErrMessage = ScheduleJobs(); if (defined($ErrMessage)) { LogMsg "Scheduling problem in HandleFoundWinetestUpdate: $ErrMessage\n"; @@ -511,11 +511,8 @@ checks whether any pending patchsets are now complete and thus can be scheduled.
sub SafetyNet { - my $Jobs = CreateJobs(); - $Jobs->Check(); - $Jobs = undef; - $Jobs = CreateJobs(); - $Jobs->Schedule(); + CheckJobs(); + ScheduleJobs();
if (opendir STAGING, "$DataDir/staging") { diff --git a/testbot/lib/WineTestBot/Jobs.pm b/testbot/lib/WineTestBot/Jobs.pm index 649f3ca..74547d2 100644 --- a/testbot/lib/WineTestBot/Jobs.pm +++ b/testbot/lib/WineTestBot/Jobs.pm @@ -277,7 +277,7 @@ use vars qw(@ISA @EXPORT @PropertyDescriptors);
require Exporter; @ISA = qw(WineTestBot::WineTestBotCollection Exporter); -@EXPORT = qw(&CreateJobs); +@EXPORT = qw(&CreateJobs &ScheduleJobs &CheckJobs);
my @PropertyDescriptors;
@@ -374,10 +374,9 @@ VM. =back =cut
-sub ScheduleOnHost +sub ScheduleOnHost($$) { - my $self = shift; - my $Hypervisors = $_[0]; + my ($SortedJobs, $Hypervisors) = @_;
my $HostVMs = CreateVMs(); $HostVMs->FilterHypervisor($Hypervisors); @@ -385,11 +384,8 @@ sub ScheduleOnHost my $PoweredOnNonBaseVMs = $HostVMs->CountPoweredOnNonBaseVMs(); my %DirtyVMsBlockingJobs;
- $self->AddFilter("Status", ["queued", "running"]); - my @SortedJobs = sort CompareJobPriority @{$self->GetItems()}; - my $DirtyIndex = 0; - foreach my $Job (@SortedJobs) + foreach my $Job (@$SortedJobs) { my $Steps = $Job->Steps; $Steps->AddFilter("Status", ["queued", "running"]); @@ -489,7 +485,7 @@ sub ScheduleOnHost =pod =over 12
-=item C<Schedule()> +=item C<ScheduleJobs()>
Goes through the WineTestBot hosts and schedules the Job tasks on each of them using WineTestBot::Jobs::ScheduleOnHost(). @@ -497,9 +493,13 @@ them using WineTestBot::Jobs::ScheduleOnHost(). =back =cut
-sub Schedule +sub ScheduleJobs() { - my $self = shift; + my $Jobs = CreateJobs(); + $Jobs->AddFilter("Status", ["queued", "running"]); + my @SortedJobs = sort CompareJobPriority @{$Jobs->GetItems()}; + # Note that even if there are no jobs to schedule + # we should check if there are VMs to revert
my %Hosts; foreach my $VM (@{CreateVMs()->GetItems()}) @@ -507,24 +507,21 @@ sub Schedule my $Host = $VM->GetHost(); $Hosts{$Host}->{$VM->VirtURI} = 1; } - my $ErrMessage; + + my @ErrMessages; foreach my $Host (keys %Hosts) { my @HostHypervisors = keys %{$Hosts{$Host}}; - my $HostErrMessage = $self->ScheduleOnHost(@HostHypervisors); - if (! defined($ErrMessage)) - { - $ErrMessage = $HostErrMessage; - } + my $HostErrMessage = ScheduleOnHost(@SortedJobs, @HostHypervisors); + push @ErrMessages, $HostErrMessage if (defined $HostErrMessage); } - - return $ErrMessage; + return @ErrMessages ? join("\n", @ErrMessages) : undef; }
=pod =over 12
-=item C<Check()> +=item C<CheckJobs()>
Goes through the list of Jobs and updates their status. As a side-effect this detects failed builds, dead child processes, etc. @@ -532,12 +529,11 @@ detects failed builds, dead child processes, etc. =back =cut
-sub Check +sub CheckJobs() { - my $self = shift; - - $self->AddFilter("Status", ["queued", "running"]); - map { $_->UpdateStatus(); } @{$self->GetItems()}; + my $Jobs = CreateJobs(); + $Jobs->AddFilter("Status", ["queued", "running"]); + map { $_->UpdateStatus(); } @{$Jobs->GetItems()};
return undef; } diff --git a/testbot/lib/WineTestBot/Patches.pm b/testbot/lib/WineTestBot/Patches.pm index 2852242..232e767 100644 --- a/testbot/lib/WineTestBot/Patches.pm +++ b/testbot/lib/WineTestBot/Patches.pm @@ -110,7 +110,7 @@ Analyzes the current patch to determine which Wine tests are impacted. Then for each impacted test it creates a high priority WineTestBot::Job to run that test. This also creates the WineTestBot::Step objects for that Job, as well as the WineTestBot::Task objects to run the test on each 'base' VM. Finally it calls -CWineTestBot::Jobs::Schedule() to run the new Jobs. +CWineTestBot::Jobs::ScheduleJobs() to run the new Jobs.
Note that the path to the file containing the actual patch is passed as a parameter. This is used to apply a combined patch for patch series. See @@ -187,7 +187,7 @@ sub Submit my $First = 1; foreach my $BaseName (keys %Targets) { - my $Jobs = WineTestBot::Jobs::CreateJobs(); + my $Jobs = CreateJobs();
# Create a new job for this patch my $NewJob = $Jobs->Add(); @@ -283,7 +283,7 @@ sub Submit } $self->Disposition($Disposition);
- WineTestBot::Jobs::CreateJobs()->Schedule(); + ScheduleJobs();
return undef; }