Francois Gouget : testbot/WineRun*: Don' t duplicate errors and factorize the log parsing.

[Alexandre Julliard]

Module: tools
Branch: master
Commit: 6a2445a55a0021bb193be98fd4a63d6772ca8796
URL:    https://source.winehq.org/git/tools.git/?a=commit;h=6a2445a55a0021bb193be98f...

Author: Francois Gouget 
Date:   Thu Jun 21 14:31:18 2018 +0200

testbot/WineRun*: Don't duplicate errors and factorize the log parsing.

Move the task log parsing functions to LogUtils.pm so the WineRun*.pl
scripts can share them.
Also stop copying the 'Build: *' result lines from the log to the err
file. It's up to the GUI to present all the relevant lines when only
showing the log summary (which it does now).
Finally, there is nothing for the script to do if the patch fails to
apply. So even if we did not get the full log or the task timed out
somehow, the status of the task should be 'badpatch' and the other
errors don't matter.

Signed-off-by: Francois Gouget 
Signed-off-by: Alexandre Julliard

---

testbot/bin/WineRunBuild.pl         | 49 ++++++++++-------------
 testbot/bin/WineRunReconfig.pl      | 49 +++++++----------------
 testbot/lib/WineTestBot/LogUtils.pm | 77 +++++++++++++++++++++++++++++++++++++
 3 files changed, 111 insertions(+), 64 deletions(-)

diff --git a/testbot/bin/WineRunBuild.pl b/testbot/bin/WineRunBuild.pl
index 1bc6cff..60af32c 100755
--- a/testbot/bin/WineRunBuild.pl
+++ b/testbot/bin/WineRunBuild.pl
@@ -46,6 +46,7 @@ use WineTestBot::Jobs;
 use WineTestBot::PatchUtils;
 use WineTestBot::VMs;
 use WineTestBot::Log;
+use WineTestBot::LogUtils;
 use WineTestBot::Engine::Notify;

@@ -391,39 +392,29 @@ if (!defined $TA->Wait($Pid, $Task->Timeout, 60))
 Debug(Elapsed($Start), " Retrieving 'Build.log'\n");
 if ($TA->GetFile("Build.log", "$TaskDir/log"))
 {
-  if (open(my $LogFile, "<", "$TaskDir/log"))
+  my $Result = ParseTaskLog("$TaskDir/log", "Build");
+  if ($Result eq "ok")
   {
-    # Collect and analyze the 'Build:' status line(s)
-    $ErrMessage ||= "";
-    foreach my $Line (<$LogFile>)
-    {
-      chomp($Line);
-      next if ($Line !~ /^Build: (.*)$/);
-      if ($1 eq "ok")
-      {
-        # We must have gotten the full log and the build did succeed.
-        # So forget any prior error.
-        $NewStatus = "completed";
-        $TAError = $ErrMessage = undef;
-      }
-      else
-      {
-        $NewStatus = ($1 eq "Patch failed to apply") ? "badpatch" : "badbuild";
-        # Collect all the build errors (32 bit, 64 bit, etc)
-        $ErrMessage .= "$1\n";
-      }
-    }
-    close($LogFile);
-
-    if (!defined $NewStatus)
-    {
-      $NewStatus = "badbuild";
-      $ErrMessage = "Missing build status line\n";
-    }
+    # We must have gotten the full log and the build did succeed.
+    # So forget any prior error.
+    $NewStatus = "completed";
+    $TAError = $ErrMessage = undef;
+  }
+  elsif ($Result eq "badpatch")
+  {
+    # This too is conclusive enough to ignore other errors.
+    $NewStatus = "badpatch";
+    $TAError = $ErrMessage = undef;
+  }
+  elsif ($Result =~ s/^nolog://)
+  {
+    FatalError("$Result\n", "retry");
   }
   else
   {
-    FatalError("Unable to open the build log for reading: $!\n", "retry");
+    # If the result line is missing we probably already have an error message
+    # that explains why.
+    $NewStatus = "badbuild";
   }
 }
 elsif (!defined $TAError)
diff --git a/testbot/bin/WineRunReconfig.pl b/testbot/bin/WineRunReconfig.pl
index d04b780..2950799 100755
--- a/testbot/bin/WineRunReconfig.pl
+++ b/testbot/bin/WineRunReconfig.pl
@@ -45,6 +45,7 @@ use WineTestBot::Config;
 use WineTestBot::Jobs;
 use WineTestBot::VMs;
 use WineTestBot::Log;
+use WineTestBot::LogUtils;
 use WineTestBot::Engine::Notify;

@@ -373,45 +374,23 @@ if (!defined $TA->Wait($Pid, $Task->Timeout, 60))
 Debug(Elapsed($Start), " Retrieving 'Reconfig.log'\n");
 if ($TA->GetFile("Reconfig.log", "$TaskDir/log"))
 {
-  if (open(my $LogFile, "<", "$TaskDir/log"))
+  my $Result = ParseTaskLog("$TaskDir/log", "Reconfig");
+  if ($Result eq "ok")
   {
-    # Collect and analyze the 'Reconfig:' status line(s).
-    my $LogErrors;
-    foreach my $Line (<$LogFile>)
-    {
-      chomp($Line);
-      next if ($Line !~ /^Reconfig: (.*)$/);
-      # Add the error message or an empty string for 'ok'
-      $LogErrors = ($LogErrors || "") . ($1 ne "ok" ? "$1\n" : "");
-    }
-    close($LogFile);
-
-    if (!defined $LogErrors)
-    {
-      if (!defined $ErrMessage)
-      {
-        $NewStatus = "badbuild";
-        $ErrMessage = "Missing reconfig status line\n";
-      }
-      # otherwise $ErrMessage probably already explains why the reconfig
-      # status line is missing
-    }
-    elsif ($LogErrors eq "")
-    {
-      # We must have gotten the full log and the build did succeed.
-      # So forget any prior error.
-      $NewStatus = "completed";
-      $TAError = $ErrMessage = undef;
-    }
-    else
-    {
-      $NewStatus = "badbuild";
-      $ErrMessage = $LogErrors . ($ErrMessage || "");
-    }
+    # We must have gotten the full log and the build did succeed.
+    # So forget any prior error.
+    $NewStatus = "completed";
+    $TAError = $ErrMessage = undef;
+  }
+  elsif ($Result =~ s/^nolog://)
+  {
+    FatalError("$Result\n", "retry");
   }
   else
   {
-    FatalError("Unable to open the build log for reading: $!\n");
+    # We should not have badpatch errors and if the result line is missing we
+    # probably already have an error message that explains why.
+    $NewStatus = "badbuild";
   }
 }
 elsif (!defined $TAError)
diff --git a/testbot/lib/WineTestBot/LogUtils.pm b/testbot/lib/WineTestBot/LogUtils.pm
new file mode 100644
index 0000000..3e40fbc
--- /dev/null
+++ b/testbot/lib/WineTestBot/LogUtils.pm
@@ -0,0 +1,77 @@
+# -*- Mode: Perl; perl-indent-level: 2; indent-tabs-mode: nil -*-
+# Copyright 2018 Francois Gouget
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
+
+use strict;
+
+package WineTestBot::LogUtils;
+
+=head1 NAME
+
+WineTestBot::LogUtils - Provides functions to parse task logs
+
+=cut
+
+
+use Exporter 'import';
+our @EXPORT = qw(ParseTaskLog);
+
+
+#
+# Task log parser
+#
+
+=pod
+=over 12
+
+=item C<ParseTaskLog()>
+
+Returns ok if the task was successful and an error code otherwise.
+
+=back
+=cut
+
+sub ParseTaskLog($$)
+{
+  my ($FileName, $ResultPrefix) = @_;
+
+  if (open(my $LogFile, "<", $FileName))
+  {
+    my $Result;
+    foreach my $Line (<$LogFile>)
+    {
+      chomp $Line;
+      if ($Line =~ /^$ResultPrefix: ok$/)
+      {
+        $Result ||= "ok";
+      }
+      elsif ($Line =~ /^$ResultPrefix: Patch failed to apply$/)
+      {
+        $Result = "badpatch";
+        last; # Should be the last and most specific message
+      }
+      elsif ($Line =~ /^$ResultPrefix: /)
+      {
+        $Result = "failed";
+      }
+    }
+    close($LogFile);
+    return $Result || "missing";
+  }
+  return "nolog:Unable to open the task log for reading: $!";
+}
+
+1;