[1/3] testbot/VMs: Cleanly separate the VM type and role, and clean up support for retired VMs.

Francois Gouget fgouget at codeweavers.com
Tue Oct 23 11:53:41 CDT 2012


- The base, winetest and extra roles replace the old base, extra and 
  retired types.
- The retired role prevents users from scheduling jobs on such a VM 
  without having to abuse the offline status. So offline is again only 
  used by WineTestBot to mark VMs that don't work and need to be looked 
  at, and makes the website's offline indicator meaningful again.
- Also with this change one no longer 'loses' the type of the VM (build 
  or windows) when retiring it. A nice upshot is that we no longer 
  schedule 64-bit tests on retired build VMs!
- The VM administration page also validates the Type and Role 
  combinations.
- The code also makes more use of the filtering capabilities to only 
  work on the relevant VMs.
---
 testbot/bin/CheckForWinetestUpdate.pl |   57 ++++++------
 testbot/bin/WineRunTask.pl            |   12 ++-
 testbot/ddl/update16.sql              |   43 +++++++++
 testbot/ddl/winetestbot.sql           |    4 +-
 testbot/doc/winetestbot-schema.dia    |   10 +--
 testbot/lib/WineTestBot/Config.pm     |    6 +-
 testbot/lib/WineTestBot/Jobs.pm       |   36 ++++----
 testbot/lib/WineTestBot/Patches.pm    |   69 ++++++---------
 testbot/lib/WineTestBot/VMs.pm        |   74 ++++++++++++----
 testbot/web/Submit.pl                 |  154 +++++++++++++++++----------------
 testbot/web/admin/VMsList.pl          |    5 +-
 testbot/web/index.pl                  |    2 +-
 12 files changed, 266 insertions(+), 206 deletions(-)
 create mode 100644 testbot/ddl/update16.sql

diff --git a/testbot/bin/CheckForWinetestUpdate.pl b/testbot/bin/CheckForWinetestUpdate.pl
index 5285d89..039c4d4 100755
--- a/testbot/bin/CheckForWinetestUpdate.pl
+++ b/testbot/bin/CheckForWinetestUpdate.pl
@@ -70,28 +70,30 @@ sub AddJob
   my $Tasks = $NewStep->Tasks;
   my $HasTasks = !1;
   my $VMs = CreateVMs();
+  if ($Bits == 64)
+  {
+      $VMs->AddFilter("Type", ["win64"]);
+      $VMs->AddFilter("Role", ["base", "winetest"]);
+  }
+  elsif ($BaseJob)
+  {
+      $VMs->AddFilter("Type", ["win32", "win64"]);
+      $VMs->AddFilter("Role", ["base"]);
+  }
+  else
+  {
+      $VMs->AddFilter("Type", ["win32", "win64"]);
+      $VMs->AddFilter("Role", ["winetest"]);
+  }
   # Don't schedule the 'offline' ones
   $VMs->AddFilter("Status", ["reverting", "sleeping", "idle", "running", "dirty"]);
   foreach my $VMKey (@{$VMs->SortKeysBySortOrder($VMs->GetKeys())})
   {
     my $VM = $VMs->GetItem($VMKey);
-    my $AddThisVM;
-    if ($Bits == 32)
-    {
-      $AddThisVM = ($BaseJob && $VM->Type eq "base") ||
-                   (! $BaseJob && $VM->Type eq "extra");
-    }
-    else
-    {
-      $AddThisVM = ($VM->Bits == 64 && $VM->Type ne "build");
-    }
-    if ($AddThisVM)
-    {
-      my $Task = $Tasks->Add();
-      $Task->VM($VM);
-      $Task->Timeout($SuiteTimeout);
-      $HasTasks = 1;
-    }
+    my $Task = $Tasks->Add();
+    $Task->VM($VM);
+    $Task->Timeout($SuiteTimeout);
+    $HasTasks = 1;
   }
 
   # Now save the whole thing
@@ -127,22 +129,17 @@ sub AddReconfigJob
 
   # Add a task for the build VM
   my $Tasks = $NewStep->Tasks;
-  my $HasTasks = !1;
   my $VMs = CreateVMs();
-  foreach my $VMKey (@{$VMs->SortKeysBySortOrder($VMs->GetKeys())})
-  {
-    my $VM = $VMs->GetItem($VMKey);
-    if ($VM->Type eq "build")
-    {
-      my $Task = $Tasks->Add();
-      $Task->VM($VM);
-      $Task->Timeout($ReconfigTimeout);
-      last;
-    }
-  }
+  $VMs->AddFilter("Type", ["build"]);
+  $VMs->AddFilter("Role", ["base"]);
+  my $BuildKey = ${$VMs->GetKeys()}[0];
+  my $VM = $VMs->GetItem($BuildKey);
+  my $Task = $Tasks->Add();
+  $Task->VM($VM);
+  $Task->Timeout($ReconfigTimeout);
 
   # Now save the whole thing
-  (my $ErrKey, my $ErrProperty, my $ErrMessage) = $Jobs->Save();
+  my ($ErrKey, $ErrProperty, $ErrMessage) = $Jobs->Save();
   if (defined($ErrMessage))
   {
     LogMsg "CheckForWinetestUpdate: Failed to save reconfig job: $ErrMessage\n";
diff --git a/testbot/bin/WineRunTask.pl b/testbot/bin/WineRunTask.pl
index 94cb265..6f63640 100755
--- a/testbot/bin/WineRunTask.pl
+++ b/testbot/bin/WineRunTask.pl
@@ -52,7 +52,7 @@ sub FatalError
     $Task->Save();
     $Job->UpdateStatus();
 
-    if ($Task->VM->Type eq "extra" || $Task->VM->Type eq "retired")
+    if ($Task->VM->Role ne "base")
     {
       $Task->VM->PowerOff();
     }
@@ -74,8 +74,7 @@ sub FatalError
     if ($Task && $Step->Type eq "suite")
     {
       my $LatestName = "$DataDir/latest/" . $Task->VM->Name . "_" .
-                       ($Task->VM->Bits == 64 &&
-                        $Step->FileType eq "exe64" ? "64" : "32") . ".err";
+                       ($Step->FileType eq "exe64" ? "64" : "32") . ".err";
       unlink($LatestName);
       link($RptFileName, $LatestName);
     }
@@ -151,8 +150,7 @@ sub RetrieveLogFile
   }
 
   my $LatestNameBase = "$DataDir/latest/" . $VM->Name . "_" .
-                       ($VM->Bits == 64 &&
-                        $Step->FileType eq "exe64" ? "64" : "32");
+                       ($Step->FileType eq "exe64" ? "64" : "32");
   unlink("${LatestNameBase}.log");
   unlink("${LatestNameBase}.err");
   link("$DataDir/jobs/" . $Job->Id . "/" . $Step->No . "/" . $Task->No . "/log",
@@ -300,7 +298,7 @@ elsif ($Step->Type eq "suite")
   $Script .= "$FileName ";
   my $Tag = lc($TagPrefix) . "-" . lc($VM->Name);
   $Tag =~ s/[^a-zA-Z0-9]/-/g;
-  if ($VM->Bits == 64)
+  if ($VM->Type eq "win64")
   {
     $Tag .= "-" . ($FileType eq "exe64" ? "64" : "32");
   }
@@ -353,7 +351,7 @@ else
 }
 $Task->Save();
 $Job->UpdateStatus();
-if ($Task->VM->Type eq "extra" || $Task->VM->Type eq "retired")
+if ($Task->VM->Role ne "base")
 {
   $Task->VM->PowerOff();
 }
diff --git a/testbot/ddl/update16.sql b/testbot/ddl/update16.sql
new file mode 100644
index 0000000..9c9e326
--- /dev/null
+++ b/testbot/ddl/update16.sql
@@ -0,0 +1,43 @@
+USE winetestbot;
+
+ALTER TABLE VMs
+  CHANGE Type OldType ENUM('base', 'extra', 'build', 'retired') NOT NULL,
+  ADD Type ENUM('win32', 'win64', 'build') NULL
+      AFTER SortOrder,
+  ADD Role ENUM('base', 'winetest', 'extra', 'retired') NULL
+      AFTER Type;
+
+UPDATE VMs
+  SET Type = 'build', Role = 'base'
+  WHERE OldType = 'build';
+
+UPDATE VMs
+  SET Type = 'win32'
+  WHERE Bits = '32' AND OldType <> 'build';
+
+UPDATE VMs
+  SET Type = 'win64'
+  WHERE Bits = '64' AND OldType <> 'build';
+
+UPDATE VMs
+  SET Role = 'base'
+  WHERE OldType = 'base';
+
+UPDATE VMs
+  SET Role = 'winetest'
+  WHERE OldType = 'extra';
+
+UPDATE VMs
+  SET Role = 'extra'
+  WHERE OldType = 'retired' AND STATUS <> 'offline';
+
+UPDATE VMs
+  SET Role = 'retired'
+  WHERE OldType = 'retired' AND Status = 'offline';
+
+
+ALTER TABLE VMs
+  DROP OldType,
+  DROP Bits,
+  MODIFY Type ENUM('win32', 'win64', 'build') NOT NULL,
+  MODIFY Role ENUM('extra', 'base', 'winetest', 'retired') NOT NULL;
diff --git a/testbot/ddl/winetestbot.sql b/testbot/ddl/winetestbot.sql
index c5d9e61..daa0ed6 100644
--- a/testbot/ddl/winetestbot.sql
+++ b/testbot/ddl/winetestbot.sql
@@ -45,9 +45,9 @@ ENGINE=InnoDB DEFAULT CHARSET=utf8;
 CREATE TABLE VMs
 (
   Name         VARCHAR(20)      NOT NULL,
-  Type         ENUM('extra', 'base', 'build', 'retired') NOT NULL,
   SortOrder    INT(3)           NOT NULL,
-  Bits         ENUM('32', '64') NOT NULL,
+  Type         ENUM('win32', 'win64', 'build') NOT NULL,
+  Role         ENUM('extra', 'base', 'winetest', 'retired') NOT NULL,
   Status       ENUM('dirty', 'reverting', 'sleeping', 'idle', 'running', 'offline') NOT NULL,
   VirtURI      VARCHAR(64)      NOT NULL,
   VirtDomain   VARCHAR(32)      NOT NULL,
diff --git a/testbot/doc/winetestbot-schema.dia b/testbot/doc/winetestbot-schema.dia
index f5a03af..043c1af 100644
--- a/testbot/doc/winetestbot-schema.dia
+++ b/testbot/doc/winetestbot-schema.dia
@@ -2173,10 +2173,10 @@
         </dia:composite>
         <dia:composite type="table_attribute">
           <dia:attribute name="name">
-            <dia:string>#Type#</dia:string>
+            <dia:string>#SortOrder#</dia:string>
           </dia:attribute>
           <dia:attribute name="type">
-            <dia:string>#ENUM#</dia:string>
+            <dia:string>#INT(3)#</dia:string>
           </dia:attribute>
           <dia:attribute name="comment">
             <dia:string>##</dia:string>
@@ -2193,10 +2193,10 @@
         </dia:composite>
         <dia:composite type="table_attribute">
           <dia:attribute name="name">
-            <dia:string>#SortOrder#</dia:string>
+            <dia:string>#Type#</dia:string>
           </dia:attribute>
           <dia:attribute name="type">
-            <dia:string>#INT(3)#</dia:string>
+            <dia:string>#ENUM#</dia:string>
           </dia:attribute>
           <dia:attribute name="comment">
             <dia:string>##</dia:string>
@@ -2213,7 +2213,7 @@
         </dia:composite>
         <dia:composite type="table_attribute">
           <dia:attribute name="name">
-            <dia:string>#Bits#</dia:string>
+            <dia:string>#Role#</dia:string>
           </dia:attribute>
           <dia:attribute name="type">
             <dia:string>#ENUM#</dia:string>
diff --git a/testbot/lib/WineTestBot/Config.pm b/testbot/lib/WineTestBot/Config.pm
index 789ddbe..98fcbb0 100644
--- a/testbot/lib/WineTestBot/Config.pm
+++ b/testbot/lib/WineTestBot/Config.pm
@@ -26,7 +26,7 @@ WineTestBot::Config - Site-independent configuration settings
 
 use vars qw (@ISA @EXPORT @EXPORT_OK $UseSSL $LogDir $DataDir $BinDir
              $DbDataSource $DbUsername $DbPassword $MaxRevertingVMs
-             $MaxRunningVMs $MaxExtraPoweredOnVms $SleepAfterRevert
+             $MaxRunningVMs $MaxNonBasePoweredOnVms $SleepAfterRevert
              $WaitForToolsInVM $AdminEMail $RobotEMail $WinePatchToOverride
              $WinePatchCc $SuiteTimeout $SingleTimeout
              $BuildTimeout $ReconfigTimeout $OverheadTimeout $TagPrefix
@@ -38,7 +38,7 @@ use vars qw (@ISA @EXPORT @EXPORT_OK $UseSSL $LogDir $DataDir $BinDir
 require Exporter;
 @ISA = qw(Exporter);
 @EXPORT = qw($UseSSL $LogDir $DataDir $BinDir
-             $MaxRevertingVMs $MaxRunningVMs $MaxExtraPoweredOnVms
+             $MaxRevertingVMs $MaxRunningVMs $MaxNonBasePoweredOnVms
              $SleepAfterRevert $WaitForToolsInVM $AdminEMail $RobotEMail
              $WinePatchToOverride $WinePatchCc $SuiteTimeout
              $SingleTimeout $BuildTimeout $ReconfigTimeout $OverheadTimeout
@@ -54,7 +54,7 @@ $BinDir = "/home/winehq/tools/testbot/bin";
 
 $MaxRevertingVMs = 1;
 $MaxRunningVMs = 2;
-$MaxExtraPoweredOnVms = 2;
+$MaxNonBasePoweredOnVms = 2;
 $SleepAfterRevert = 30;
 $WaitForToolsInVM = 30;
 
diff --git a/testbot/lib/WineTestBot/Jobs.pm b/testbot/lib/WineTestBot/Jobs.pm
index 07a0685..b56e203 100644
--- a/testbot/lib/WineTestBot/Jobs.pm
+++ b/testbot/lib/WineTestBot/Jobs.pm
@@ -412,7 +412,7 @@ on the host.
 =item *
 
 FIXME: The actual limit on the number of powered on VMs is blurred by the
-$MaxExtraPoweredOnVms setting and the last loop in ScheduleOnHost().
+$MaxNonBasePoweredOnVms setting and the last loop in ScheduleOnHost().
 
 =item *
 
@@ -423,7 +423,8 @@ when reverting too many VMs at once.
 =item *
 
 No Task is started while there are VMs that are being reverted. This is so that
-the tests are not disrupted by the disk or CPU activity caused reverting a VM.
+the tests are not disrupted by the disk or CPU activity caused by reverting a
+VM.
 
 =cut
 
@@ -438,7 +439,7 @@ sub ScheduleOnHost
   my $HostVMs = CreateVMs();
   $HostVMs->FilterHypervisor($Hypervisors);
   my ($RevertingVMs, $RunningVMs) = $HostVMs->CountRevertingRunningVMs();
-  my $PoweredOnExtraVMs = $HostVMs->CountPoweredOnExtraVMs();
+  my $PoweredOnNonBaseVMs = $HostVMs->CountPoweredOnNonBaseVMs();
   my %DirtyVMsBlockingJobs;
 
   $self->AddFilter("Status", ["queued", "running"]);
@@ -498,40 +499,41 @@ sub ScheduleOnHost
 
   if ($RunningVMs != 0)
   {
+    # We don't revert VMs while jobs are running so we're done
     return undef;
   }
 
-  # Sort the VMs by decreasing order of priority of their Jobs
+  # Sort the VMs by decreasing order of priority of the jobs they block
   my @DirtyVMsByIndex = sort { $DirtyVMsBlockingJobs{$a} <=> $DirtyVMsBlockingJobs{$b} } keys %DirtyVMsBlockingJobs;
   my $VMKey;
   foreach $VMKey (@DirtyVMsByIndex)
   {
+    last if ($RevertingVMs >= $MaxRevertingVMs);
+
     my $VM = $HostVMs->GetItem($VMKey);
-    if ($RevertingVMs < $MaxRevertingVMs)
+    if ($VM->Role ne "base")
     {
-      if ($VM->Type eq "extra" || $VM->Type eq "retired")
-      {
-        if ($PoweredOnExtraVMs < $MaxExtraPoweredOnVms)
-        {
-          $VM->RunRevert();
-          $PoweredOnExtraVMs++;
-          $RevertingVMs++;
-        }
-      }
-      else
+      if ($PoweredOnNonBaseVMs < $MaxNonBasePoweredOnVms)
       {
         $VM->RunRevert();
+        $PoweredOnNonBaseVMs++;
         $RevertingVMs++;
       }
     }
+    else
+    {
+      $VM->RunRevert();
+      $RevertingVMs++;
+    }
   }
+
+  # Again for the VMs that don't block any job
   foreach $VMKey (@{$HostVMs->GetKeys()})
   {
     my $VM = $HostVMs->GetItem($VMKey);
     if (! defined($DirtyVMsBlockingJobs{$VMKey}) &&
         $RevertingVMs < $MaxRevertingVMs &&
-        $VM->Status eq 'dirty' && $VM->Type ne "extra" &&
-        $VM->Type ne "retired")
+        $VM->Status eq 'dirty' && $VM->Role eq "base")
     {
       $VM->RunRevert();
       $RevertingVMs++;
diff --git a/testbot/lib/WineTestBot/Patches.pm b/testbot/lib/WineTestBot/Patches.pm
index be8ba9a..c9e0145 100644
--- a/testbot/lib/WineTestBot/Patches.pm
+++ b/testbot/lib/WineTestBot/Patches.pm
@@ -124,6 +124,7 @@ sub Submit
   my $self = shift;
   my ($PatchFileName, $IsSet) = @_;
 
+  # See also OnSubmit() in web/Submit.pl
   my %Targets;
   if (open(BODY, "<$DataDir/patches/" . $self->Id))
   {
@@ -222,6 +223,7 @@ sub Submit
     my $Tasks = $NewStep->Tasks;
     my $VMs = CreateVMs();
     $VMs->AddFilter("Type", ["build"]);
+    $VMs->AddFilter("Role", ["base"]);
     my $BuildKey = ${$VMs->GetKeys()}[0];
     my $VM = $VMs->GetItem($BuildKey);
     my $Task = $Tasks->Add();
@@ -230,53 +232,32 @@ sub Submit
   
     foreach my $TestSet (keys %{$Targets{$BaseName}})
     {
-      # Add 32-bit test run
-      $NewStep = $Steps->Add();
-      my $TestExecutablePart = $BaseName;
-      if ($Targets{$BaseName}{$TestSet} eq "patchprograms")
+      # Add 32 and 64-bit tasks
+      foreach my $Bits ("32", "64")
       {
-        $TestExecutablePart .= ".exe";
-      }
-      $NewStep->FileName("${TestExecutablePart}_test.exe");
-      $NewStep->FileType("exe32");
-      $NewStep->InStaging(!1);
-    
-      # Add 32-bit tasks
-      $Tasks = $NewStep->Tasks;
-      $VMs = CreateVMs();
-      $VMs->AddFilter("Type", ["base"]);
-      # Don't schedule the 'offline' ones
-      $VMs->AddFilter("Status", ["reverting", "sleeping", "idle", "running", "dirty"]);
-      my $Have64VMs = !1;
-      my $SortedKeys = $VMs->SortKeysBySortOrder($VMs->GetKeys());
-      foreach my $VMKey (@$SortedKeys)
-      {
-        my $VM = $VMs->GetItem($VMKey);
-        my $Task = $Tasks->Add();
-        $Task->VM($VM);
-        $Task->Timeout($SingleTimeout);
-        $Task->CmdLineArg($TestSet);
-        if ($VM->Bits == 64)
-        {
-          $Have64VMs = 1;
-        }
-      }
-    
-      if ($Have64VMs)
-      {
-        # Add 64-bit test run
-        $NewStep = $Steps->Add();
-        $NewStep->FileName("${TestExecutablePart}_test64.exe");
-        $NewStep->FileType("exe64");
-        $NewStep->InStaging(!1);
-      
-        # Add 64-bit tasks
-        $Tasks = $NewStep->Tasks;
-        foreach my $VMKey (@$SortedKeys)
+        $VMs = CreateVMs();
+        $VMs->AddFilter("Type", $Bits eq "32" ? ["win32", "win64"] : ["win64"]);
+        $VMs->AddFilter("Role", ["base"]);
+        # Don't schedule the 'offline' ones
+        $VMs->AddFilter("Status", ["reverting", "sleeping", "idle", "running", "dirty"]);
+        if (@{$VMs->GetKeys()})
         {
-          my $VM = $VMs->GetItem($VMKey);
-          if ($VM->Bits == 64)
+          # Create the corresponding Step
+          $NewStep = $Steps->Add();
+          my $FileName = $BaseName;
+          $FileName .= ".exe" if ($Targets{$BaseName}{$TestSet} eq "patchprograms");
+          $FileName .= "_test";
+          $FileName .= "64" if ($Bits eq "64");
+          $NewStep->FileName("$FileName.exe");
+          $NewStep->FileType("exe$Bits");
+          $NewStep->InStaging(!1);
+          $Tasks = $NewStep->Tasks;
+
+          # And a task for each VM
+          my $SortedKeys = $VMs->SortKeysBySortOrder($VMs->GetKeys());
+          foreach my $VMKey (@$SortedKeys)
           {
+            my $VM = $VMs->GetItem($VMKey);
             my $Task = $Tasks->Add();
             $Task->VM($VM);
             $Task->Timeout($SingleTimeout);
diff --git a/testbot/lib/WineTestBot/VMs.pm b/testbot/lib/WineTestBot/VMs.pm
index 5206b05..84d3fe8 100644
--- a/testbot/lib/WineTestBot/VMs.pm
+++ b/testbot/lib/WineTestBot/VMs.pm
@@ -98,27 +98,50 @@ commands in it. This part is used to start the tasks in the VM but is
 implemented independently from the VM's hypervisor since most do not provide
 this functionality.
 
-There are four types of VMs:
+The VM type defines what the it can do:
 
 =over 12
 
-=item base
+=item build
 
-This defines the set of Windows VMs that the Wine tests are run on by default,
-especially Wine commits and wine-patches emails.
+This is a Unix VM that can build the 32-bit and 64-bit Windows test binaries.
 
-=item extra
+=item win32
 
-This is a set of extra Windows VMs that users can manually chose to run their
-tests on.
+This is a 32-bit Windows VM that can run the 32-bit tests.
 
-=item build
+=item win64
+
+This is a 64-bit Windows VM that can run both the 32-bit and 64-bit tests.
+
+=back
 
-This is a Unix VM used to build the Wine test binaries.
+
+The VM role defines what we use it for:
+
+=over 12
 
 =item retired
 
-These VMs are no longer used.
+A retired VM is no longer used at all. No new jobs can be scheduled to run on
+them.
+
+=item base
+
+A base VM is used for every suitable task. This is the only role that build VMs
+can play besides retired. For Windows VMs, this means that it will run the
+WineTest jobs, the wine-patches jobs, and also the manually submitted jobs
+unless the submitter decided otherwise.
+
+=item winetest
+
+This is only valid for Windows VMs. By default these VMs only run the WineTest
+jobs. They can also be selected for manually submitted jobs.
+
+=item extra
+
+This is only valid for Windows VMs. They are only used if selected for a
+manually submitted job.
 
 =back
 
@@ -155,8 +178,10 @@ next step will be to revert it to the idle snapshot so it can be used again.
 
 =item offline
 
-This VM should not be used. This is typically the case of Retired VMs but it
-can also happen if an error happens while manipulating the VM.
+This VM should not be used. The WineTestBot automatically puts VMs into this
+state if errors happen when manipulating them, such as if they fail to revert,
+etc. The main web status page has a warning indicator on when some VMs are
+offline.
 
 =back
 
@@ -417,6 +442,18 @@ sub Status
   return $NewStatus;
 }
 
+sub Validate
+{
+  my $self = shift;
+
+  if ($self->Type ne "win32" && $self->Type ne "win64" &&
+      ($self->Role eq "winetest" || $self->Role eq "extra"))
+  {
+      return ("Role", "Only win32 and win64 VMs can have a role of '" . $self->Role . "'");
+  }
+  return $self->SUPER::Validate(@_);
+}
+
 sub OnSaved
 {
   my $self = shift;
@@ -466,8 +503,8 @@ WineTestBot::VMs - A VM collection
 
 =head1 DESCRIPTION
 
-This is the collection of VMs the testbot knows about, including the build VM
-and retired (no longer used) VMs.
+This is the collection of VMs the testbot knows about, no matter their type,
+role or status.
 
 =cut
 
@@ -493,9 +530,9 @@ BEGIN
 {
   @PropertyDescriptors = (
     CreateBasicPropertyDescriptor("Name", "VM name", 1, 1, "A", 20),
-    CreateEnumPropertyDescriptor("Type", "Type of VM", !1, 1, ['extra', 'base', 'build', 'retired']),
     CreateBasicPropertyDescriptor("SortOrder", "Display order", !1, 1, "N", 3),
-    CreateEnumPropertyDescriptor("Bits", "32 or 64 bits", !1, 1, ['32', '64']),
+    CreateEnumPropertyDescriptor("Type", "Type of VM", !1, 1, ['win32', 'win64', 'build']),
+    CreateEnumPropertyDescriptor("Role", "VM Role", !1, 1, ['extra', 'base', 'winetest', 'retired']),
     CreateEnumPropertyDescriptor("Status", "Current status", !1, 1, ['dirty', 'reverting', 'sleeping', 'idle', 'running', 'offline']),
     CreateBasicPropertyDescriptor("VirtURI", "LibVirt URI of the VM", !1, 1, "A", 64),
     CreateBasicPropertyDescriptor("VirtDomain", "LibVirt Domain for the VM", !1, 1, "A", 32),
@@ -542,7 +579,7 @@ sub CountRevertingRunningVMs
   return ($RevertingVMs, $RunningVMs);
 }
 
-sub CountPoweredOnExtraVMs
+sub CountPoweredOnNonBaseVMs
 {
   my $self = shift;
 
@@ -552,8 +589,7 @@ sub CountPoweredOnExtraVMs
     my $VM = $self->GetItem($VMKey);
     my $VMStatus = $VM->Status;
 
-    if (($VM->Type eq "extra" ||
-         $VM->Type eq "retired") &&
+    if ($VM->Role ne "base" &&
         ($VMStatus eq "reverting" ||
          $VMStatus eq "sleeping" ||
          $VMStatus eq "idle" ||
diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl
index 9481629..7d44903 100644
--- a/testbot/web/Submit.pl
+++ b/testbot/web/Submit.pl
@@ -211,11 +211,13 @@ sub GenerateFields
       if ($self->GetParam("Page") == 3)
       {
         my $VMs = CreateVMs();
+        # VMs that are only visible with ShowAll
+        $VMs->AddFilter("Role", ["winetest", "extra"]);
         foreach my $VMKey (@{$VMs->GetKeys()})
         {
           my $VM = $VMs->GetItem($VMKey);
           my $FieldName = "vm_" . $self->CGI->escapeHTML($VMKey);
-          if (defined($self->GetParam($FieldName)) && $VM->Type ne "base")
+          if (defined $self->GetParam($FieldName))
           {
             $self->{ShowAll} = 1;
             last;
@@ -228,38 +230,42 @@ sub GenerateFields
       }
   
       my $VMs = CreateVMs();
+      if ($self->{FileType} eq "exe64" || $self->{FileType} eq "dll64")
+      {
+          $VMs->AddFilter("Type", ["win64"]);
+      }
+      else
+      {
+          $VMs->AddFilter("Type", ["win32", "win64"]);
+      }
       # Don't even show the 'offline' ones
       $VMs->AddFilter("Status", ["reverting", "sleeping", "idle", "running", "dirty"]);
       if ($self->{ShowAll})
       {
-        $VMs->AddFilter("Type", ["base", "extra", "retired"]);
+        # All but the retired ones
+        $VMs->AddFilter("Role", ["base", "winetest", "extra"]);
       }
       else
       {
-        $VMs->AddFilter("Type", ["base"]);
+        $VMs->AddFilter("Role", ["base"]);
       }
       my $SortedKeys = $VMs->SortKeysBySortOrder($VMs->GetKeys());
       foreach my $VMKey (@$SortedKeys)
       {
         my $VM = $VMs->GetItem($VMKey);
-        if ($VM->Bits == 64 || $self->{FileType} eq "exe32" ||
-            $self->{FileType} eq "patchdlls" ||
-            $self->{FileType} eq "patchprograms")
+        my $FieldName = "vm_" . $self->CGI->escapeHTML($VM->GetKey());
+        print "<div class='ItemProperty'><label>",
+              $self->CGI->escapeHTML($VM->Name);
+        if ($VM->Description)
         {
-          my $FieldName = "vm_" . $self->CGI->escapeHTML($VM->GetKey());
-          print "<div class='ItemProperty'><label>",
-                $self->CGI->escapeHTML($VM->Name);
-          if ($VM->Description)
-          {
-            print " (", $self->CGI->escapeHTML($VM->Description), ")";
-          }
-          print "</label><div class='ItemValue'><input type='checkbox' name='$FieldName'";
-          if ($self->GetParam("Page") == 1 || $self->GetParam($FieldName))
-          {
-            print " checked='checked'";
-          }
-          print "/></div></div>\n";
+          print " (", $self->CGI->escapeHTML($VM->Description), ")";
+        }
+        print "</label><div class='ItemValue'><input type='checkbox' name='$FieldName'";
+        if ($self->GetParam("Page") == 1 || $self->GetParam($FieldName))
+        {
+          print " checked='checked'";
         }
+        print "/></div></div>\n";
       }
     }
     else
@@ -368,11 +374,12 @@ sub DisplayProperty
       {
         my $Show64 = !1;
         my $VMs = CreateVMs();
+        $VMs->AddFilter("Type", ["win64"]);
         foreach my $VMKey (@{$VMs->GetKeys()})
         {
           my $VM = $VMs->GetItem($VMKey);
           my $FieldName = "vm_" . $self->CGI->escapeHTML($VM->GetKey());
-          if ($self->GetParam($FieldName) && $VM->Bits == 64)
+          if ($self->GetParam($FieldName))
           {
             $Show64 = 1;
             last;
@@ -798,6 +805,8 @@ sub OnSubmit
     $FileNameRandomPart = $self->GetCurrentSession()->Id;
   }
 
+  # See also Patches::Submit() in lib/WineTestBot/Patches.pm
+
   # First create a new job
   my $Jobs = CreateJobs();
   my $NewJob = $Jobs->Add();
@@ -816,86 +825,79 @@ sub OnSubmit
   {
     $NewJob->Branch($Branch);
   }
-  
-  # Add a step to the job
   my $Steps = $NewJob->Steps;
-  my $NewStep = $Steps->Add();
-  $NewStep->FileName($FileNameRandomPart . " " . $self->GetParam("FileName"));
+
   my $FileType = $self->GetParam("FileType");
-  $NewStep->FileType($FileType);
-  $NewStep->InStaging(1);
   if ($FileType eq "patchdlls" || $FileType eq "patchprograms")
   {
-    $NewStep->Type("build");
-    $NewStep->DebugLevel(0);
+    # This is a patch so add a build step...
+    my $BuildStep = $Steps->Add();
+    $BuildStep->FileName($FileNameRandomPart . " " . $self->GetParam("FileName"));
+    $BuildStep->FileType($FileType);
+    $BuildStep->InStaging(1);
+    $BuildStep->Type("build");
+    $BuildStep->DebugLevel(0);
 
-    # Add build task
-    my $Tasks = $NewStep->Tasks;
+    # ...with a build task
+    my $Tasks = $BuildStep->Tasks;
     my $VMs = CreateVMs();
     $VMs->AddFilter("Type", ["build"]);
+    $VMs->AddFilter("Role", ["base"]);
     my $BuildKey = ${$VMs->GetKeys()}[0];
     my $VM = $VMs->GetItem($BuildKey);
     my $Task = $Tasks->Add();
     $Task->VM($VM);
     $Task->Timeout($BuildTimeout);
-
-    # Add test run step
-    $NewStep = $Steps->Add();
-    $NewStep->FileName($self->GetParam("TestExecutable"));
-    $NewStep->FileType("exe32");
-    $NewStep->InStaging(!1);
   }
 
-  $NewStep->Type("single");
-  $NewStep->DebugLevel($self->GetParam("DebugLevel"));
-  $NewStep->ReportSuccessfulTests(defined($self->GetParam("ReportSuccessfulTests")));
-  
-  # Add a task for each selected VM
-  my $Tasks = $NewStep->Tasks;
-  my $VMs = CreateVMs();
-  my $SortedKeys = $VMs->SortKeysBySortOrder($VMs->GetKeys());
-  foreach my $VMKey (@$SortedKeys)
+  # Add steps and tasks for the 32 and 64-bit tests
+  foreach my $Bits ("32", "64")
   {
-    my $VM = $VMs->GetItem($VMKey);
-    my $FieldName = "vm_" . $self->CGI->escapeHTML($VM->GetKey());
-    if ($self->GetParam($FieldName))
-    {
-      my $Task = $Tasks->Add();
-      $Task->VM($VM);
-      $Task->Timeout($SingleTimeout);
-      $Task->CmdLineArg($self->GetParam("CmdLineArg"));
-    }
-  }
+    next if ($Bits eq "32" && $FileType eq "exe64");
+    next if ($Bits eq "64" && $FileType eq "exe32");
+    next if ($Bits eq "64" && $FileType =~ /^patch/ && !defined($self->GetParam("Run64")));
+    my $Tasks;
 
-  if (($FileType eq "patchdlls" || $FileType eq "patchprograms") &&
-      defined($self->GetParam("Run64")))
-  {
-    # Add step and tasks for 64-bit exe
-    $NewStep = $Steps->Add();
-    my $FileName64 = $self->GetParam("TestExecutable");
-    $FileName64 =~ s/^([a-zA-Z0-9_.]+)_test\.exe/\1_test64.exe/;
-    $NewStep->FileName($FileName64);
-    $NewStep->FileType("exe64");
-    $NewStep->InStaging(!1);
-    $NewStep->Type("single");
-    $NewStep->DebugLevel($self->GetParam("DebugLevel"));
-    $NewStep->ReportSuccessfulTests(defined($self->GetParam("ReportSuccessfulTests")));
-  
-    # Add a task for each selected 64-bit VM
-    my $Tasks = $NewStep->Tasks;
     my $VMs = CreateVMs();
+    $VMs->AddFilter("Type", $Bits eq "32" ? ["win32", "win64"] : ["win64"]);
     my $SortedKeys = $VMs->SortKeysBySortOrder($VMs->GetKeys());
     foreach my $VMKey (@$SortedKeys)
     {
       my $VM = $VMs->GetItem($VMKey);
       my $FieldName = "vm_" . $self->CGI->escapeHTML($VM->GetKey());
-      if ($VM->Bits == 64 && $self->GetParam($FieldName))
+      next if (!$self->GetParam($FieldName)); # skip unselected VMs
+
+      if (!$Tasks)
       {
-        my $Task = $Tasks->Add();
-        $Task->VM($VM);
-        $Task->Timeout($SingleTimeout);
-        $Task->CmdLineArg($self->GetParam("CmdLineArg"));
+        # First create the test step
+        my $TestStep = $Steps->Add();
+        if ($FileType eq "patchdlls" || $FileType eq "patchprograms")
+        {
+          my $FileName=$self->GetParam("TestExecutable");
+          if ($Bits eq "64")
+          {
+            $FileName =~ s/^([a-zA-Z0-9_.]+)_test\.exe/\1_test64.exe/;
+          }
+          $TestStep->FileName($FileName);
+          $TestStep->InStaging(!1);
+        }
+        else
+        {
+          $TestStep->FileName($FileNameRandomPart . " " . $self->GetParam("FileName"));
+          $TestStep->InStaging(1);
+        }
+        $TestStep->FileType("exe$Bits");
+        $TestStep->Type("single");
+        $TestStep->DebugLevel($self->GetParam("DebugLevel"));
+        $TestStep->ReportSuccessfulTests(defined($self->GetParam("ReportSuccessfulTests")));
+        $Tasks = $TestStep->Tasks;
       }
+
+      # Then add a task for this VM
+      my $Task = $Tasks->Add();
+      $Task->VM($VM);
+      $Task->Timeout($SingleTimeout);
+      $Task->CmdLineArg($self->GetParam("CmdLineArg"));
     }
   }
 
diff --git a/testbot/web/admin/VMsList.pl b/testbot/web/admin/VMsList.pl
index 05a89e2..c148582 100644
--- a/testbot/web/admin/VMsList.pl
+++ b/testbot/web/admin/VMsList.pl
@@ -40,8 +40,9 @@ sub DisplayProperty
 
   my $PropertyName = $PropertyDescriptor->GetName();
 
-  return $PropertyName eq "Name" || $PropertyName eq "Bits" ||
-         $PropertyName eq "Status" || $PropertyName eq "Description";
+  return $PropertyName eq "Name" || $PropertyName eq "Type" ||
+         $PropertyName eq "Role" || $PropertyName eq "Status" ||
+         $PropertyName eq "Description";
 }
 
 sub SortKeys
diff --git a/testbot/web/index.pl b/testbot/web/index.pl
index 516c654..20ac591 100644
--- a/testbot/web/index.pl
+++ b/testbot/web/index.pl
@@ -128,7 +128,7 @@ sub DisplayProperty
 
   my $PropertyName = $PropertyDescriptor->GetName();
   return $PropertyName eq "Name" || $PropertyName eq "Type" ||
-         $PropertyName eq "Bits" || $PropertyName eq "Status" ||
+         $PropertyName eq "Role" || $PropertyName eq "Status" ||
          $PropertyName eq "Description";
 }
 
-- 
1.7.10.4




More information about the wine-patches mailing list