Simplify skiatest framework.

tests/skia_test.cpp

Index: tests/skia_test.cpp
diff --git a/tests/skia_test.cpp b/tests/skia_test.cpp
index cf446d21052f922800eb34a0f4186a6fdca3575c..2aca99d4a101ef03fa50aa5d3b704c3ea99d5f2f 100644
--- a/tests/skia_test.cpp
+++ b/tests/skia_test.cpp
@@ -10,11 +10,13 @@
 #include "Resources.h"
 #include "SkCommonFlags.h"
 #include "SkGraphics.h"
+#include "SkInstCnt.h"
 #include "SkOSFile.h"
 #include "SkRunnable.h"
 #include "SkTArray.h"
 #include "SkTaskGroup.h"
 #include "SkTemplates.h"
+#include "SkThread.h"
 #include "SkTime.h"
 #include "Test.h"
 
@@ -30,78 +32,86 @@ DEFINE_bool2(extendedTest, x, false, "run extended tests for pathOps.");
 // need to explicitly declare this, or we get some weird infinite loop llist
 template TestRegistry* TestRegistry::gHead;
 
-class Iter {
+// The threads report back to this object when they are done.
+class Status {
 public:
-    Iter() { this->reset(); }
-    void reset() { fReg = TestRegistry::Head(); }
-
-    Test* next(Reporter* r) {
-        if (fReg) {
-            TestRegistry::Factory fact = fReg->factory();
-            fReg = fReg->next();
-            Test* test = fact(NULL);
-            test->setReporter(r);
-            return test;
-        }
-        return NULL;
-    }
-
-private:
-    const TestRegistry* fReg;
-};
-
-class DebugfReporter : public Reporter {
-public:
-    explicit DebugfReporter(int total) : fDone(0), fTotal(total) {}
-
-    bool allowExtendedTest() const SK_OVERRIDE { return FLAGS_extendedTest; }
-    bool verbose()           const SK_OVERRIDE { return FLAGS_veryVerbose; }
-
-protected:
-    void onReportFailed(const skiatest::Failure& failure) SK_OVERRIDE {
-        SkString desc;
-        failure.getFailureString(&desc);
-        SkDebugf("\nFAILED: %s", desc.c_str());
-    }
-
-    void onEnd(Test* test) SK_OVERRIDE {
+    explicit Status(int total)
+        : fDone(0), fTestCount(0), fFailCount(0), fTotal(total) {}
+    // Threadsafe.
+    void endTest(const char* testName,
+                 bool success,
+                 SkMSec elapsed,
+                 int testCount) {
         const int done = 1 + sk_atomic_inc(&fDone);
-
-        if (!test->passed()) {
-            SkDebugf("\n---- %s FAILED", test->getName());
+        for (int i = 0; i < testCount; ++i) {
+            sk_atomic_inc(&fTestCount);
+        }
+        if (!success) {
+            SkDebugf("\n---- %s FAILED", testName);
         }
 
         SkString prefix(kSkOverwriteLine);
         SkString time;
         if (FLAGS_verbose) {
             prefix.printf("\n");
-            time.printf("%5dms ", test->elapsedMs());
+            time.printf("%5dms ", elapsed);
         }
-        SkDebugf("%s[%3d/%3d] %s%s", prefix.c_str(), done, fTotal, time.c_str(), test->getName());
+        SkDebugf("%s[%3d/%3d] %s%s", prefix.c_str(), done, fTotal, time.c_str(),
+                 testName);
     }
 
+    void reportFailure() { sk_atomic_inc(&fFailCount); }
+
+    int32_t testCount() { return fTestCount; }
+    int32_t failCount() { return fFailCount; }
+
 private:
     int32_t fDone;  // atomic
+    int32_t fTestCount;  // atomic
+    int32_t fFailCount;  // atomic
     const int fTotal;
 };
 
 // Deletes self when run.
 class SkTestRunnable : public SkRunnable {
 public:
-  // Takes ownership of test.
-  SkTestRunnable(Test* test, int32_t* failCount) : fTest(test), fFailCount(failCount) {}
+    SkTestRunnable(const Test& test,
+                   Status* status,
+                   GrContextFactory* grContextFactory = NULL)
+        : fTest(test), fStatus(status), fGrContextFactory(grContextFactory) {}
 
   virtual void run() {
-      fTest->run();
-      if(!fTest->passed()) {
-          sk_atomic_inc(fFailCount);
+      struct TestReporter : public skiatest::Reporter {
+      public:
+          TestReporter() : fError(false), fTestCount(0) {}
+          void bumpTestCount() SK_OVERRIDE { ++fTestCount; }
+          bool allowExtendedTest() const SK_OVERRIDE {
+              return FLAGS_extendedTest;
+          }
+          bool verbose() const SK_OVERRIDE { return FLAGS_veryVerbose; }
+          void reportFailed(const skiatest::Failure& failure) SK_OVERRIDE {
+              SkDebugf("\nFAILED: %s", failure.toString().c_str());
+              fError = true;
+          }
+          bool fError;
+          int fTestCount;
+      } reporter;
+
+      const SkMSec start = SkTime::GetMSecs();
+      fTest.proc(&reporter, fGrContextFactory);
+      SkMSec elapsed = SkTime::GetMSecs() - start;
+      if (reporter.fError) {
+          fStatus->reportFailure();
       }
+      fStatus->endTest(fTest.name, !reporter.fError, elapsed,
+                       reporter.fTestCount);
       SkDELETE(this);
   }
 
 private:
-    SkAutoTDelete<Test> fTest;
-    int32_t* fFailCount;
+    Test fTest;
+    Status* fStatus;
+    GrContextFactory* fGrContextFactory;
 };
 
 static bool should_run(const char* testName, bool isGPUTest) {
@@ -137,7 +147,7 @@ int test_main() {
                 header.appendf(" %s", FLAGS_match[index]);
             }
         }
-        SkString tmpDir = Test::GetTmpDir();
+        SkString tmpDir = skiatest::GetTmpDir();
         if (!tmpDir.isEmpty()) {
             header.appendf(" --tmpDir %s", tmpDir.c_str());
         }
@@ -161,61 +171,62 @@ int test_main() {
     // Count tests first.
     int total = 0;
     int toRun = 0;
-    Test* test;
 
-    Iter iter;
-    while ((test = iter.next(NULL/*reporter not needed*/)) != NULL) {
-        SkAutoTDelete<Test> owned(test);
-        if (should_run(test->getName(), test->isGPUTest())) {
+    for (const TestRegistry* iter = TestRegistry::Head(); iter;
+         iter = iter->next()) {
+        const Test& test = iter->factory();
+        if (should_run(test.name, test.needsGpu)) {
             toRun++;
         }
         total++;
     }
 
     // Now run them.
-    iter.reset();
-    int32_t failCount = 0;
     int skipCount = 0;
 
     SkTaskGroup::Enabler enabled(FLAGS_threads);
     SkTaskGroup cpuTests;
-    SkTArray<Test*> gpuTests;  // Always passes ownership to an SkTestRunnable
+    SkTArray<const Test*> gpuTests;
 
-    DebugfReporter reporter(toRun);
-    for (int i = 0; i < total; i++) {
-        SkAutoTDelete<Test> test(iter.next(&reporter));
-        if (!should_run(test->getName(), test->isGPUTest())) {
+    Status status(toRun);
+    for (const TestRegistry* iter = TestRegistry::Head(); iter;
+         iter = iter->next()) {
+        const Test& test = iter->factory();
+        if (!should_run(test.name, test.needsGpu)) {
             ++skipCount;
-        } else if (test->isGPUTest()) {
-            gpuTests.push_back() = test.detach();
+        } else if (test.needsGpu) {
+            gpuTests.push_back(&test);
         } else {
-            cpuTests.add(SkNEW_ARGS(SkTestRunnable, (test.detach(), &failCount)));
+            cpuTests.add(SkNEW_ARGS(SkTestRunnable, (test, &status)));
         }
     }
 
+    GrContextFactory* grContextFactoryPtr = NULL;
 #if SK_SUPPORT_GPU
     // Give GPU tests a context factory if that makes sense on this machine.
     GrContextFactory grContextFactory;
-    for (int i = 0; i < gpuTests.count(); i++) {
-        gpuTests[i]->setGrContextFactory(&grContextFactory);
-    }
+    grContextFactoryPtr = &grContextFactory;
+
 #endif
 
     // Run GPU tests on this thread.
     for (int i = 0; i < gpuTests.count(); i++) {
-        SkNEW_ARGS(SkTestRunnable, (gpuTests[i], &failCount))->run();
+        SkNEW_ARGS(SkTestRunnable, (*gpuTests[i], &status, grContextFactoryPtr))
+                ->run();
     }
 
     // Block until threaded tests finish.
     cpuTests.wait();
 
     if (FLAGS_verbose) {
-        SkDebugf("\nFinished %d tests, %d failures, %d skipped. (%d internal tests)",
-                 toRun, failCount, skipCount, reporter.countTests());
+        SkDebugf(
+                "\nFinished %d tests, %d failures, %d skipped. "
+                "(%d internal tests)",
+                toRun, status.failCount(), skipCount, status.testCount());
     }
 
     SkDebugf("\n");
-    return (failCount == 0) ? 0 : 1;
+    return (status.failCount() == 0) ? 0 : 1;
 }
 
 #if !defined(SK_BUILD_FOR_IOS) && !defined(SK_BUILD_FOR_NACL)