Francois Gouget : testbot/lib: Turn the Jobs::Schedule() and Jobs::Check() methods into functions.
Alexandre Julliard
julliard at winehq.org
Fri Nov 2 15:37:54 CDT 2012
Module: tools
Branch: master
Commit: 73c62ab7dc4bafb83bfc676b018b5db55e5edd3a
URL: http://source.winehq.org/git/tools.git/?a=commit;h=73c62ab7dc4bafb83bfc676b018b5db55e5edd3a
Author: Francois Gouget <fgouget at 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);
- at EXPORT = qw(&CreateJobs);
+ at 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
-C<WineTestBot::Jobs::Schedule()> to run the new Jobs.
+C<WineTestBot::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;
}
More information about the wine-cvs
mailing list