Francois Gouget : testbot: Fix the detection and reporting of timed out Wine tests.

[Alexandre Julliard]

Module: tools
Branch: master
Commit: 0d1500d714eee1e50d3639c92a65c8e6c31b21f2
URL:    https://source.winehq.org/git/tools.git/?a=commit;h=0d1500d714eee1e50d3639c9...

Author: Francois Gouget <fgouget(a)codeweavers.com>
Date:   Mon Sep 24 12:43:25 2018 +0200

testbot: Fix the detection and reporting of timed out Wine tests.

Modify ParseWineTestReport() to report the number of test units and
timeouts found in the report and remove the IsSuite parameter.
This allows the caller to check whether the report has the expected
number of test units, and to better decide whether the test as a whole
timed out. The policy is to now only consider it to have timed out if
all the test units timed out, which would typically only happen when
running a single test.

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard(a)winehq.org>

---

 testbot/bin/WineRunTask.pl          |  6 +++---
 testbot/bin/WineRunWineTest.pl      |  6 +++---
 testbot/lib/WineTestBot/LogUtils.pm | 16 +++++++++-------
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/testbot/bin/WineRunTask.pl b/testbot/bin/WineRunTask.pl
index 5ff3fc4..077426b 100755
--- a/testbot/bin/WineRunTask.pl
+++ b/testbot/bin/WineRunTask.pl
@@ -523,13 +523,13 @@ if (!defined $TA->Wait($Pid, $Timeout, $Keepalive))
   }
 }
 
-my $TimedOut;
 Debug(Elapsed($Start), " Retrieving the report file to '$RptFileName'\n");
 if ($TA->GetFile($RptFileName, "$TaskDir/$RptFileName"))
 {
   chmod 0664, "$TaskDir/$RptFileName";
 
-  (my $LogFailures, my $LogErrors, $TimedOut) = ParseWineTestReport("$TaskDir/$RptFileName", $IsWineTest, $Step->Type eq "suite", $TaskTimedOut);
+  my ($TestUnitCount, $TimeoutCount, $LogFailures, $LogErrors) = ParseWineTestReport("$TaskDir/$RptFileName", $IsWineTest, $TaskTimedOut);
+  $TaskTimedOut = 1 if ($TestUnitCount == $TimeoutCount);
   if (!defined $LogFailures and @$LogErrors == 1)
   {
     # Could not open the file
@@ -563,4 +563,4 @@ FatalTAError(undef, $TAError, $PossibleCrash) if (defined $TAError);
 # Wrap up
 #
 
-WrapUpAndExit($NewStatus, $TaskFailures, undef, $TaskTimedOut || $TimedOut);
+WrapUpAndExit($NewStatus, $TaskFailures, undef, $TaskTimedOut);
diff --git a/testbot/bin/WineRunWineTest.pl b/testbot/bin/WineRunWineTest.pl
index e3b8b17..90e3403 100755
--- a/testbot/bin/WineRunWineTest.pl
+++ b/testbot/bin/WineRunWineTest.pl
@@ -534,7 +534,6 @@ elsif (!defined $TAError)
 # Grab the test logs if any
 #
 
-my $TimedOut;
 if ($Step->Type ne "build")
 {
   my $BuildList = $Task->CmdLineArg;
@@ -547,7 +546,8 @@ if ($Step->Type ne "build")
     {
       chmod 0664, "$TaskDir/$RptFileName";
 
-      (my $LogFailures, my $LogErrors, $TimedOut) = ParseWineTestReport("$TaskDir/$RptFileName", 1, $Step->Type eq "suite", $TaskTimedOut);
+      my ($TestUnitCount, $TimeoutCount, $LogFailures, $LogErrors) = ParseWineTestReport("$TaskDir/$RptFileName", 1, $TaskTimedOut);
+      $TaskTimedOut = 1 if ($TestUnitCount == $TimeoutCount);
       if (!defined $LogFailures and @$LogErrors == 1)
       {
         # Could not open the file
@@ -595,4 +595,4 @@ FatalTAError(undef, $TAError, $PossibleCrash) if (defined $TAError);
 # Wrap up
 #
 
-WrapUpAndExit($NewStatus, $TaskFailures, undef, $TaskTimedOut || $TimedOut);
+WrapUpAndExit($NewStatus, $TaskFailures, undef, $TaskTimedOut);
diff --git a/testbot/lib/WineTestBot/LogUtils.pm b/testbot/lib/WineTestBot/LogUtils.pm
index b0b8040..17d44a4 100644
--- a/testbot/lib/WineTestBot/LogUtils.pm
+++ b/testbot/lib/WineTestBot/LogUtils.pm
@@ -277,23 +277,23 @@ a list of extra errors, and whether the test timed out.
 =back
 =cut
 
-sub ParseWineTestReport($$$$)
+sub ParseWineTestReport($$$)
 {
-  my ($FileName, $IsWineTest, $IsSuite, $TaskTimedOut) = @_;
+  my ($FileName, $IsWineTest, $TaskTimedOut) = @_;
 
   my $LogFile;
   if (!open($LogFile, "<", $FileName))
   {
     my $BaseName = basename($FileName);
-    return (undef, ["Unable to open '$BaseName' for reading: $!"], undef);
+    return (undef, undef, undef, ["Unable to open '$BaseName' for reading: $!"]);
   }
 
   my $Parser = {
     IsWineTest => $IsWineTest,
-    IsSuite => $IsSuite,
     TaskTimedOut => $TaskTimedOut,
 
-    TimedOut => undef,
+    TestUnitCount => 0,
+    TimeoutCount => 0,
     Failures => undef,
     Errors => [],
   };
@@ -309,6 +309,7 @@ sub ParseWineTestReport($$$$)
       # Close the previous test unit
       _CloseTestUnit($Parser, $Cur, 0) if ($Cur->{Dll} ne "");
       $Cur = _NewCurrentUnit($Dll, $Unit);
+      $Parser->{TestUnitCount}++;
 
       # Recognize skipped messages in case we need to skip tests in the VMs
       $Cur->{Rc} = 0 if ($Type eq "skipped");
@@ -414,7 +415,7 @@ sub ParseWineTestReport($$$$)
         # so record the failure but don't add an error message.
         $Parser->{Failures}++;
         $Cur->{IsBroken} = 1;
-        $Parser->{TimedOut} = $Parser->{IsSuite};
+        $Parser->{TimeoutCount}++;
       }
       elsif ((!$Pid and !%{$Cur->{Pids}}) or
              ($Pid and !$Cur->{Pids}->{$Pid} and !$Cur->{Pids}->{0}))
@@ -445,7 +446,8 @@ sub ParseWineTestReport($$$$)
   _CloseTestUnit($Parser, $Cur, 1);
   close($LogFile);
 
-  return ($Parser->{Failures}, $Parser->{Errors}, $Parser->{TimedOut});
+  return ($Parser->{TestUnitCount}, $Parser->{TimeoutCount},
+          $Parser->{Failures}, $Parser->{Errors});
 }