Test Runner: Fix race in watchdog timeouts.

src/tests/test_utils/runner/TestSuite.cpp

@@ -14,7 +14,9 @@
 #include "common/system_utils.h"
 #include "util/Timer.h"
 
+#include <stdlib.h>
 #include <time.h>
+
 #include <fstream>
 #include <unordered_map>
 
@@ -944,7 +946,7 @@ TestSuite::TestSuite(int *argc, char **argv)
         mResultsFile = resultFileName.str();
     }
 
-    if (!mResultsFile.empty() || !mHistogramJsonFile.empty())
+    if (!mBotMode)
     {
         testing::TestEventListeners &listeners = testing::UnitTest::GetInstance()->listeners();
         listeners.Append(new TestEventListener(mResultsFile, mHistogramJsonFile,
@@ -987,6 +989,7 @@ bool TestSuite::parseSingleArg(const char *argument)
 
 void TestSuite::onCrashOrTimeout(TestResultType crashOrTimeout)
 {
+    std::lock_guard<std::mutex> guard(mTestResults.currentTestMutex);
     if (mTestResults.currentTest.valid())
     {
         TestResult &result        = mTestResults.results[mTestResults.currentTest];
@@ -1191,12 +1194,7 @@ int TestSuite::run()
             mTestResults.allDone = true;
         }
 
-        for (int tries = 0; tries < 10; ++tries)
-        {
-            if (!mWatchdogThread.joinable())
-                break;
-            angle::Sleep(100);
-        }
+        mWatchdogThread.join();
         return retVal;
     }
 
@@ -1333,16 +1331,17 @@ void TestSuite::startWatchdog()
                 if (mTestResults.currentTestTimer.getElapsedTime() >
                     static_cast<double>(mTestTimeout))
                 {
-                    onCrashOrTimeout(TestResultType::Timeout);
-                    exit(2);
+                    break;
                 }
 
                 if (mTestResults.allDone)
                     return;
             }
 
-            angle::Sleep(1000);
+            angle::Sleep(500);
         } while (true);
+        onCrashOrTimeout(TestResultType::Timeout);
+        ::_Exit(1);
     };
     mWatchdogThread = std::thread(watchdogMain);
 }

src/tests/test_utils/runner/TestSuite_unittest.cpp

@@ -48,11 +48,11 @@ TEST_F(TestSuiteTest, RunMockTests)
     executablePath += std::string("/") + kTestHelperExecutable + GetExecutableExtension();
 
     constexpr uint32_t kMaxTempDirLen = 100;
-    char tempFileName[kMaxTempDirLen * 2];
-    ASSERT_TRUE(GetTempDir(tempFileName, kMaxTempDirLen));
+    char tempDirName[kMaxTempDirLen * 2];
+    ASSERT_TRUE(GetTempDir(tempDirName, kMaxTempDirLen));
 
     std::stringstream tempFNameStream;
-    tempFNameStream << tempFileName << "/test_temp_" << rand() << ".json";
+    tempFNameStream << tempDirName << GetPathSeparator() << "test_temp_" << rand() << ".json";
     mTempFileName = tempFNameStream.str();
 
     std::string resultsFileName = "--results-file=" + mTempFileName;
@@ -62,7 +62,7 @@ TEST_F(TestSuiteTest, RunMockTests)
                                       "--gtest_filter=MockTestSuiteTest.DISABLED_*",
                                       "--gtest_also_run_disabled_tests",
                                       "--bot-mode",
-                                      "--test-timeout=10",
+                                      "--test-timeout=2",
                                       resultsFileName.c_str()};
 
     ProcessHandle process(args, true, true);
@@ -71,6 +71,9 @@ TEST_F(TestSuiteTest, RunMockTests)
     EXPECT_TRUE(process->finished());
     EXPECT_EQ(process->getStderr(), "");
 
+    // Uncomment this for debugging.
+    // printf("stdout:\n%s\n", process->getStdout().c_str());
+
     TestResults actual;
     ASSERT_TRUE(GetTestResultsFromFile(mTempFileName.c_str(), &actual));
     EXPECT_TRUE(DeleteFile(mTempFileName.c_str()));
@@ -101,7 +104,7 @@ TEST(MockTestSuiteTest, DISABLED_Fail)
 // Trigger a test timeout.
 TEST(MockTestSuiteTest, DISABLED_Timeout)
 {
-    angle::Sleep(30000);
+    angle::Sleep(5000);
 }
 
 // Trigger a test crash.

util/windows/test_utils_win.cpp

@@ -403,6 +403,15 @@ Process *LaunchProcess(const std::vector<const char *> &args,
 bool GetTempDir(char *tempDirOut, uint32_t maxDirNameLen)
 {
     DWORD pathLen = ::GetTempPathA(maxDirNameLen, tempDirOut);
+    // Strip last path character if present.
+    if (pathLen > 0)
+    {
+        size_t lastChar = strlen(tempDirOut) - 1;
+        if (tempDirOut[lastChar] == '\\')
+        {
+            tempDirOut[lastChar] = 0;
+        }
+    }
     return (pathLen < MAX_PATH && pathLen > 0);
 }