From: Francois Gouget <fgouget@codeweavers.com>
Subject: [PATCH] testbot: Fix the detection and reporting of timed out Wine tests.
Message-Id: <E1g4OKr-0004wZ-Bf@amboise>
Date: Mon, 24 Sep 2018 12:43:25 +0200 (CEST)

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@codeweavers.com>
---
 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 5ff3fc4a5..077426b23 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 e3b8b17ed..90e3403f7 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 b0b804046..17d44a44c 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});
 }
 
 

-- 
2.18.0