[Tools] testbot: Centralize handling of the Job, Step and Task directories.

Also avoid invoking external tools to delete directories.

Signed-off-by: Francois Gouget <[email protected]>
---
 testbot/bin/Engine.pl                 |  2 +-
 testbot/bin/Janitor.pl                |  5 ++---
 testbot/bin/WineRunBuild.pl           | 16 ++++++----------
 testbot/bin/WineRunReconfig.pl        |  6 +-----
 testbot/bin/WineRunTask.pl            | 13 +++++--------
 testbot/bin/WineSendLog.pl            | 15 ++++-----------
 testbot/lib/WineTestBot/Jobs.pm       | 31 ++++++++++++++++++++++++++----
 testbot/lib/WineTestBot/Steps.pm      | 32 ++++++++++++++++++++++++++-----
 testbot/lib/WineTestBot/StepsTasks.pm | 13 +++++++++++++
 testbot/lib/WineTestBot/Tasks.pm      | 36 +++++++++++++++++++++++++++--------
 testbot/web/GetFile.pl                |  4 ++--
 testbot/web/JobDetails.pl             |  6 ++----
 12 files changed, 118 insertions(+), 61 deletions(-)

diff --git a/testbot/bin/Engine.pl b/testbot/bin/Engine.pl
index 48404496..598d2bd7 100755
--- a/testbot/bin/Engine.pl
+++ b/testbot/bin/Engine.pl
@@ -141,7 +141,7 @@ sub Cleanup(;$$)
         if ($Requeue)
         {
           LogMsg "Requeuing $TaskKey\n";
-          system("rm", "-r", "-f", "$DataDir/jobs/$TaskKey");
+          $Task->RmTree();
           $Task->Status("queued");
           $Task->Started(undef);
           $Task->Ended(undef);
diff --git a/testbot/bin/Janitor.pl b/testbot/bin/Janitor.pl
index 383ae315..fd1b256b 100755
--- a/testbot/bin/Janitor.pl
+++ b/testbot/bin/Janitor.pl
@@ -60,7 +60,7 @@ if ($JobPurgeDays != 0)
     if (defined($Job->Ended) && $Job->Ended < $DeleteBefore)
     {
       LogMsg "Deleting job ", $Job->Id, "\n";
-      system "rm", "-rf", "$DataDir/jobs/" . $Job->Id;
+      $Job->RmTree();
       my $ErrMessage = $Jobs->DeleteItem($Job);
       if (defined($ErrMessage))
       {
@@ -135,8 +135,7 @@ if ($JobArchiveDays != 0)
       LogMsg "Archiving job ", $Job->Id, "\n";
       foreach my $Step (@{$Job->Steps->GetItems()})
       {
-        unlink "$DataDir/jobs/" . $Job->Id . "/" . $Step->No . "/" .
-               $Step->FileName;
+        unlink $Step->GetDir() . "/" . $Step->FileName;
       }
 
       $Job->Archived(1);
diff --git a/testbot/bin/WineRunBuild.pl b/testbot/bin/WineRunBuild.pl
index e58d710a..b91d594b 100755
--- a/testbot/bin/WineRunBuild.pl
+++ b/testbot/bin/WineRunBuild.pl
@@ -159,17 +159,13 @@ if (!defined $Task)
 }
 
 my $OldUMask = umask(002);
-mkdir "$DataDir/jobs/$JobId";
-mkdir "$DataDir/jobs/$JobId/$StepNo";
-mkdir "$DataDir/jobs/$JobId/$StepNo/$TaskNo";
+my $TaskDir = $Task->CreateDir();
 umask($OldUMask);
-
-my $VM = $Task->VM;
-my $StepDir = "$DataDir/jobs/$JobId/$StepNo";
-my $TaskDir = "$StepDir/$TaskNo";
 my $FullLogFileName = "$TaskDir/log";
 my $FullErrFileName = "$TaskDir/err";
 
+my $VM = $Task->VM;
+
 my $Start = Time();
 LogMsg "Task $JobId/$StepNo/$TaskNo started\n";
 
@@ -344,6 +340,7 @@ if (!defined $BaseName)
 # Run the build
 #
 
+my $StepDir = $Step->GetDir();
 my $FileName = $Step->FileName;
 my $TA = $VM->GetAgent();
 Debug(Elapsed($Start), " Sending '$StepDir/$FileName'\n");
@@ -456,9 +453,6 @@ if ($NewStatus eq "completed")
     my $OtherFileName = $OtherStep->FileName;
     next if ($OtherFileName !~ /^[\w_.]+_test(?:64)?\.exe$/);
 
-    my $OtherStepDir = "$DataDir/jobs/$JobId/" . $OtherStep->No;
-    mkdir $OtherStepDir;
-
     my $Bits = $OtherStep->FileType eq "exe64" ? "64" : "32";
     my $TestExecutable;
     if ($Step->FileType ne "patchprograms")
@@ -469,7 +463,9 @@ if ($NewStatus eq "completed")
     {
       $TestExecutable = "build-mingw$Bits/programs/$BaseName/tests/${BaseName}.exe_test.exe";
     }
+
     Debug(Elapsed($Start), " Retrieving '$OtherFileName'\n");
+    my $OtherStepDir = $OtherStep->CreateDir();
     if (!$TA->GetFile($TestExecutable, "$OtherStepDir/$OtherFileName"))
     {
       FatalTAError($TA, "Could not retrieve '$OtherFileName'");
diff --git a/testbot/bin/WineRunReconfig.pl b/testbot/bin/WineRunReconfig.pl
index 3eec9ed5..ff99fb7c 100755
--- a/testbot/bin/WineRunReconfig.pl
+++ b/testbot/bin/WineRunReconfig.pl
@@ -159,14 +159,10 @@ if (!defined $Task)
 }
 
 my $OldUMask = umask(002);
-mkdir "$DataDir/jobs/$JobId";
-mkdir "$DataDir/jobs/$JobId/$StepNo";
-mkdir "$DataDir/jobs/$JobId/$StepNo/$TaskNo";
+my $TaskDir = $Task->CreateDir();
 umask($OldUMask);
 
 my $VM = $Task->VM;
-my $StepDir = "$DataDir/jobs/$JobId/$StepNo";
-my $TaskDir = "$StepDir/$TaskNo";
 my $FullLogFileName = "$TaskDir/log";
 my $FullErrFileName = "$TaskDir/err";
 
diff --git a/testbot/bin/WineRunTask.pl b/testbot/bin/WineRunTask.pl
index 65755f9b..3807a182 100755
--- a/testbot/bin/WineRunTask.pl
+++ b/testbot/bin/WineRunTask.pl
@@ -184,19 +184,15 @@ if (!defined $Task)
 }
 
 my $OldUMask = umask(002);
-mkdir "$DataDir/jobs/$JobId";
-mkdir "$DataDir/jobs/$JobId/$StepNo";
-mkdir "$DataDir/jobs/$JobId/$StepNo/$TaskNo";
+my $TaskDir = $Task->CreateDir();
 umask($OldUMask);
-
-my $VM = $Task->VM;
-my $RptFileName = $VM->Name . ".rpt";
-my $StepDir = "$DataDir/jobs/$JobId/$StepNo";
-my $TaskDir = "$StepDir/$TaskNo";
 my $FullLogFileName = "$TaskDir/log";
 my $FullErrFileName = "$TaskDir/err";
 my $FullScreenshotFileName = "$TaskDir/screenshot.png";
 
+my $VM = $Task->VM;
+my $RptFileName = $VM->Name . ".rpt";
+
 my $Start = Time();
 LogMsg "Task $JobId/$StepNo/$TaskNo started\n";
 
@@ -375,6 +371,7 @@ if ($FileType ne "exe32" && $FileType ne "exe64")
 {
   FatalError("Unexpected file type $FileType found\n");
 }
+my $StepDir = $Step->GetDir();
 my $FileName = $Step->FileName;
 Debug(Elapsed($Start), " Sending '$StepDir/$FileName'\n");
 if (!$TA->SendFile("$StepDir/$FileName", $FileName, 0))
diff --git a/testbot/bin/WineSendLog.pl b/testbot/bin/WineSendLog.pl
index 4d872a48..1bb37f49 100755
--- a/testbot/bin/WineSendLog.pl
+++ b/testbot/bin/WineSendLog.pl
@@ -225,8 +225,7 @@ EOF
 
     print SENDMAIL "\n=== ", $StepTask->GetTitle(), " ===\n";
 
-    my $TaskDir = "$DataDir/jobs/" . $Job->Id . "/" . $StepTask->StepNo .
-                  "/" . $StepTask->TaskNo;
+    my $TaskDir = $StepTask->GetTaskDir();
     if (open LOGFILE, "<$TaskDir/log")
     {
       my $HasLogEntries = !1;
@@ -336,10 +335,8 @@ EOF
     print SENDMAIL "Content-Disposition: attachment; filename=",
                    $StepTask->VM->Name, ".log\n\n";
 
-    my $TaskDir = "$DataDir/jobs/" . $Job->Id . "/" . $StepTask->StepNo .
-                  "/" . $StepTask->TaskNo;
-
     my $PrintSeparator = !1;
+    my $TaskDir = $StepTask->GetTaskDir();
     if (open LOGFILE, "<$TaskDir/log")
     {
       my $Line;
@@ -382,9 +379,7 @@ EOF
   {
     my $StepTask = $StepsTasks->GetItem($Key);
 
-    my $TaskDir = "$DataDir/jobs/" . $Job->Id . "/" . $StepTask->StepNo .
-                  "/" . $StepTask->TaskNo;
-
+    my $TaskDir = $StepTask->GetTaskDir();
     my ($BotFailure, $MessagesFromErr) = CheckErrLog("$TaskDir/err");
     if (! $BotFailure)
     {
@@ -466,10 +461,8 @@ EOF
         my $StepTask = $StepsTasks->GetItem($Key);
         print $result "\n=== ", $StepTask->GetTitle(), " ===\n";
 
-        my $TaskDir = "$DataDir/jobs/" . $Job->Id . "/" . $StepTask->StepNo .
-                      "/" . $StepTask->TaskNo;
-
         my $PrintSeparator = !1;
+        my $TaskDir = $StepTask->GetTaskDir();
         if (open(my $logfile, "<", "$TaskDir/log"))
         {
           my $Line;
diff --git a/testbot/lib/WineTestBot/Jobs.pm b/testbot/lib/WineTestBot/Jobs.pm
index c6eb2900..b34b31ee 100644
--- a/testbot/lib/WineTestBot/Jobs.pm
+++ b/testbot/lib/WineTestBot/Jobs.pm
@@ -51,6 +51,8 @@ A Job is composed of multiple WineTestBot::Step objects.
 
 =cut
 
+use File::Path;
+
 use WineTestBot::Config;
 use WineTestBot::Branches;
 use WineTestBot::Engine::Notify;
@@ -82,6 +84,28 @@ sub InitializeNew($$)
   $self->SUPER::InitializeNew($Collection);
 }
 
+sub GetDir($)
+{
+  my ($self) = @_;
+  my $JobId = $self->GetKey();
+  return "$DataDir/jobs/$JobId";
+}
+
+sub CreateDir($)
+{
+  my ($self) = @_;
+  my $Dir = $self->GetDir();
+  File::Path::make_path($Dir, mode => 0775);
+  return $Dir;
+}
+
+sub RmTree($)
+{
+  my ($self) = @_;
+  my $Dir = $self->GetDir();
+  File::Path::remove_tree($Dir);
+}
+
 sub Status($;$)
 {
   my ($self, $NewStatus) = @_;
@@ -268,7 +292,6 @@ sub Restart($)
     return "This job is already " . $self->Status;
   }
 
-  my $JobDir = "$DataDir/jobs/" . $self->Id;
   my $FirstStep = 1;
   my $Steps = $self->Steps;
   my @SortedSteps = sort { $a->No <=> $b->No } @{$Steps->GetItems()};
@@ -281,7 +304,7 @@ sub Restart($)
       {
         # The first step contains the patch or test executable
         # so only delete its task folders
-        system("rm", "-rf", "$JobDir/" . $Step->No . "/" . $Task->No);
+        $Task->RmTree();
       }
       $Task->Status("queued");
       $Task->Started(undef);
@@ -289,7 +312,7 @@ sub Restart($)
       $Task->TestFailures(undef);
     }
     # Subsequent steps only contain files generated by the previous steps
-    system("rm", "-rf", "$JobDir/" . $Step->No) if (!$FirstStep);
+    $Step->RmTree() if (!$FirstStep);
     $FirstStep = undef;
     $Step->Status("queued");
   }
@@ -534,7 +557,7 @@ sub ScheduleOnHost($$$)
     if (@SortedSteps != 0)
     {
       my $Step = $SortedSteps[0];
-      $Step->HandleStaging($Job->GetKey());
+      $Step->HandleStaging();
       my $PrepareNextStep;
       my $Tasks = $Step->Tasks;
       $Tasks->AddFilter("Status", ["queued"]);
diff --git a/testbot/lib/WineTestBot/Steps.pm b/testbot/lib/WineTestBot/Steps.pm
index 8a9ad273..6f2b399e 100644
--- a/testbot/lib/WineTestBot/Steps.pm
+++ b/testbot/lib/WineTestBot/Steps.pm
@@ -33,6 +33,8 @@ a WineTestBot::Task object for each VM it should be run on.
 =cut
 
 use File::Copy;
+use File::Path;
+
 use WineTestBot::Config;
 use WineTestBot::WineTestBotObjects;
 
@@ -56,9 +58,31 @@ sub InitializeNew($$)
   $self->SUPER::InitializeNew($Collection);
 }
 
+sub GetDir($)
+{
+  my ($self) = @_;
+  my ($JobId, $StepNo) = @{$self->GetMasterKey()};
+  return "$DataDir/jobs/$JobId/$StepNo";
+}
+
+sub CreateDir($)
+{
+  my ($self) = @_;
+  my $Dir = $self->GetDir();
+  File::Path::make_path($Dir, mode => 0775);
+  return $Dir;
+}
+
+sub RmTree($)
+{
+  my ($self) = @_;
+  my $Dir = $self->GetDir();
+  File::Path::remove_tree($Dir);
+}
+
 sub HandleStaging($$)
 {
-  my ($self, $JobKey) = @_;
+  my ($self) = @_;
 
   if (! $self->InStaging)
   {
@@ -72,10 +96,8 @@ sub HandleStaging($$)
   }
   my $BaseName = $1;
   my $StagingFileName = "$DataDir/staging/$FileName";
-  my $FinalFileName = "$DataDir/jobs/$JobKey/" . $self->GetKey() .
-                      "/$BaseName";
-  mkdir "$DataDir/jobs/$JobKey";
-  mkdir "$DataDir/jobs/$JobKey/" . $self->GetKey();
+  my $StepDir = $self->CreateDir();
+  my $FinalFileName = "$StepDir/$BaseName";
   if (!move($StagingFileName, $FinalFileName))
   {
     return "Could not move the staging file: $!";
diff --git a/testbot/lib/WineTestBot/StepsTasks.pm b/testbot/lib/WineTestBot/StepsTasks.pm
index 73be78c6..33f99b0b 100644
--- a/testbot/lib/WineTestBot/StepsTasks.pm
+++ b/testbot/lib/WineTestBot/StepsTasks.pm
@@ -39,6 +39,19 @@ use vars qw(@ISA @EXPORT);
 require Exporter;
 @ISA = qw(WineTestBot::WineTestBotItem Exporter);
 
+sub GetStepDir($)
+{
+  my ($self) = @_;
+  my ($JobId, $_StepTaskId) = @{$self->GetMasterKey()};
+  return "$DataDir/jobs/$JobId/". $self->StepNo;
+}
+
+sub GetTaskDir($)
+{
+  my ($self) = @_;
+  return $self->GetStepDir() ."/". $self->TaskNo;
+}
+
 sub GetTitle($)
 {
   my ($self) = @_;
diff --git a/testbot/lib/WineTestBot/Tasks.pm b/testbot/lib/WineTestBot/Tasks.pm
index 32e41e43..62eff5d7 100644
--- a/testbot/lib/WineTestBot/Tasks.pm
+++ b/testbot/lib/WineTestBot/Tasks.pm
@@ -57,16 +57,37 @@ sub InitializeNew($$)
   $self->SUPER::InitializeNew($Collection);
 }
 
+sub GetDir($)
+{
+  my ($self) = @_;
+  my ($JobId, $StepNo, $TaskNo) = @{$self->GetMasterKey()};
+  return "$DataDir/jobs/$JobId/$StepNo/$TaskNo";
+}
+
+sub CreateDir($)
+{
+  my ($self) = @_;
+  my $Dir = $self->GetDir();
+  File::Path::make_path($Dir, mode => 0775);
+  return $Dir;
+}
+
+sub RmTree($)
+{
+  my ($self) = @_;
+  my $Dir = $self->GetDir();
+  File::Path::remove_tree($Dir);
+}
+
 sub _SetupTask($$)
 {
-  my ($VM, $Task) = @_;
+  my ($VM, $self) = @_;
 
-  # Capture Perl errors in the task's generic error log
-  my ($JobId, $StepNo, $TaskNo) = @{$Task->GetMasterKey()};
-  my $TaskDir = "$DataDir/jobs/$JobId/$StepNo/$TaskNo";
   # Remove the previous run's files if any
-  rmtree $TaskDir;
-  mkdir $TaskDir;
+  $self->RmTree();
+
+  # Capture Perl errors in the task's generic error log
+  my $TaskDir = $self->CreateDir();
   if (open(STDERR, ">>", "$TaskDir/err"))
   {
     # Make sure stderr still flushes after each print
@@ -136,8 +157,7 @@ sub UpdateStatus($$)
   {
     my ($JobId, $StepNo, $TaskNo) = @{$self->GetMasterKey()};
     my $OldUMask = umask(002);
-    my $TaskDir = "$DataDir/jobs/$JobId/$StepNo/$TaskNo";
-    mkdir $TaskDir;
+    my $TaskDir = $self->CreateDir();
     if (open TASKLOG, ">>$TaskDir/err")
     {
       print TASKLOG "Child process died unexpectedly\n";
diff --git a/testbot/web/GetFile.pl b/testbot/web/GetFile.pl
index 54d4c4b8..e4d54871 100644
--- a/testbot/web/GetFile.pl
+++ b/testbot/web/GetFile.pl
@@ -26,7 +26,7 @@ use WineTestBot::Config;
 use WineTestBot::Jobs;
 use WineTestBot::Steps;
 
-sub GetFile($$$$)
+sub GetFile($$$)
 {
   my ($Request, $JobKey, $StepKey) = @_;
 
@@ -53,7 +53,7 @@ sub GetFile($$$$)
     return !1;
   }
 
-  my $FileName = "$DataDir/jobs/$JobKey/$StepKey/" . $Step->FileName;
+  my $FileName = $Step->GetDir() ."/". $Step->FileName;
   if (! sysopen(FILE, $FileName, O_RDONLY))
   {
     return !1;
diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl
index 41a0fe35..e825da41 100644
--- a/testbot/web/JobDetails.pl
+++ b/testbot/web/JobDetails.pl
@@ -244,8 +244,7 @@ sub GenerateBody($)
   foreach my $Key (@$Keys)
   {
     my $Item = $self->{Collection}->GetItem($Key);
-    my $TaskDir = "$DataDir/jobs/" . $self->{JobId} . "/" . $Item->StepNo .
-                  "/" . $Item->TaskNo;
+    my $TaskDir = $Item->GetTaskDir();
     my $VM = $Item->VM;
     print "<h2><a name='k", $self->escapeHTML($Key), "'></a>" ,
           $self->escapeHTML($Item->GetTitle()), "</h2>\n";
@@ -465,8 +464,7 @@ sub GenerateDataCell($$$$$)
   }
   elsif ($PropertyName eq "FileName")
   {
-    my $FileName = "$DataDir/jobs/" . $self->{JobId} . "/" . $Item->StepNo .
-                   "/" . $Item->FileName;
+    my $FileName = $Item->GetStepDir() ."/". $Item->FileName;
     if (-r $FileName)
     {
       my $URI = "/GetFile.pl?JobKey=" . uri_escape($self->{JobId}) .
-- 
2.14.2