[PATCH 1/2] testbot: Move reference logs to the task and prepare for having more than one.

Francois Gouget fgouget at codeweavers.com
Mon Feb 3 20:50:07 CST 2020


The old reference report filename format was '<vmname>_<reportname>'
which means there could only be one per report. Each job should only
have one report of a given type per VM so add it to the reference
filename.
Also the use of an underscore as the separator made it hard to split
the components apart as both <vmname> and <reportname> can contain
underscores such as 'w1064v1809_fr' and 'win32_fr_FR.report'. Dashes
are a better choice since they are not allowed in either the VM name
or the report name.
So the new refeence filename format is '<vmname>-job<jobid>-<reportname>'.

Note:
* After applying this patch run UpdateTaskLogs to move and rename the
  existing reference reports.
* When downgrading to run code before this patch it's best to run
  UpdateTaskLogs --delete before downgrading and then UpdateTaskLogs
  after the downgrade.

Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
---

To simplify one can just apply both patches in the series and then just 
run UpdateTaskLogs once to upgrade the existing reports and logs.

It may be a good idea to run it as follows to help with debugging in 
case something goes wrong:

cd testbot/var && ../bin/UpdateTaskLogs --debug 2>&1 | tee UpdateTaskLogs.log


 testbot/bin/Janitor.pl                |   3 +-
 testbot/bin/UpdateTaskLogs            | 113 +++++++++++++++++++++++---
 testbot/bin/WineSendLog.pl            |   2 +-
 testbot/lib/WineTestBot/LogUtils.pm   |   5 +-
 testbot/lib/WineTestBot/StepsTasks.pm |   2 +-
 testbot/lib/WineTestBot/Tasks.pm      |   2 +-
 testbot/web/JobDetails.pl             |   4 +-
 7 files changed, 110 insertions(+), 21 deletions(-)

diff --git a/testbot/bin/Janitor.pl b/testbot/bin/Janitor.pl
index bf3989a712..dd13966018 100755
--- a/testbot/bin/Janitor.pl
+++ b/testbot/bin/Janitor.pl
@@ -314,7 +314,8 @@ if (opendir(my $dh, "$DataDir/latest"))
     my $Age = int((-M $FileName) + 0.5);
     my $TTL = $JobPurgeDays ? $JobPurgeDays - $Age : undef;
 
-    if ($Entry =~ /^([a-zA-Z0-9_]+)_(?:exe|win|wow)(?:32|64)[a-zA-Z0-9_]*\.report(?:\.err)?$/)
+    # Keep the regexp in sync with WineTestBot::Task::GetRefReportName()
+    if ($Entry =~ /^([a-zA-Z0-9_]+)-job[0-9]+-[a-zA-Z0-9_]+\.report(?:\.err)?$/)
     {
       # Keep the reference WineTest reports for all VMs even if they are
       # retired or scheduled for deletion.
diff --git a/testbot/bin/UpdateTaskLogs b/testbot/bin/UpdateTaskLogs
index 887061dab2..11dd6d31b9 100755
--- a/testbot/bin/UpdateTaskLogs
+++ b/testbot/bin/UpdateTaskLogs
@@ -261,18 +261,66 @@ sub DoUpdateLatestReport($$$)
   return $Rc;
 }
 
+sub MoveRefReport($$;$)
+{
+  my ($RefDir, $RefName, $NewDir) = @_;
+
+  my $RefPath = "$RefDir/$RefName";
+  return Delete("$RefPath.err", "orphaned") if (!-f $RefPath);
+
+  $NewDir ||= $RefDir;
+  my $NewName = $RefName;
+  if ($RefName =~ /^([a-zA-Z0-9_]+)_((?:exe|win|wow)(?:32|64)[a-zA-Z0-9_]*\.report(?:\.err)?)$/)
+  {
+    # We don't know which job generated this reference log.
+    # So use a fixed, arbitrary JobId.
+    $NewName = "$1-job000000-$2";
+  }
+
+  my $NewPath = "$NewDir/$NewName";
+  return 0 if ($RefPath eq $NewPath);
+
+  my $Rc = 0;
+  my $TaskKey = Path2TaskKey($NewDir);
+  if (-f $NewPath and -M $NewPath <= -M $RefPath)
+  {
+    # A WineTest job has probably completed after the upgrade already
+    $Rc += Delete($RefPath);
+    $Rc += Delete("$RefPath.err");
+    return $Rc;
+  }
+
+  if (-f $NewPath)
+  {
+    Error "Could not move '$RefName' because '$NewName' already exists and is older!\n";
+    return 1;
+  }
+
+  my $RelRefDir = ($RefDir eq $NewDir) ? "" : "../";
+  Debug("$TaskKey: $RelRefDir$RefName* -> $NewName\n");
+  foreach my $Suffix ("", ".err")
+  {
+    next if (!-f "$RefPath$Suffix");
+    next if (rename("$RefPath$Suffix", "$NewPath$Suffix"));
+    Error "Could not move '$RefName$Suffix' to '$NewName$Suffix' for $TaskKey: $!\n";
+    $Rc = 1;
+  }
+
+  return $Rc;
+}
+
 sub ProcessTaskLogs($$$)
 {
   my ($Step, $Task, $CollectOnly) = @_;
 
   my $Rc = 0;
-  my $StepDir = $Step->GetDir();
+  my $ReportNames;
   my $TaskDir = $Task->GetDir();
 
-  if (($OptDelete or $OptRebuild) and !$CollectOnly)
+  if (!$CollectOnly)
   {
-    # Save / delete the task's reference reports... all of them,
-    # even if they were not supposed to exist (e.g. failed builds).
+    # Upgrade the naming scheme of the task's old reference reports,
+    # delete those that are not meant to exist.
     my ($ErrMessage, $ReportNames, $_TaskMissions) = $Task->GetReportNames();
     if ($ErrMessage)
     {
@@ -280,9 +328,30 @@ sub ProcessTaskLogs($$$)
       $Rc = 1;
     }
     foreach my $ReportName (@$ReportNames)
+    {
+      my $StepReportName = $Task->VM->Name ."_$ReportName";
+      my $StepReportPath = $Step->GetFullFileName($StepReportName);
+      if (-f "$TaskDir/$ReportName")
+      {
+        $Rc += MoveRefReport(dirname($StepReportPath), $StepReportName, $TaskDir);
+        # The old WineTest reference reports for Windows tasks may end up being
+        # duplicated: one copy in the build step and another in the step for
+        # the task. MoveRefReport() moved the latter, now ensure Delete() can
+        # remove the former.
+        $StepReportPath = $Step->GetFullFileName($StepReportName);
+      }
+      $Rc += Delete($StepReportPath, "unneeded");
+    }
+  }
+
+  if (($OptDelete or $OptRebuild) and !$CollectOnly)
+  {
+    # Save / delete the task's reference reports... all of them,
+    # even if they were not supposed to exist (e.g. failed builds).
+    foreach my $ReportName (@$ReportNames)
     {
       my $RefReportName = $Task->GetRefReportName($ReportName);
-      my $RefReportPath = "$StepDir/$RefReportName";
+      my $RefReportPath = "$TaskDir/$RefReportName";
 
       if (-f $RefReportPath or -f "$RefReportPath.err")
       {
@@ -290,7 +359,7 @@ sub ProcessTaskLogs($$$)
         {
           # (Re)Build the .err file before adding the reference report to
           # latest/.
-          my $ErrMessage = BuildErrFile($StepDir, $RefReportName, 1, 0);
+          my $ErrMessage = BuildErrFile($TaskDir, $RefReportName, 1, 0);
           if (defined $ErrMessage)
           {
             Error "$ErrMessage\n";
@@ -315,7 +384,7 @@ sub ProcessTaskLogs($$$)
     {
       next if ($ReportName !~ /\.report$/);
       my $RefReportName = $Task->GetRefReportName($ReportName);
-      next if (-f "$StepDir/$RefReportName");
+      next if (-f "$TaskDir/$RefReportName");
 
       my $LatestReportPath = $LatestReports{$RefReportName};
       if (!defined $LatestReportPath)
@@ -328,9 +397,9 @@ sub ProcessTaskLogs($$$)
         Debug(TaskKeyStr($Task) .": Snapshotting $RefReportName from ". Path2TaskKey($LatestReportPath) ."\n");
         foreach my $Suffix ("", ".err")
         {
-          unlink "$StepDir/$RefReportName$Suffix";
+          unlink "$TaskDir/$RefReportName$Suffix";
           if (-f "$LatestReportPath$Suffix" and
-              !link("$LatestReportPath$Suffix", "$StepDir/$RefReportName$Suffix"))
+              !link("$LatestReportPath$Suffix", "$TaskDir/$RefReportName$Suffix"))
           {
             Error "Could not link '$RefReportName$Suffix': $!\n";
             $Rc = 1;
@@ -390,18 +459,38 @@ sub ProcessLatestReports()
   my $Rc = 0;
   my $LatestGlob = "$DataDir/latest/*.report";
 
-  # Perform cleanups and updates
+  # Delete or rename the old reference reports
   foreach my $LatestReportPath (glob("$LatestGlob $LatestGlob.err"))
   {
     my $RefReportName = basename($LatestReportPath);
-    # Keep the regexp in sync with WineTestBot::Task::GetRefReportName()
     next if ($RefReportName !~ /^([a-zA-Z0-9_]+_[a-zA-Z0-9_]+\.report)(?:\.err)?$/);
     $RefReportName = $1; # untaint
     $LatestReportPath = "$DataDir/latest/$RefReportName";
 
     if ($OptDelete or $OptRebuild)
     {
-      # Delete the reports that should be deleted / rebuilt
+      $Rc += Delete($LatestReportPath);
+      $Rc += Delete("$LatestReportPath.err");
+    }
+    else
+    {
+      # MoveRefReport() also renames .err files
+      $Rc += MoveRefReport("$DataDir/latest", $RefReportName);
+    }
+  }
+
+  # Then perform cleanups and rebuild missing files
+  foreach my $LatestReportPath (glob("$LatestGlob $LatestGlob.err"))
+  {
+    my $RefReportName = basename($LatestReportPath);
+    # Keep the regexp in sync with WineTestBot::Task::GetRefReportName()
+    next if ($RefReportName !~ /^([a-zA-Z0-9_]+-job[0-9]+-[a-zA-Z0-9_]+\.report)(?:\.err)?$/);
+    $RefReportName = $1; # untaint
+    $LatestReportPath = "$DataDir/latest/$RefReportName";
+
+    if ($OptDelete or $OptRebuild)
+    {
+      # These can be rebuilt from the task reports
       $Rc += Delete($LatestReportPath);
       $Rc += Delete("$LatestReportPath.err");
     }
diff --git a/testbot/bin/WineSendLog.pl b/testbot/bin/WineSendLog.pl
index 44d7255e71..e0cd585b11 100755
--- a/testbot/bin/WineSendLog.pl
+++ b/testbot/bin/WineSendLog.pl
@@ -293,7 +293,7 @@ EOF
       next if (!$LogInfo->{ErrCount});
 
       my $AllNew;
-      my $RefReportPath = $StepTask->GetFullFileName($StepTask->GetRefReportName($LogName));
+      my $RefReportPath = "$TaskDir/". $StepTask->GetRefReportName($LogName);
       TagNewErrors($RefReportPath, $LogInfo);
       if ($LogInfo->{NoRef})
       {
diff --git a/testbot/lib/WineTestBot/LogUtils.pm b/testbot/lib/WineTestBot/LogUtils.pm
index 5b567bc23b..36f367497b 100644
--- a/testbot/lib/WineTestBot/LogUtils.pm
+++ b/testbot/lib/WineTestBot/LogUtils.pm
@@ -1070,11 +1070,10 @@ sub SnapshotLatestReport($$)
   {
     next if (!-f "$DataDir/latest/$RefReportName$Suffix");
 
-    # FIXME: The reference reports are stored at the step level!
     if (!link("$DataDir/latest/$RefReportName$Suffix",
-              "$TaskDir/../$RefReportName$Suffix"))
+              "$TaskDir/$RefReportName$Suffix"))
     {
-      push @ErrMessages, "Could not create the '../$RefReportName$Suffix' link: $!";
+      push @ErrMessages, "Could not create the '$RefReportName$Suffix' link: $!";
     }
   }
 
diff --git a/testbot/lib/WineTestBot/StepsTasks.pm b/testbot/lib/WineTestBot/StepsTasks.pm
index bad0e24375..f65e6a6327 100644
--- a/testbot/lib/WineTestBot/StepsTasks.pm
+++ b/testbot/lib/WineTestBot/StepsTasks.pm
@@ -71,7 +71,7 @@ sub GetTaskDir($)
 sub GetRefReportName($$)
 {
   my ($self, $ReportName) = @_;
-  return $self->VM->Name ."_$ReportName";
+  return $self->VM->Name ."-job000000-$ReportName";
 }
 
 sub GetTitle($)
diff --git a/testbot/lib/WineTestBot/Tasks.pm b/testbot/lib/WineTestBot/Tasks.pm
index 1a46f0d57c..7b9ac9b816 100644
--- a/testbot/lib/WineTestBot/Tasks.pm
+++ b/testbot/lib/WineTestBot/Tasks.pm
@@ -141,7 +141,7 @@ sub GetReportNames($)
 sub GetRefReportName($$)
 {
   my ($self, $ReportName) = @_;
-  return $self->VM->Name ."_$ReportName";
+  return $self->VM->Name ."-job000000-$ReportName";
 }
 
 sub _SetupTask($$)
diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl
index f1ffd498ed..3dce91595d 100644
--- a/testbot/web/JobDetails.pl
+++ b/testbot/web/JobDetails.pl
@@ -494,7 +494,7 @@ EOF
         if ($LogInfo->{ErrCount})
         {
           # Identify new errors in test reports
-          my $RefReportPath = $StepTask->GetFullFileName($StepTask->GetRefReportName($MoreInfo->{Full}));
+          my $RefReportPath = "$TaskDir/". $StepTask->GetRefReportName($MoreInfo->{Full});
           TagNewErrors($RefReportPath, $LogInfo);
         }
       }
@@ -563,7 +563,7 @@ EOF
         if ($LogName =~ /\.report$/)
         {
           # For test reports try to identify the new errors
-          my $RefReportPath = $StepTask->GetFullFileName($StepTask->GetRefReportName($LogName));
+          my $RefReportPath = "$TaskDir/". $StepTask->GetRefReportName($LogName);
           TagNewErrors($RefReportPath, $LogInfo);
         }
 
-- 
2.20.1




More information about the wine-devel mailing list