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..adacdc3e075c6941b3c83afbd30c23ad2a638d35 100644
--- a/tests/skia_test.cpp
+++ b/tests/skia_test.cpp
@@ -26,62 +26,70 @@
 using namespace skiatest;
 
 DEFINE_bool2(extendedTest, x, false, "run extended tests for pathOps.");
+DEFINE_string2(tmpDir, t, NULL, "tmp directory for tests to use.");
 
 // need to explicitly declare this, or we get some weird infinite loop llist
 template TestRegistry* TestRegistry::gHead;
 
-class Iter {
-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;
-};
+static SkString get_tmp_dir() {
+    return SkString(FLAGS_tmpDir.isEmpty() ? NULL : FLAGS_tmpDir[0]);
+}
 
 class DebugfReporter : public Reporter {
 public:
-    explicit DebugfReporter(int total) : fDone(0), fTotal(total) {}
-
+    DebugfReporter() : fErrorCount(0), fTestCount(0) {
+    }
     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);
+    void reportFailed(const skiatest::Failure& failure) SK_OVERRIDE {
+        SkString desc = FailureToString(failure);
         SkDebugf("\nFAILED: %s", desc.c_str());
+        ++fErrorCount;
+    }
+    SkString getTmpDir() SK_OVERRIDE {
+        return get_tmp_dir();
+    }
+    void bumpTestCount() SK_OVERRIDE {
+        ++fTestCount;
     }
+    int fErrorCount;
+    int fTestCount;
+};
 
-    void onEnd(Test* test) SK_OVERRIDE {
+class DebugfStatusKeeper {
+public:
+    explicit DebugfStatusKeeper(int total) : fDone(0), fTotal(total) {
+    }
+    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);
+    }
+
+    int32_t testCount() {
+        return fTestCount;
     }
 
 private:
     int32_t fDone;  // atomic
+    int32_t fTestCount;  // atomic
     const int fTotal;
 };
 
@@ -89,19 +97,34 @@ private:
 class SkTestRunnable : public SkRunnable {
 public:
   // Takes ownership of test.
-  SkTestRunnable(Test* test, int32_t* failCount) : fTest(test), fFailCount(failCount) {}
+    SkTestRunnable(const Test& test,
+                   int32_t* failCount,
+                   DebugfStatusKeeper* status,
+                   GrContextFactory* grContextFactory = NULL)
+        : fTest(test)
+        , fFailCount(failCount)
+        , fStatus(status)
+        , fGrContextFactory(grContextFactory) {
+    }
 
   virtual void run() {
-      fTest->run();
-      if(!fTest->passed()) {
+      DebugfReporter reporter;
+      const SkMSec start = SkTime::GetMSecs();
+      fTest.proc(&reporter, fGrContextFactory);
+      SkMSec elapsed = SkTime::GetMSecs() - start;
+      bool success = 0 == reporter.fErrorCount;
+      if (!success) {
           sk_atomic_inc(fFailCount);
       }
+      fStatus->endTest(fTest.name, success, elapsed, reporter.fTestCount);
       SkDELETE(this);
   }
 
 private:
-    SkAutoTDelete<Test> fTest;
+    Test fTest;
     int32_t* fFailCount;
+    DebugfStatusKeeper* fStatus;
+    GrContextFactory* fGrContextFactory;
 };
 
 static bool should_run(const char* testName, bool isGPUTest) {
@@ -137,7 +160,7 @@ int test_main() {
                 header.appendf(" %s", FLAGS_match[index]);
             }
         }
-        SkString tmpDir = Test::GetTmpDir();
+        SkString tmpDir = get_tmp_dir();
         if (!tmpDir.isEmpty()) {
             header.appendf(" --tmpDir %s", tmpDir.c_str());
         }
@@ -161,57 +184,60 @@ 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())) {
+    DebugfStatusKeeper 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, &failCount, &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], &failCount, &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, failCount, skipCount, status.testCount());
     }
 
     SkDebugf("\n");