[PATCH] testbot: Modify ParseTaskLog() so it can return more information.

[Francois Gouget]

This also makes the 'NoLog' error case a bit cleaner.

Signed-off-by: Francois Gouget 
---
 testbot/bin/WineRunBuild.pl         | 10 +++++-----
 testbot/bin/WineRunReconfig.pl      |  8 ++++----
 testbot/bin/WineRunWineTest.pl      | 12 ++++++------
 testbot/lib/WineTestBot/LogUtils.pm | 28 +++++++++++++++++++---------
 4 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/testbot/bin/WineRunBuild.pl b/testbot/bin/WineRunBuild.pl
index 1d57c4408..a8b7803e9 100755
--- a/testbot/bin/WineRunBuild.pl
+++ b/testbot/bin/WineRunBuild.pl
@@ -391,23 +391,23 @@ if (!defined $TA->Wait($Pid, $Task->Timeout, 60))
 Debug(Elapsed($Start), " Retrieving 'Build.log'\n");
 if ($TA->GetFile("Build.log", "$TaskDir/log"))
 {
-  my ($Result, $_Type) = ParseTaskLog("$TaskDir/log");
-  if ($Result eq "ok")
+  my $Summary = ParseTaskLog("$TaskDir/log");
+  if ($Summary->{Task} eq "ok")
   {
     # 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")
+  elsif ($Summary->{Task} eq "badpatch")
   {
     # This too is conclusive enough to ignore other errors.
     $NewStatus = "badpatch";
     $TAError = $ErrMessage = undef;
   }
-  elsif ($Result =~ s/^nolog://)
+  elsif ($Summary->{NoLog})
   {
-    FatalError("$Result\n", "retry");
+    FatalError("$Summary->{NoLog}\n", "retry");
   }
   else
   {
diff --git a/testbot/bin/WineRunReconfig.pl b/testbot/bin/WineRunReconfig.pl
index cf511f306..ec12d960a 100755
--- a/testbot/bin/WineRunReconfig.pl
+++ b/testbot/bin/WineRunReconfig.pl
@@ -392,17 +392,17 @@ if (!defined $TA->Wait($Pid, $Task->Timeout, 60))
 Debug(Elapsed($Start), " Retrieving 'Reconfig.log'\n");
 if ($TA->GetFile("Reconfig.log", "$TaskDir/log"))
 {
-  my ($Result, $_Type) = ParseTaskLog("$TaskDir/log");
-  if ($Result eq "ok")
+  my $Summary = ParseTaskLog("$TaskDir/log");
+  if ($Summary->{Task} eq "ok")
   {
     # 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://)
+  elsif ($Summary->{NoLog})
   {
-    FatalError("$Result\n", "retry");
+    FatalError("$Summary->{NoLog}\n", "retry");
   }
   else
   {
diff --git a/testbot/bin/WineRunWineTest.pl b/testbot/bin/WineRunWineTest.pl
index 508605b39..514c57cda 100755
--- a/testbot/bin/WineRunWineTest.pl
+++ b/testbot/bin/WineRunWineTest.pl
@@ -512,8 +512,8 @@ if (!defined $TA->Wait($Pid, $Task->Timeout, 60))
 Debug(Elapsed($Start), " Retrieving 'Task.log'\n");
 if ($TA->GetFile("Task.log", "$TaskDir/log"))
 {
-  my ($Result, $Type) = ParseTaskLog("$TaskDir/log");
-  if ($Result eq "ok")
+  my $Summary = ParseTaskLog("$TaskDir/log");
+  if ($Summary->{Task} eq "ok")
   {
     # We must have gotten the full log and the task completed successfully
     # (with or without test failures). So clear any previous errors, including
@@ -521,17 +521,17 @@ if ($TA->GetFile("Task.log", "$TaskDir/log"))
     $NewStatus = "completed";
     $TaskFailures = $TAError = $ErrMessage = $PossibleCrash = undef;
   }
-  elsif ($Result eq "badpatch")
+  elsif ($Summary->{Task} eq "badpatch")
   {
     # This too is conclusive enough to ignore other errors.
     $NewStatus = "badpatch";
     $TaskFailures = $TAError = $ErrMessage = $PossibleCrash = undef;
   }
-  elsif ($Result =~ s/^nolog://)
+  elsif ($Summary->{NoLog})
   {
-    FatalError("$Result\n", "retry");
+    FatalError("$Summary->{NoLog}\n", "retry");
   }
-  elsif ($Type eq "build")
+  elsif ($Summary->{Type} eq "build")
   {
     # The error happened before the tests started so blame the build.
     $NewStatus = "badbuild";
diff --git a/testbot/lib/WineTestBot/LogUtils.pm b/testbot/lib/WineTestBot/LogUtils.pm
index 21f7dd054..d23a99c6b 100644
--- a/testbot/lib/WineTestBot/LogUtils.pm
+++ b/testbot/lib/WineTestBot/LogUtils.pm
@@ -69,7 +69,17 @@ sub _IsPerlError($)

=item C<ParseTaskLog()>

-Returns ok if the task was successful and an error code otherwise.
+Returns a hashtable containing a summary of the task log:
+=over
+
+=item Type
+'tests' if the task ran Wine tests and 'build' otherwise.
+
+=item Task
+Either 'ok' if the task was successful or a code indicating why it failed.
+
+=item NoLog
+Contains an error message if the task log could not be read.

=back
 =cut
@@ -80,33 +90,33 @@ sub ParseTaskLog($)

if (open(my $LogFile, "<", $FileName))
   {
-    my $Result;
-    my $Type = "build";
+    my $Summary = {Type => "build"};
     foreach my $Line (<$LogFile>)
     {
       chomp $Line;
       if ($Line eq "Task: tests")
       {
-        $Type = "tests";
+        $Summary->{Type} = "tests";
       }
       elsif ($Line eq "Task: ok")
       {
-        $Result ||= "ok";
+        $Summary->{Task} ||= "ok";
       }
       elsif ($Line eq "Task: Patch failed to apply")
       {
-        $Result = "badpatch";
+        $Summary->{Task} = "badpatch";
         last; # Should be the last and most specific message
       }
       elsif ($Line =~ /^Task: / or _IsPerlError($Line))
       {
-        $Result = "failed";
+        $Summary->{Task} = "failed";
       }
     }
     close($LogFile);
-    return ($Result || "missing", $Type);
+    $Summary->{Task} ||= "missing";
+    return $Summary;
   }
-  return ("nolog:Unable to open the task log for reading: $!", undef);
+  return {NoLog => "Unable to open the task log for reading: $!"};
 }