[PATCH] testbot/LogUtils: Return the log errors as a single object.

[Francois Gouget]

The $LogInfo structure is more extensible and can carry additional
information such as the total number of errors (ErrCount) and the total
number of new errors (NewCount).
Adjust _AddErrorGroup(), TagNewErrors() and _DumpErrors() to take the
new $LogInfo structure returned by GetLogErrors() instead of the
($Groups, $Errors) tuple.

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
 testbot/bin/WineSendLog.pl          |  26 +++---
 testbot/lib/WineTestBot/LogUtils.pm | 129 +++++++++++++++++-----------
 testbot/web/JobDetails.pl           |  21 +++--
 3 files changed, 103 insertions(+), 73 deletions(-)

diff --git a/testbot/bin/WineSendLog.pl b/testbot/bin/WineSendLog.pl
index 6704f9828..fdf7e2409 100755
--- a/testbot/bin/WineSendLog.pl
+++ b/testbot/bin/WineSendLog.pl
@@ -208,18 +208,18 @@ EOF
     $JobErrors->{$Key}->{LogNames} = $LogNames;
     foreach my $LogName (@$LogNames)
     {
-      my ($Groups, $Errors) = GetLogErrors("$TaskDir/$LogName");
-      next if (!$Groups or !@$Groups);
+      my $LogInfo = GetLogErrors("$TaskDir/$LogName");
+      next if (!$LogInfo->{ErrCount});
       $JobErrors->{$Key}->{HasErrors} = 1;
-      $JobErrors->{$Key}->{$LogName}->{Groups} = $Groups;
-      $JobErrors->{$Key}->{$LogName}->{Errors} = $Errors;
+      $JobErrors->{$Key}->{$LogName} = $LogInfo;
 
       print $Sendmail "\n=== ", GetTitle($StepTask, $LogName), " ===\n";
 
-      foreach my $GroupName (@$Groups)
+      foreach my $GroupName (@{$LogInfo->{ErrGroupNames}})
       {
         print $Sendmail ($GroupName ? "\n$GroupName:\n" : "\n");
-        print $Sendmail "$_\n" for (@{$Errors->{$GroupName}->{Errors}});
+        my $Group = $LogInfo->{ErrGroups}->{$GroupName};
+        print $Sendmail "$_\n" for (@{$Group->{Errors}});
       }
     }
   }
@@ -288,14 +288,14 @@ EOF
 
     foreach my $LogName (@{$JobErrors->{$Key}->{LogNames}})
     {
-      my $LogErrors = $JobErrors->{$Key}->{$LogName};
+      my $LogInfo = $JobErrors->{$Key}->{$LogName};
       # Skip if there are no errors
-      next if (!$LogErrors->{Groups});
+      next if (!$LogInfo->{ErrCount});
 
       my $AllNew;
       my $RefReportPath = $StepTask->GetFullFileName($StepTask->GetRefReportName($LogName));
-      my $NewCount = TagNewErrors($RefReportPath, $LogErrors->{Groups}, $LogErrors->{Errors});
-      if (!defined $NewCount)
+      TagNewErrors($RefReportPath, $LogInfo);
+      if (!defined $LogInfo->{NewCount})
       {
         # Test reports should have reference WineTest results and if not
         # reporting the errors as new would cause false positives.
@@ -304,7 +304,7 @@ EOF
         # Build logs don't have reference logs so for them every error is new.
         $AllNew = 1;
       }
-      elsif (!$NewCount)
+      elsif (!$LogInfo->{NewCount})
       {
         # There is no new error
         next;
@@ -312,9 +312,9 @@ EOF
 
       push @Messages, "\n=== ". GetTitle($StepTask, $LogName) ." ===\n";
 
-      foreach my $GroupName (@{$LogErrors->{Groups}})
+      foreach my $GroupName (@{$LogInfo->{ErrGroupNames}})
       {
-        my $Group = $LogErrors->{Errors}->{$GroupName};
+        my $Group = $LogInfo->{ErrGroups}->{$GroupName};
         next if (!$AllNew and !$Group->{NewCount});
 
         push @Messages, ($GroupName ? "\n$GroupName:\n" : "\n");
diff --git a/testbot/lib/WineTestBot/LogUtils.pm b/testbot/lib/WineTestBot/LogUtils.pm
index e1a8176ac..815e0d8db 100644
--- a/testbot/lib/WineTestBot/LogUtils.pm
+++ b/testbot/lib/WineTestBot/LogUtils.pm
@@ -658,33 +658,35 @@ sub GetLogLabel($)
 }
 
 
-sub _DumpErrors($$$)
+sub _DumpErrors($$)
 {
-  my ($Label, $Groups, $Errors) = @_;
+  my ($Label, $LogInfo) = @_;
 
-  print STDERR "$Label:\n";
-  print STDERR "  Groups=", scalar(@$Groups), " [", join(",", @$Groups), "]\n";
-  my @ErrorKeys = sort keys %$Errors;
+  print STDERR "$Label: ", join(" ", keys %$LogInfo), "\n";
+  my $GroupNames = $LogInfo->{ErrGroupNames};
+  print STDERR "  Groups=", scalar(@$GroupNames), " [", join(",", @$GroupNames), "]\n";
+  my @ErrorKeys = sort keys %{$LogInfo->{Groups}};
   print STDERR "  Errors=", scalar(@ErrorKeys), " [", join(",", @ErrorKeys), "]\n";
-  foreach my $GroupName (@$Groups)
+  foreach my $GroupName (@$GroupNames)
   {
     print STDERR "  [$GroupName]\n";
-    print STDERR "    [$_]\n" for (@{$Errors->{$GroupName}->{Errors}});
+    my $Group = $LogInfo->{Groups}->{$GroupName};
+    print STDERR "    [$_]\n" for (@{$Group->{Errors}});
   }
 }
 
-sub _AddErrorGroup($$$)
+sub _AddErrorGroup($$)
 {
-  my ($Groups, $Errors, $GroupName) = @_;
+  my ($LogInfo, $GroupName) = @_;
 
   # In theory the error group names are all unique. But, just in case, make
-  # sure we don't overwrite $Errors->{$GroupName}.
-  if (!$Errors->{$GroupName})
+  # sure we don't overwrite $LogInfo->{ErrGroups}->{$GroupName}.
+  if (!$LogInfo->{ErrGroups}->{$GroupName})
   {
-    push @$Groups, $GroupName;
-    $Errors->{$GroupName} = {Errors => []};
+    push @{$LogInfo->{ErrGroupNames}}, $GroupName;
+    $LogInfo->{ErrGroups}->{$GroupName} = { Errors => [] };
   }
-  return $Errors->{$GroupName};
+  return $LogInfo->{ErrGroups}->{$GroupName};
 }
 
 =pod
@@ -696,8 +698,24 @@ Analyzes the specified log and associated error file to filter out unimportant
 messages and only return the errors, split by module (for Wine reports that's
 per dll / program being tested).
 
-Returns a list of modules containing errors, and a hashtable containing the list of errors for each module.
+Returns a hashtable containing:
+=over
+
+=item ErrCount
+The number of errors. This is undefined if no log file was found.
+
+=item ErrGroupNames
+An array containing the names of all the error groups.
+
+=item ErrGroups
+A hashtable indexed by the error group name. Each entry contains:
+
+=over
+=item Errors
+An array containing the error messages.
+=back
 
+=back
 =back
 =cut
 
@@ -716,12 +734,14 @@ sub GetLogErrors($)
     $GetCategory = \&GetLogLineCategory;
   }
 
-  my $NoLog = 1;
-  my $Groups = [];
-  my $Errors = {};
+  my $LogInfo = {
+    ErrCount => undef, # until we open a log
+    ErrGroupNames => [],
+    ErrGroups => {},
+  };
   if (open(my $LogFile, "<", $LogFileName))
   {
-    $NoLog = 0;
+    $LogInfo->{ErrCount} ||= 0;
     my $CurrentModule = "";
     my $CurrentGroup;
     foreach my $Line (<$LogFile>)
@@ -743,22 +763,23 @@ sub GetLogErrors($)
       }
       if (!$CurrentGroup)
       {
-        $CurrentGroup = _AddErrorGroup($Groups, $Errors, $CurrentModule);
+        $CurrentGroup = _AddErrorGroup($LogInfo, $CurrentModule);
       }
       push @{$CurrentGroup->{Errors}},  $Line;
+      $LogInfo->{ErrCount}++;
     }
     close($LogFile);
   }
   elsif (-f $LogFileName)
   {
-    $NoLog = 0;
-    my $Group = _AddErrorGroup($Groups, $Errors, "TestBot errors");
-    $Group->{Errors} = ["Could not open '". basename($LogFileName) ."' for reading: $!"];
+    my $Group = _AddErrorGroup($LogInfo, "TestBot errors");
+    push @{$Group->{Errors}}, "Could not open '". basename($LogFileName) ."' for reading: $!";
+    $LogInfo->{ErrCount}++;
   }
 
   if (open(my $LogFile, "<", "$LogFileName.err"))
   {
-    $NoLog = 0;
+    $LogInfo->{ErrCount} ||= 0;
     # Add the related extra errors
     my $CurrentGroup;
     foreach my $Line (<$LogFile>)
@@ -769,20 +790,21 @@ sub GetLogErrors($)
         # Note: $GroupName must not depend on the previous content as this
         #       would break diffs.
         my $GroupName = $IsReport ? "Report errors" : "Task errors";
-        $CurrentGroup = _AddErrorGroup($Groups, $Errors, $GroupName);
+        $CurrentGroup = _AddErrorGroup($LogInfo, $GroupName);
       }
       push @{$CurrentGroup->{Errors}}, $Line;
+      $LogInfo->{ErrCount}++;
     }
     close($LogFile);
   }
   elsif (-f "$LogFileName.err")
   {
-    $NoLog = 0;
-    my $Group = _AddErrorGroup($Groups, $Errors, "TestBot errors");
-    $Group->{Errors} = ["Could not open '". basename($LogFileName) .".err' for reading: $!"];
+    my $Group = _AddErrorGroup($LogInfo, "TestBot errors");
+    push @{$Group->{Errors}}, "Could not open '". basename($LogFileName) .".err' for reading: $!";
+    $LogInfo->{ErrCount}++;
   }
 
-  return $NoLog ? (undef, undef) : ($Groups, $Errors);
+  return $LogInfo;
 }
 
 sub _DumpDiff($$)
@@ -849,42 +871,52 @@ sub _GetLineKey($)
 
 Compares the specified errors to the reference report to identify new errors.
 
-Adds two fields to each error group:
+The $LogInfo structure is augmented with the following fields:
 =over
 
 =item NewCount
-A count of the new errors.
+The total number of new errors or undef if the reference log could not be read.
+
+=item ErrGroups
+=over
+
+=item NewCount
+A count of the group's new errors.
 
 =item IsNew
-An array where entries are set to true to identify new errors.
+An array indicating which entries in the group's Errors array are new
+or undef if there are no new errors in this group.
 
 =back
 
-Returns a global count of the new errors. Returns undef if there is no
-reference log to identify the new errors.
-
+=back
 =back
 =cut
 
-sub TagNewErrors($$$)
+sub TagNewErrors($$)
 {
-  my ($RefLogPath, $Groups, $Errors) = @_;
+  my ($RefLogPath, $LogInfo) = @_;
 
-  return 0 if (!$Groups or !@$Groups);
+  if (!$LogInfo->{ErrCount})
+  {
+    $LogInfo->{NewCount} = 0;
+    return;
+  }
 
-  my ($RefGroups, $RefErrors) = GetLogErrors($RefLogPath);
-  return undef if (!$RefGroups);
+  my $RefInfo = GetLogErrors($RefLogPath);
+  # Don't tag the errors as new if there is no reference log
+  return if (!defined $RefInfo->{ErrCount});
 
-  my $NewCount = 0;
-  foreach my $GroupName (@$Groups)
+  $LogInfo->{NewCount} = 0;
+  foreach my $GroupName (@{$LogInfo->{ErrGroupNames}})
   {
-    my $Group = $Errors->{$GroupName};
+    my $Group = $LogInfo->{ErrGroups}->{$GroupName};
     $Group->{NewCount} = 0;
 
-    if ($RefErrors->{$GroupName})
+    my $RefGroup = $RefInfo->{ErrGroups}->{$GroupName};
+    if ($RefGroup)
     {
-      my $Diff = Algorithm::Diff->new($RefErrors->{$GroupName}->{Errors},
-                                      $Group->{Errors},
+      my $Diff = Algorithm::Diff->new($RefGroup->{Errors}, $Group->{Errors},
                                       { keyGen => \&_GetLineKey });
       my $ErrIndex = 0;
       while ($Diff->Next())
@@ -909,6 +941,7 @@ sub TagNewErrors($$$)
     }
     else
     {
+      # All errors in this group are new
       $Group->{NewCount} = @{$Group->{Errors}};
       if ($Group->{NewCount})
       {
@@ -918,10 +951,8 @@ sub TagNewErrors($$$)
         }
       }
     }
-    $NewCount += $Group->{NewCount};
+    $LogInfo->{NewCount} += $Group->{NewCount};
   }
-
-  return $NewCount;
 }
 
 1;
diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl
index ee4013a1b..76de1bb23 100644
--- a/testbot/web/JobDetails.pl
+++ b/testbot/web/JobDetails.pl
@@ -500,21 +500,20 @@ EOF
       #
 
       # Figure out which logs / reports actually have errors
-      my $LogSummaries;
+      my $LogInfos;
       foreach my $LogName (@{$MoreInfo->{Logs}})
       {
         next if ($LogName =~ /^old_/);
-        my ($Groups, $Errors) = GetLogErrors("$TaskDir/$LogName");
-        next if (!$Groups or !@$Groups);
-        $LogSummaries->{$LogName}->{Groups} = $Groups;
-        $LogSummaries->{$LogName}->{Errors} = $Errors;
+        my $LogInfo = GetLogErrors("$TaskDir/$LogName");
+        next if (!$LogInfo->{ErrCount});
+        $LogInfos->{$LogName} = $LogInfo;
       }
-      my $ShowLogName = ($ReportCount > 1 or scalar(keys %$LogSummaries) > 1);
+      my $ShowLogName = ($ReportCount > 1 or scalar(keys %$LogInfos) > 1);
 
       my $LogIsEmpty = 1;
       foreach my $LogName (@{$MoreInfo->{Logs}})
       {
-        next if (!$LogSummaries->{$LogName});
+        next if (!$LogInfos->{$LogName});
         $LogIsEmpty = 0;
 
         if ($ShowLogName)
@@ -524,21 +523,21 @@ EOF
           print "<div class='HrTitle'>$Label<div class='HrLine'></div></div>\n";
         }
 
-        my $Summary = $LogSummaries->{$LogName};
+        my $LogInfo = $LogInfos->{$LogName};
         if ($LogName =~ /\.report$/)
         {
           # For test reports try to identify the new errors
           my $RefReportPath = $StepTask->GetFullFileName($StepTask->GetRefReportName($LogName));
-          TagNewErrors($RefReportPath, $Summary->{Groups}, $Summary->{Errors});
+          TagNewErrors($RefReportPath, $LogInfo);
         }
 
-        foreach my $GroupName (@{$Summary->{Groups}})
+        foreach my $GroupName (@{$LogInfo->{ErrGroupNames}})
         {
           print "<div class='LogDllName'>$GroupName</div>\n" if ($GroupName);
 
           print "<pre><code>";
           my $ErrIndex = 0;
-          my $Group = $Summary->{Errors}->{$GroupName};
+          my $Group = $LogInfo->{ErrGroups}->{$GroupName};
           foreach my $Line (@{$Group->{Errors}})
           {
             if ($Group->{IsNew}->[$ErrIndex])
-- 
2.20.1