[PATCH] testbot/LogUtils: Return the ParseWineTestReport() results as a single object.

Francois Gouget fgouget at codeweavers.com
Fri Jan 24 02:52:35 CST 2020


Rename the variable holding it to $LogInfo for consistency with the
other functions as it plays the same role and may eventually carry the
same information.
Store the extra errors in the $LogInfo->{Extra} field and rename
_AddError() to _AddExtra() to match and avoid confusion with
_AddLogError().

Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
---
 testbot/bin/UpdateTaskLogs          |  10 +--
 testbot/bin/WineRunTask.pl          |  18 ++--
 testbot/bin/WineRunWineTest.pl      |  18 ++--
 testbot/lib/WineTestBot/LogUtils.pm | 125 ++++++++++++++++------------
 4 files changed, 95 insertions(+), 76 deletions(-)

diff --git a/testbot/bin/UpdateTaskLogs b/testbot/bin/UpdateTaskLogs
index ca9bf44a37..00a6f23223 100755
--- a/testbot/bin/UpdateTaskLogs
+++ b/testbot/bin/UpdateTaskLogs
@@ -190,19 +190,19 @@ sub BuildErrFile($$$$)
 
   my $TaskKey = Path2TaskKey($Dir);
 
-  my ($TestUnitCount, $TimeoutCount, $LogFailures, $LogErrors) = ParseWineTestReport("$Dir/$ReportName", $IsWineTest, $TaskTimedOut);
-  if (!defined $LogFailures and @$LogErrors == 1)
+  my $LogInfo = ParseWineTestReport("$Dir/$ReportName", $IsWineTest, $TaskTimedOut);
+  if (!defined $LogInfo->{Failures} and @{$LogInfo->{Extra}} == 1)
   {
-    return "$TaskKey: $LogErrors->[0]";
+    return "$TaskKey: $LogInfo->{Extra}->[0]";
   }
-  return undef if (!@$LogErrors);
+  return undef if (!@{$LogInfo->{Extra}});
 
   Debug("$TaskKey: Creating $ReportName.err\n");
   if (open(my $Log, ">", "$Dir/$ReportName.err"))
   {
     # Save the extra errors detected by ParseWineTestReport() in
     # $ReportName.err (see WineRunWineTest.pl).
-    print $Log "$_\n" for (@$LogErrors);
+    print $Log "$_\n" for (@{$LogInfo->{Extra}});
     close($Log);
     return undef;
   }
diff --git a/testbot/bin/WineRunTask.pl b/testbot/bin/WineRunTask.pl
index 62bd162eed..3044cd76bd 100755
--- a/testbot/bin/WineRunTask.pl
+++ b/testbot/bin/WineRunTask.pl
@@ -556,24 +556,24 @@ if ($TA->GetFile($RptFileName, "$TaskDir/$RptFileName"))
 {
   chmod 0664, "$TaskDir/$RptFileName";
 
-  my ($TestUnitCount, $TimeoutCount, $LogFailures, $LogErrors) = ParseWineTestReport("$TaskDir/$RptFileName", $IsWineTest, $TaskTimedOut);
-  $TaskTimedOut = 1 if ($TestUnitCount == $TimeoutCount);
-  if (!defined $LogFailures and @$LogErrors == 1)
+  my $LogInfo = ParseWineTestReport("$TaskDir/$RptFileName", $IsWineTest, $TaskTimedOut);
+  $TaskTimedOut = 1 if ($LogInfo->{TestUnitCount} == $LogInfo->{TimeoutCount});
+  if (!defined $LogInfo->{Failures} and @{$LogInfo->{Extra}} == 1)
   {
     # Could not open the file
     $NewStatus = 'boterror';
-    Error "$LogErrors->[0]\n";
-    LogTaskError("$LogErrors->[0]\n");
+    Error "$LogInfo->{Extra}->[0]\n";
+    LogTaskError("$LogInfo->{Extra}->[0]\n");
   }
   else
   {
-    # $LogFailures can legitimately be undefined in case of a timeout
-    $TaskFailures += $LogFailures || 0;
-    if (@$LogErrors and open(my $Log, ">", "$TaskDir/$RptFileName.err"))
+    # $LogInfo->{Failures} can legitimately be undefined in case of a timeout
+    $TaskFailures += $LogInfo->{Failures} || 0;
+    if (@{$LogInfo->{Extra}} and open(my $Log, ">", "$TaskDir/$RptFileName.err"))
     {
       # Save the extra errors detected by ParseWineTestReport() in
       # $RptFileName.err (see WineRunWineTest.pl).
-      print $Log "$_\n" for (@$LogErrors);
+      print $Log "$_\n" for (@{$LogInfo->{Extra}});
       close($Log);
     }
   }
diff --git a/testbot/bin/WineRunWineTest.pl b/testbot/bin/WineRunWineTest.pl
index fecbab0098..950661b70c 100755
--- a/testbot/bin/WineRunWineTest.pl
+++ b/testbot/bin/WineRunWineTest.pl
@@ -578,20 +578,20 @@ foreach my $Mission (@{$TaskMissions->{Missions}})
   {
     chmod 0664, "$TaskDir/$RptFileName";
 
-    my ($TestUnitCount, $TimeoutCount, $LogFailures, $LogErrors) = ParseWineTestReport("$TaskDir/$RptFileName", $Step->FileType eq "patch", $TaskTimedOut);
-    $TaskTimedOut = 1 if ($TestUnitCount == $TimeoutCount);
-    if (!defined $LogFailures and @$LogErrors == 1)
+    my $LogInfo = ParseWineTestReport("$TaskDir/$RptFileName", $Step->FileType eq "patch", $TaskTimedOut);
+    $TaskTimedOut = 1 if ($LogInfo->{TestUnitCount} == $LogInfo->{TimeoutCount});
+    if (!defined $LogInfo->{Failures} and @{$LogInfo->{Extra}} == 1)
     {
       # Could not open the file
       $NewStatus = 'boterror';
-      Error "$LogErrors->[0]\n";
-      LogTaskError("$LogErrors->[0]\n");
+      Error "$LogInfo->{Extra}->[0]\n";
+      LogTaskError("$LogInfo->{Extra}->[0]\n");
     }
     else
     {
-      # $LogFailures can legitimately be undefined in case of a timeout
-      $TaskFailures += $LogFailures || 0;
-      if (@$LogErrors and open(my $Log, ">", "$TaskDir/$RptFileName.err"))
+      # $LogInfo->{Failures} can legitimately be undefined in case of a timeout
+      $TaskFailures += $LogInfo->{Failures} || 0;
+      if (@{$LogInfo->{Extra}} and open(my $Log, ">", "$TaskDir/$RptFileName.err"))
       {
         # Save the extra errors detected by ParseWineTestReport() in
         # $RptFileName.err:
@@ -600,7 +600,7 @@ foreach my $Mission (@{$TaskMissions->{Missions}})
         #   if there are multiple .report files in the directory.
         # - The .err file can be moved to the latest directory next to the
         #   reference report.
-        print $Log "$_\n" for (@$LogErrors);
+        print $Log "$_\n" for (@{$LogInfo->{Extra}});
         close($Log);
       }
     }
diff --git a/testbot/lib/WineTestBot/LogUtils.pm b/testbot/lib/WineTestBot/LogUtils.pm
index 8cd3ae1178..d05a811d5c 100644
--- a/testbot/lib/WineTestBot/LogUtils.pm
+++ b/testbot/lib/WineTestBot/LogUtils.pm
@@ -232,56 +232,56 @@ sub _NewCurrentUnit($$)
   };
 }
 
-sub _AddError($$;$)
+sub _AddExtra($$;$)
 {
-  my ($Parser, $Error, $Cur) = @_;
+  my ($LogInfo, $Error, $Cur) = @_;
 
   $Error = "$Cur->{Dll}:$Cur->{Unit} $Error" if (defined $Cur);
-  push @{$Parser->{Errors}}, $Error;
-  $Parser->{Failures}++;
+  push @{$LogInfo->{Extra}}, $Error;
+  $LogInfo->{Failures}++;
 }
 
 sub _CheckUnit($$$$)
 {
-  my ($Parser, $Cur, $Unit, $Type) = @_;
+  my ($LogInfo, $Cur, $Unit, $Type) = @_;
 
   if ($Cur->{Units}->{$Unit} or $Cur->{Unit} eq "")
   {
-    $Parser->{IsWineTest} = 1;
+    $LogInfo->{IsWineTest} = 1;
   }
   # To avoid issuing many duplicate errors,
   # only report the first misplaced message.
-  elsif ($Parser->{IsWineTest} and !$Cur->{IsBroken})
+  elsif ($LogInfo->{IsWineTest} and !$Cur->{IsBroken})
   {
-    _AddError($Parser, "contains a misplaced $Type message for $Unit", $Cur);
+    _AddExtra($LogInfo, "contains a misplaced $Type message for $Unit", $Cur);
     $Cur->{IsBroken} = 1;
   }
 }
 
 sub _CheckSummaryCounter($$$$)
 {
-  my ($Parser, $Cur, $Field, $Type) = @_;
+  my ($LogInfo, $Cur, $Field, $Type) = @_;
 
   if ($Cur->{"Line$Field"} != 0 and $Cur->{"Summary$Field"} == 0)
   {
-    _AddError($Parser, "has unaccounted for $Type messages", $Cur);
+    _AddExtra($LogInfo, "has unaccounted for $Type messages", $Cur);
   }
   elsif ($Cur->{"Line$Field"} == 0 and $Cur->{"Summary$Field"} != 0)
   {
-    _AddError($Parser, "is missing some $Type messages", $Cur);
+    _AddExtra($LogInfo, "is missing some $Type messages", $Cur);
   }
 }
 
 sub _CloseTestUnit($$$)
 {
-  my ($Parser, $Cur, $Last) = @_;
+  my ($LogInfo, $Cur, $Last) = @_;
 
   # Verify the summary lines
   if (!$Cur->{IsBroken})
   {
-    _CheckSummaryCounter($Parser, $Cur, "Failures", "failure");
-    _CheckSummaryCounter($Parser, $Cur, "Todos", "todo");
-    _CheckSummaryCounter($Parser, $Cur, "Skips", "skip");
+    _CheckSummaryCounter($LogInfo, $Cur, "Failures", "failure");
+    _CheckSummaryCounter($LogInfo, $Cur, "Todos", "todo");
+    _CheckSummaryCounter($LogInfo, $Cur, "Skips", "skip");
   }
 
   # Note that the summary lines may count some failures twice
@@ -290,7 +290,7 @@ sub _CloseTestUnit($$$)
 
   if ($Cur->{UnitSize} > $MaxUnitSize)
   {
-    _AddError($Parser, "prints too much data ($Cur->{UnitSize} bytes)", $Cur);
+    _AddExtra($LogInfo, "prints too much data ($Cur->{UnitSize} bytes)", $Cur);
   }
   if (!$Cur->{IsBroken} and defined $Cur->{Rc})
   {
@@ -298,31 +298,31 @@ sub _CloseTestUnit($$$)
     # after the 'done' line (e.g. by subprocesses).
     if ($Cur->{LineFailures} != 0 and $Cur->{Rc} == 0)
     {
-      _AddError($Parser, "returned success despite having failures", $Cur);
+      _AddExtra($LogInfo, "returned success despite having failures", $Cur);
     }
-    elsif (!$Parser->{IsWineTest} and $Cur->{Rc} != 0)
+    elsif (!$LogInfo->{IsWineTest} and $Cur->{Rc} != 0)
     {
-      _AddError($Parser, "The test returned a non-zero exit code");
+      _AddExtra($LogInfo, "The test returned a non-zero exit code");
     }
-    elsif ($Parser->{IsWineTest} and $Cur->{LineFailures} == 0 and $Cur->{Rc} != 0)
+    elsif ($LogInfo->{IsWineTest} and $Cur->{LineFailures} == 0 and $Cur->{Rc} != 0)
     {
-      _AddError($Parser, "returned a non-zero exit code despite reporting no failures", $Cur);
+      _AddExtra($LogInfo, "returned a non-zero exit code despite reporting no failures", $Cur);
     }
   }
   # For executables TestLauncher's done line may not be recognizable.
-  elsif ($Parser->{IsWineTest} and !defined $Cur->{Rc})
+  elsif ($LogInfo->{IsWineTest} and !defined $Cur->{Rc})
   {
     if (!$Last)
     {
-      _AddError($Parser, "has no done line (or it is garbled)", $Cur);
+      _AddExtra($LogInfo, "has no done line (or it is garbled)", $Cur);
     }
-    elsif ($Last and !$Parser->{TaskTimedOut})
+    elsif ($Last and !$LogInfo->{TaskTimedOut})
     {
-      _AddError($Parser, "The report seems to have been truncated");
+      _AddExtra($LogInfo, "The report seems to have been truncated");
     }
   }
 
-  $Parser->{Failures} += $Cur->{LineFailures};
+  $LogInfo->{Failures} += $Cur->{LineFailures};
 }
 
 =pod
@@ -330,9 +330,29 @@ sub _CloseTestUnit($$$)
 
 =item C<ParseWineTestReport()>
 
-Parses a Wine test report and returns the number of failures and extra errors,
-a list of extra errors, and whether the test timed out.
+Returns a hashtable containing a summary of the WineTest report:
+=over
+
+=item IsWineTest
+True if this is a regular Wine test report, false if this is some other Windows
+binary.
+
+=item TaskTimedOut
+True if the overall task timed out waiting for the test to complete.
+
+=item TestUnitCount
+The number of test units.
+
+=item TimeoutCount
+The number of test units that timed out.
 
+=item Failures
+The number of failed tests.
+
+=item Extra
+An array containing the extra errors detected during the inconsistency check.
+
+=back
 =back
 =cut
 
@@ -343,18 +363,18 @@ sub ParseWineTestReport($$$)
   my $LogFile;
   if (!open($LogFile, "<", $FileName))
   {
-    my $BaseName = basename($FileName);
-    return (undef, undef, undef, ["Unable to open '$BaseName' for reading: $!"]);
+    my $LogName = basename($FileName);
+    return {Extra => ["Unable to open '$LogName' for reading: $!"]};
   }
 
-  my $Parser = {
+  my $LogInfo = {
     IsWineTest => $IsWineTest,
     TaskTimedOut => $TaskTimedOut,
 
     TestUnitCount => 0,
     TimeoutCount => 0,
     Failures => undef,
-    Errors => [],
+    Extra => [],
   };
 
   my $Cur = _NewCurrentUnit("", "");
@@ -366,9 +386,9 @@ sub ParseWineTestReport($$$)
       my ($Dll, $Unit, $Type) = ($1, $2, $3);
 
       # Close the previous test unit
-      _CloseTestUnit($Parser, $Cur, 0) if ($Cur->{Dll} ne "");
+      _CloseTestUnit($LogInfo, $Cur, 0) if ($Cur->{Dll} ne "");
       $Cur = _NewCurrentUnit($Dll, $Unit);
-      $Parser->{TestUnitCount}++;
+      $LogInfo->{TestUnitCount}++;
 
       # Recognize skipped messages in case we need to skip tests in the VMs
       $Cur->{Rc} = 0 if ($Type eq "skipped");
@@ -383,21 +403,21 @@ sub ParseWineTestReport($$$)
       }
       else
       {
-        _AddError($Parser, "Misplaced $SubUnit subtest\n");
+        _AddExtra($LogInfo, "Misplaced $SubUnit subtest\n");
       }
     }
     elsif (($Cur->{Unit} ne "" and
             $Line =~ /($Cur->{UnitsRE})\.c:\d+: Test (?:failed|succeeded inside todo block): /) or
            $Line =~ /^([_a-z0-9]+)\.c:\d+: Test (?:failed|succeeded inside todo block): /)
     {
-      _CheckUnit($Parser, $Cur, $1, "failure");
+      _CheckUnit($LogInfo, $Cur, $1, "failure");
       $Cur->{LineFailures}++;
     }
     elsif (($Cur->{Unit} ne "" and
             $Line =~ /($Cur->{UnitsRE})\.c:\d+: Test marked todo: /) or
            $Line =~ /^([_a-z0-9]+)\.c:\d+: Test marked todo: /)
     {
-      _CheckUnit($Parser, $Cur, $1, "todo");
+      _CheckUnit($LogInfo, $Cur, $1, "todo");
       $Cur->{LineTodos}++;
     }
     elsif (($Cur->{Unit} ne "" and
@@ -417,7 +437,7 @@ sub ParseWineTestReport($$$)
       # This also replaces a test summary line.
       $Cur->{Pids}->{0} = 1;
       $Cur->{SummaryFailures}++;
-      $Parser->{IsWineTest} = 1;
+      $LogInfo->{IsWineTest} = 1;
 
       $Cur->{LineFailures}++;
     }
@@ -433,7 +453,7 @@ sub ParseWineTestReport($$$)
         $Cur->{Pids}->{$Pid || 0} = 1;
         $Cur->{SummaryFailures}++;
       }
-      _CheckUnit($Parser, $Cur, $Unit, "unhandled exception");
+      _CheckUnit($LogInfo, $Cur, $Unit, "unhandled exception");
       $Cur->{LineFailures}++;
     }
     elsif (($Cur->{Unit} ne "" and
@@ -451,11 +471,11 @@ sub ParseWineTestReport($$$)
         $Cur->{SummaryFailures} += $Failures;
         $Cur->{SummaryTodos} += $Todos;
         $Cur->{SummarySkips} += $Skips;
-        $Parser->{IsWineTest} = 1;
+        $LogInfo->{IsWineTest} = 1;
       }
       else
       {
-        _CheckUnit($Parser, $Cur, $Unit, "test summary") if ($Todos or $Failures);
+        _CheckUnit($LogInfo, $Cur, $Unit, "test summary") if ($Todos or $Failures);
       }
     }
     elsif (($Cur->{Dll} ne "" and
@@ -464,17 +484,17 @@ sub ParseWineTestReport($$$)
     {
       my ($Dll, $Unit, $Pid, $Rc) = ($1, $2, $3, $4);
 
-      if ($Parser->{IsWineTest} and ($Dll ne $Cur->{Dll} or $Unit ne $Cur->{Unit}))
+      if ($LogInfo->{IsWineTest} and ($Dll ne $Cur->{Dll} or $Unit ne $Cur->{Unit}))
       {
         # First close the current test unit taking into account
         # it may have been polluted by the new one.
         $Cur->{IsBroken} = 1;
-        _CloseTestUnit($Parser, $Cur, 0);
+        _CloseTestUnit($LogInfo, $Cur, 0);
 
         # Then switch to the new one, warning it's missing a start line,
         # and that its results may be inconsistent.
         ($Cur->{Dll}, $Cur->{Unit}) = ($Dll, $Unit);
-        _AddError($Parser, "had no start line (or it is garbled)", $Cur);
+        _AddExtra($LogInfo, "had no start line (or it is garbled)", $Cur);
         $Cur->{IsBroken} = 1;
       }
 
@@ -482,9 +502,9 @@ sub ParseWineTestReport($$$)
       {
         # The done line will already be shown as a timeout (see JobDetails)
         # so record the failure but don't add an error message.
-        $Parser->{Failures}++;
+        $LogInfo->{Failures}++;
         $Cur->{IsBroken} = 1;
-        $Parser->{TimeoutCount}++;
+        $LogInfo->{TimeoutCount}++;
       }
       elsif ((!$Pid and !%{$Cur->{Pids}}) or
              ($Pid and !$Cur->{Pids}->{$Pid} and !$Cur->{Pids}->{0}))
@@ -492,12 +512,12 @@ sub ParseWineTestReport($$$)
         # The main summary line is missing
         if ($Rc & 0xc0000000)
         {
-          _AddError($Parser, sprintf("%s:%s crashed (%08x)", $Dll, $Unit, $Rc & 0xffffffff));
+          _AddExtra($LogInfo, sprintf("%s:%s crashed (%08x)", $Dll, $Unit, $Rc & 0xffffffff));
           $Cur->{IsBroken} = 1;
         }
-        elsif ($Parser->{IsWineTest} and !$Cur->{IsBroken})
+        elsif ($LogInfo->{IsWineTest} and !$Cur->{IsBroken})
         {
-          _AddError($Parser, "$Dll:$Unit has no test summary line (early exit of the main process?)");
+          _AddExtra($LogInfo, "$Dll:$Unit has no test summary line (early exit of the main process?)");
         }
       }
       elsif ($Rc & 0xc0000000)
@@ -511,12 +531,11 @@ sub ParseWineTestReport($$$)
       $Cur->{Rc} = $Rc;
     }
   }
-  $Cur->{IsBroken} = 1 if ($Parser->{TaskTimedOut});
-  _CloseTestUnit($Parser, $Cur, 1);
+  $Cur->{IsBroken} = 1 if ($LogInfo->{TaskTimedOut});
+  _CloseTestUnit($LogInfo, $Cur, 1);
   close($LogFile);
 
-  return ($Parser->{TestUnitCount}, $Parser->{TimeoutCount},
-          $Parser->{Failures}, $Parser->{Errors});
+  return $LogInfo;
 }
 
 
-- 
2.20.1



More information about the wine-devel mailing list