Log dEQP QPA files as test artifacts.

scripts/run_gtest_angle_test.py

@@ -78,6 +78,9 @@ def main():
     if 'GTEST_SHARD_INDEX' in env:
         extra_flags += ['--shard-index=' + env['GTEST_SHARD_INDEX']]
         env.pop('GTEST_SHARD_INDEX')
+    if 'ISOLATED_OUTDIR' in env:
+        extra_flags += ['--isolated-outdir=' + env['ISOLATED_OUTDIR']]
+        env.pop('ISOLATED_OUTDIR')
 
     # Assume we want to set up the sandbox environment variables all the
     # time; doing so is harmless on non-Linux platforms and is needed

src/tests/deqp_support/angle_deqp_gtest.cpp

@@ -23,6 +23,7 @@
 #include "platform/PlatformMethods.h"
 #include "tests/test_expectations/GPUTestConfig.h"
 #include "tests/test_expectations/GPUTestExpectationsParser.h"
+#include "tests/test_utils/runner/TestSuite.h"
 #include "util/OSWindow.h"
 #include "util/test_utils.h"
 
@@ -30,11 +31,19 @@ namespace angle
 {
 namespace
 {
+#if !defined(NDEBUG)
+constexpr bool kIsDebug = true;
+#else
+constexpr bool kIsDebug                = false;
+#endif  // !defined(NDEBUG)
+
 bool gGlobalError = false;
 bool gExpectError = false;
-uint32_t gBatchId = 0;
 bool gVerbose     = false;
 
+// Set this to true temporarily to enable image logging in release. Useful for diagnosing errors.
+bool gLogImages = kIsDebug;
+
 constexpr char kInfoTag[] = "*RESULT";
 
 void HandlePlatformError(PlatformMethods *platform, const char *errorMessage)
@@ -108,7 +117,6 @@ constexpr char kdEQPEGLString[]    = "--deqp-egl-display-type=";
 constexpr char kANGLEEGLString[]   = "--use-angle=";
 constexpr char kANGLEPreRotation[] = "--emulated-pre-rotation=";
 constexpr char kdEQPCaseString[]   = "--deqp-case=";
-constexpr char kBatchIdString[]    = "--batch-id=";
 constexpr char kVerboseString[]    = "--verbose";
 
 std::array<char, 500> gCaseStringBuffer;
@@ -127,7 +135,8 @@ constexpr uint32_t kDefaultPreRotation = 0;
 const APIInfo *gInitAPI = nullptr;
 uint32_t gPreRotation   = kDefaultPreRotation;
 
-constexpr const char *gdEQPEGLConfigNameString = "--deqp-gl-config-name=";
+constexpr const char gdEQPEGLConfigNameString[] = "--deqp-gl-config-name=";
+constexpr const char gdEQPLogImagesString[]     = "--deqp-log-images=";
 
 // Default the config to RGBA8
 const char *gEGLConfigName = "rgba8888d24s8";
@@ -387,7 +396,7 @@ class dEQPTest : public testing::TestWithParam<size_t>
         }
 
         gExpectError          = (caseInfo.mExpectation != GPUTestExpectationsParser::kGpuTestPass);
-        TestResult result = deqp_libtester_run(caseInfo.mDEQPName.c_str());
+        dEQPTestResult result = deqp_libtester_run(caseInfo.mDEQPName.c_str());
 
         bool testSucceeded = countTestResultAndReturnSuccess(result);
 
@@ -414,20 +423,20 @@ class dEQPTest : public testing::TestWithParam<size_t>
         }
     }
 
-    bool countTestResultAndReturnSuccess(TestResult result) const
+    bool countTestResultAndReturnSuccess(dEQPTestResult result) const
     {
         switch (result)
         {
-            case TestResult::Pass:
+            case dEQPTestResult::Pass:
                 sPassedTestCount++;
                 return true;
-            case TestResult::Fail:
+            case dEQPTestResult::Fail:
                 sFailedTestCount++;
                 return false;
-            case TestResult::NotSupported:
+            case dEQPTestResult::NotSupported:
                 sNotSupportedTestCount++;
                 return true;
-            case TestResult::Exception:
+            case dEQPTestResult::Exception:
                 sTestExceptionCount++;
                 return false;
             default:
@@ -533,16 +542,30 @@ void dEQPTest<TestModuleIndex>::SetUpTestCase()
         argv.push_back("--deqp-visibility=hidden");
     }
 
-    std::string logNameString;
-    if (gBatchId != 0)
-    {
+    TestSuite *testSuite = TestSuite::GetInstance();
+
     std::stringstream logNameStream;
-        logNameStream << "--deqp-log-filename=test-results-batch-" << std::setfill('0')
-                      << std::setw(3) << gBatchId << ".qpa";
-        logNameString = logNameStream.str();
+    logNameStream << "TestResults";
+    if (testSuite->getBatchId() != -1)
+    {
+        logNameStream << "-Batch" << std::setfill('0') << std::setw(3) << testSuite->getBatchId();
+    }
+    logNameStream << ".qpa";
+
+    std::stringstream logArgStream;
+    logArgStream << "--deqp-log-filename=" << testSuite->addTestArtifact(logNameStream.str());
+
+    std::string logNameString = logArgStream.str();
     argv.push_back(logNameString.c_str());
 
+    if (!gLogImages)
+    {
+        argv.push_back("--deqp-log-images=disable");
+    }
+
     // Flushing during multi-process execution punishes HDDs. http://anglebug.com/5157
+    if (testSuite->getBatchId() != -1)
+    {
         argv.push_back("--deqp-log-flush=disable");
     }
 
@@ -697,10 +720,21 @@ void HandleCaseName(const char *caseString, int *argc, int argIndex, char **argv
     argv[argIndex] = gCaseStringBuffer.data();
 }
 
-void HandleBatchId(const char *batchIdString)
+void HandleLogImages(const char *logImagesString)
 {
-    std::stringstream batchIdStream(batchIdString);
-    batchIdStream >> gBatchId;
+    if (strcmp(logImagesString, "enable") == 0)
+    {
+        gLogImages = true;
+    }
+    else if (strcmp(logImagesString, "disable") == 0)
+    {
+        gLogImages = false;
+    }
+    else
+    {
+        std::cout << "Error parsing log images setting. Use enable/disable.";
+        exit(1);
+    }
 }
 }  // anonymous namespace
 
@@ -731,15 +765,15 @@ void InitTestHarness(int *argc, char **argv)
         {
             HandleCaseName(argv[argIndex] + strlen(kdEQPCaseString), argc, argIndex, argv);
         }
-        else if (strncmp(argv[argIndex], kBatchIdString, strlen(kBatchIdString)) == 0)
-        {
-            HandleBatchId(argv[argIndex] + strlen(kBatchIdString));
-        }
         else if (strncmp(argv[argIndex], kVerboseString, strlen(kVerboseString)) == 0 ||
                  strcmp(argv[argIndex], "-v") == 0)
         {
             gVerbose = true;
         }
+        else if (strncmp(argv[argIndex], gdEQPLogImagesString, strlen(gdEQPLogImagesString)) == 0)
+        {
+            HandleLogImages(argv[argIndex] + strlen(gdEQPLogImagesString));
+        }
         argIndex++;
     }
 

src/tests/deqp_support/angle_deqp_libtester.h

@@ -29,7 +29,7 @@
 #endif
 
 // Possible results of deqp_libtester_run
-enum class TestResult
+enum class dEQPTestResult
 {
     Pass,
     Fail,
@@ -44,6 +44,6 @@ ANGLE_LIBTESTER_EXPORT bool deqp_libtester_init_platform(int argc,
                                                          void *logErrorFunc,
                                                          uint32_t preRotation);
 ANGLE_LIBTESTER_EXPORT void deqp_libtester_shutdown_platform();
-ANGLE_LIBTESTER_EXPORT TestResult deqp_libtester_run(const char *caseName);
+ANGLE_LIBTESTER_EXPORT dEQPTestResult deqp_libtester_run(const char *caseName);
 
 #endif  // ANGLE_DEQP_LIBTESTER_H_

src/tests/deqp_support/angle_deqp_libtester_main.cpp

@@ -139,7 +139,7 @@ ANGLE_LIBTESTER_EXPORT void deqp_libtester_shutdown_platform()
     g_platform = nullptr;
 }
 
-ANGLE_LIBTESTER_EXPORT TestResult deqp_libtester_run(const char *caseName)
+ANGLE_LIBTESTER_EXPORT dEQPTestResult deqp_libtester_run(const char *caseName)
 {
     const char *emptyString = "";
     if (g_platform == nullptr)
@@ -158,18 +158,18 @@ ANGLE_LIBTESTER_EXPORT TestResult deqp_libtester_run(const char *caseName)
             switch (result.getCode())
             {
                 case QP_TEST_RESULT_PASS:
-                    return TestResult::Pass;
+                    return dEQPTestResult::Pass;
                 case QP_TEST_RESULT_NOT_SUPPORTED:
                     std::cout << "Not supported! " << result.getDescription() << std::endl;
-                    return TestResult::NotSupported;
+                    return dEQPTestResult::NotSupported;
                 case QP_TEST_RESULT_QUALITY_WARNING:
                     std::cout << "Quality warning! " << result.getDescription() << std::endl;
-                    return TestResult::Pass;
+                    return dEQPTestResult::Pass;
                 case QP_TEST_RESULT_COMPATIBILITY_WARNING:
                     std::cout << "Compatiblity warning! " << result.getDescription() << std::endl;
-                    return TestResult::Pass;
+                    return dEQPTestResult::Pass;
                 default:
-                    return TestResult::Fail;
+                    return dEQPTestResult::Fail;
             }
         }
         else
@@ -180,8 +180,8 @@ ANGLE_LIBTESTER_EXPORT TestResult deqp_libtester_run(const char *caseName)
     catch (const std::exception &e)
     {
         std::cout << "Exception running test: " << e.what() << std::endl;
-        return TestResult::Exception;
+        return dEQPTestResult::Exception;
     }
 
-    return TestResult::Fail;
+    return dEQPTestResult::Fail;
 }

src/tests/test_utils/runner/README.md

@@ -26,6 +26,7 @@ following additional command-line arguments:
  * `--test-timeout` limits the amount of time spent in each test
  * `--flaky-retries` allows for tests to fail a fixed number of times and still pass
  * `--disable-crash-handler` forces off OS-level crash handling
+ * `--isolated-outdir` specifies a test artifacts directory
 
 `--isolated-script-test-output` and `--isolated-script-perf-test-output` mirror `--results-file`
 and `--histogram-json-file` respectively.

src/tests/test_utils/runner/TestSuite.cpp

@@ -46,11 +46,14 @@ constexpr char kPrintTestStdout[]      = "--print-test-stdout";
 constexpr char kResultFileArg[]        = "--results-file=";
 constexpr char kTestTimeoutArg[]       = "--test-timeout=";
 constexpr char kDisableCrashHandler[]  = "--disable-crash-handler";
+constexpr char kIsolatedOutDir[]       = "--isolated-outdir=";
 
 constexpr char kStartedTestString[] = "[ RUN      ] ";
 constexpr char kPassedTestString[]  = "[       OK ] ";
 constexpr char kFailedTestString[]  = "[  FAILED  ] ";
 
+constexpr char kArtifactsFakeTestName[] = "TestArtifactsFakeTest";
+
 #if defined(NDEBUG)
 constexpr int kDefaultTestTimeout = 20;
 #else
@@ -238,6 +241,45 @@ void WriteResultsFile(bool interrupted,
     js::Value tests;
     tests.SetObject();
 
+    // If we have any test artifacts, make a fake test to house them.
+    if (!testResults.testArtifactPaths.empty())
+    {
+        js::Value artifactsTest;
+        artifactsTest.SetObject();
+
+        artifactsTest.AddMember("actual", "PASS", allocator);
+        artifactsTest.AddMember("expected", "PASS", allocator);
+
+        js::Value artifacts;
+        artifacts.SetObject();
+
+        for (const std::string &testArtifactPath : testResults.testArtifactPaths)
+        {
+            std::vector<std::string> pieces =
+                SplitString(testArtifactPath, "/\\", WhitespaceHandling::TRIM_WHITESPACE,
+                            SplitResult::SPLIT_WANT_NONEMPTY);
+            ASSERT(!pieces.empty());
+
+            js::Value basename;
+            basename.SetString(pieces.back(), allocator);
+
+            js::Value artifactPath;
+            artifactPath.SetString(testArtifactPath, allocator);
+
+            js::Value artifactArray;
+            artifactArray.SetArray();
+            artifactArray.PushBack(artifactPath, allocator);
+
+            artifacts.AddMember(basename, artifactArray, allocator);
+        }
+
+        artifactsTest.AddMember("artifacts", artifacts, allocator);
+
+        js::Value fakeTestName;
+        fakeTestName.SetString(testResults.testArtifactsFakeTestName, allocator);
+        tests.AddMember(fakeTestName, artifactsTest, allocator);
+    }
+
     std::map<TestResultType, uint32_t> counts;
 
     for (const auto &resultIter : testResults.results)
@@ -558,39 +600,67 @@ std::string ParseTestSuiteName(const char *executable)
     return std::string(baseNameStart, baseNameStart + strlen(baseNameStart) - suffixLen);
 }
 
-bool GetTestResultsFromJSON(const js::Document &document, TestResults *resultsOut)
+bool GetTestArtifactsFromJSON(const js::Value::ConstObject &obj,
+                              std::vector<std::string> *testArtifactPathsOut)
 {
-    if (!document.HasMember("tests") || !document["tests"].IsObject())
+    if (!obj.HasMember("artifacts"))
     {
+        printf("No artifacts member.\n");
         return false;
     }
 
-    const js::Value::ConstObject &tests = document["tests"].GetObject();
-    for (auto iter = tests.MemberBegin(); iter != tests.MemberEnd(); ++iter)
+    const js::Value &jsArtifacts = obj["artifacts"];
+    if (!jsArtifacts.IsObject())
     {
-        // Get test identifier.
-        const js::Value &name = iter->name;
-        if (!name.IsString())
+        printf("Artifacts are not an object.\n");
+        return false;
+    }
+
+    const js::Value::ConstObject &artifacts = jsArtifacts.GetObject();
+    for (const auto &artifactMember : artifacts)
+    {
+        const js::Value &artifact = artifactMember.value;
+        if (!artifact.IsArray())
         {
+            printf("Artifact is not an array of strings of size 1.\n");
             return false;
         }
 
-        TestIdentifier id;
-        if (!TestIdentifier::ParseFromString(name.GetString(), &id))
+        const js::Value::ConstArray &artifactArray = artifact.GetArray();
+        if (artifactArray.Size() != 1)
         {
+            printf("Artifact is not an array of strings of size 1.\n");
             return false;
         }
 
-        // Get test result.
-        const js::Value &value = iter->value;
-        if (!value.IsObject())
+        const js::Value &artifactName = artifactArray[0];
+        if (!artifactName.IsString())
         {
+            printf("Artifact is not an array of strings of size 1.\n");
+            return false;
+        }
+
+        testArtifactPathsOut->push_back(artifactName.GetString());
+    }
+
+    return true;
+}
+
+bool GetSingleTestResultFromJSON(const js::Value &name,
+                                 const js::Value::ConstObject &obj,
+                                 TestResults *resultsOut)
+{
+
+    TestIdentifier id;
+    if (!TestIdentifier::ParseFromString(name.GetString(), &id))
+    {
+        printf("Could not parse test identifier.\n");
         return false;
     }
 
-        const js::Value::ConstObject &obj = value.GetObject();
     if (!obj.HasMember("expected") || !obj.HasMember("actual"))
     {
+        printf("No expected or actual member.\n");
         return false;
     }
 
@@ -599,6 +669,7 @@ bool GetTestResultsFromJSON(const js::Document &document, TestResults *resultsOu
 
     if (!expected.IsString() || !actual.IsString())
     {
+        printf("Expected or actual member is not a string.\n");
         return false;
     }
 
@@ -656,6 +727,52 @@ bool GetTestResultsFromJSON(const js::Document &document, TestResults *resultsOu
     result.elapsedTimeSeconds = elapsedTimeSeconds;
     result.type               = resultType;
     result.flakyFailures      = flakyFailures;
+    return true;
+}
+
+bool GetTestResultsFromJSON(const js::Document &document, TestResults *resultsOut)
+{
+    if (!document.HasMember("tests") || !document["tests"].IsObject())
+    {
+        printf("JSON document has no tests member.\n");
+        return false;
+    }
+
+    const js::Value::ConstObject &tests = document["tests"].GetObject();
+    for (const auto &testMember : tests)
+    {
+        // Get test identifier.
+        const js::Value &name = testMember.name;
+        if (!name.IsString())
+        {
+            printf("Name is not a string.\n");
+            return false;
+        }
+
+        // Get test result.
+        const js::Value &value = testMember.value;
+        if (!value.IsObject())
+        {
+            printf("Test result is not an object.\n");
+            return false;
+        }
+
+        const js::Value::ConstObject &obj = value.GetObject();
+
+        if (BeginsWith(name.GetString(), kArtifactsFakeTestName))
+        {
+            if (!GetTestArtifactsFromJSON(obj, &resultsOut->testArtifactPaths))
+            {
+                return false;
+            }
+        }
+        else
+        {
+            if (!GetSingleTestResultFromJSON(name, obj, resultsOut))
+            {
+                return false;
+            }
+        }
     }
 
     return true;
@@ -697,6 +814,10 @@ bool MergeTestResults(TestResults *input, TestResults *output, int flakyRetries)
         }
     }
 
+    output->testArtifactPaths.insert(output->testArtifactPaths.end(),
+                                     input->testArtifactPaths.begin(),
+                                     input->testArtifactPaths.end());
+
     return true;
 }
 
@@ -1093,6 +1214,16 @@ TestSuite::TestSuite(int *argc, char **argv)
         }
     }
 
+    {
+        std::stringstream fakeTestName;
+        fakeTestName << kArtifactsFakeTestName;
+        if (mShardIndex != -1)
+        {
+            fakeTestName << "-Shard" << std::setfill('0') << std::setw(2) << mShardIndex;
+        }
+        mTestResults.testArtifactsFakeTestName = fakeTestName.str();
+    }
+
     if (mBotMode)
     {
         // Split up test batches.
@@ -1172,6 +1303,7 @@ bool TestSuite::parseSingleArg(const char *argument)
             ParseStringArg(kFilterFileArg, argument, &mFilterFile) ||
             ParseStringArg(kHistogramJsonFileArg, argument, &mHistogramJsonFile) ||
             ParseStringArg("--isolated-script-test-perf-output=", argument, &mHistogramJsonFile) ||
+            ParseStringArg(kIsolatedOutDir, argument, &mTestArtifactDirectory) ||
             ParseFlag("--bot-mode", argument, &mBotMode) ||
             ParseFlag("--debug-test-groups", argument, &mDebugTestGroups) ||
             ParseFlag(kGTestListTests, argument, &mGTestListTests) ||
@@ -1273,6 +1405,15 @@ bool TestSuite::launchChildTestProcess(uint32_t batchId,
         args.push_back(timeoutStr.c_str());
     }
 
+    std::string artifactsDir;
+    if (!mTestArtifactDirectory.empty())
+    {
+        std::stringstream artifactsDirStream;
+        artifactsDirStream << kIsolatedOutDir << mTestArtifactDirectory;
+        artifactsDir = artifactsDirStream.str();
+        args.push_back(artifactsDir.c_str());
+    }
+
     // Launch child process and wait for completion.
     processInfo.process = LaunchProcess(args, true, true);
 
@@ -1471,8 +1612,8 @@ int TestSuite::run()
         {
             startWatchdog();
         }
-        int retVal = RUN_ALL_TESTS();
 
+        int retVal = RUN_ALL_TESTS();
         {
             std::lock_guard<std::mutex> guard(mTestResults.currentTestMutex);
             mTestResults.allDone = true;
@@ -1650,6 +1791,20 @@ void TestSuite::registerSlowTests(const char *slowTests[], size_t numSlowTests)
     }
 }
 
+std::string TestSuite::addTestArtifact(const std::string &artifactName)
+{
+    mTestResults.testArtifactPaths.push_back(artifactName);
+
+    if (mTestArtifactDirectory.empty())
+    {
+        return artifactName;
+    }
+
+    std::stringstream pathStream;
+    pathStream << mTestArtifactDirectory << GetPathSeparator() << artifactName;
+    return pathStream.str();
+}
+
 bool GetTestResultsFromFile(const char *fileName, TestResults *resultsOut)
 {
     std::ifstream ifs(fileName);

src/tests/test_utils/runner/TestSuite.h

@@ -94,6 +94,8 @@ struct TestResults
     Timer currentTestTimer;
     double currentTestTimeout = 0.0;
     bool allDone              = false;
+    std::string testArtifactsFakeTestName;
+    std::vector<std::string> testArtifactPaths;
 };
 
 struct FileLine
@@ -135,6 +137,12 @@ class TestSuite
 
     static TestSuite *GetInstance() { return mInstance; }
 
+    // Returns the path to the artifact in the output directory.
+    std::string addTestArtifact(const std::string &artifactName);
+
+    int getShardIndex() const { return mShardIndex; }
+    int getBatchId() const { return mBatchId; }
+
   private:
     bool parseSingleArg(const char *argument);
     bool launchChildTestProcess(uint32_t batchId, const std::vector<TestIdentifier> &testsInBatch);
@@ -176,6 +184,7 @@ class TestSuite
     std::thread mWatchdogThread;
     HistogramWriter mHistogramWriter;
     std::vector<std::string> mSlowTests;
+    std::string mTestArtifactDirectory;
 };
 
 bool GetTestResultsFromFile(const char *fileName, TestResults *resultsOut);