Fix directory creation in skqp app

Bug: skia:
Change-Id: Id26832ccce7d09d3d3d640bcdd11a424f18943f7
Reviewed-on: https://skia-review.googlesource.com/92682
Reviewed-by: Hal Canary <halcanary@google.com>
Commit-Queue: Hal Canary <halcanary@google.com>

5 files changed, 59 insertions, 45 deletions

diff --git a/infra/cts/run_testlab.go b/infra/cts/run_testlab.go
index cf88c30cc9..98aa160d21 100644
--- a/infra/cts/run_testlab.go
+++ b/infra/cts/run_testlab.go
@@ -43,20 +43,20 @@ var (
 		"A0001": {"22"},
 		// "E5803":       {"22"},    deprecated
 		// "F5121":       {"23"},    deprecated
-		"G8142":       {"25"},
-		"HWMHA":       {"24"},
-		"SH-04H":      {"23"},
-		"athene":      {"23"},
-		"athene_f":    {"23"},
-		"hammerhead":  {"23"},
-		"harpia":      {"23"},
-		"hero2lte":    {"23"},
-		"herolte":     {"24v"},
-		"j1acevelte":  {"22"},
-		"j5lte":       {"23"},
-		"j7xelte":     {"23"},
-		"lucye":       {"24"},
-		"mako":        {"22"},
+		"G8142":      {"25"},
+		"HWMHA":      {"24"},
+		"SH-04H":     {"23"},
+		"athene":     {"23"},
+		"athene_f":   {"23"},
+		"hammerhead": {"23"},
+		"harpia":     {"23"},
+		"hero2lte":   {"23"},
+		"herolte":    {"24v"},
+		"j1acevelte": {"22"},
+		"j5lte":      {"23"},
+		"j7xelte":    {"23"},
+		"lucye":      {"24"},
+		// "mako":        {"22"},   deprecated
 		"osprey_umts": {"22"},
 		"p1":          {"22"},
 		"sailfish":    {"26"},
@@ -83,7 +83,8 @@ const (
 	--app=%s
 	--results-bucket=%s
 	--results-dir=%s
-	--directories-to-pull=/sdcard/Android/data/org.skia.skqpapp
+	--directories-to-pull=/sdcard/Android/data/org.skia.skqp
+	--timeout 30m
 	%s
 `
 	MODEL_VERSION_TMPL   = "--device model=%s,version=%s,orientation=portrait"
diff --git a/platform_tools/android/apps/skqp/src/main/AndroidManifest.xml b/platform_tools/android/apps/skqp/src/main/AndroidManifest.xml
index b080286f2f..f39fddad56 100644
--- a/platform_tools/android/apps/skqp/src/main/AndroidManifest.xml
+++ b/platform_tools/android/apps/skqp/src/main/AndroidManifest.xml
@@ -3,6 +3,9 @@
           package="org.skia.skqp"
           android:versionCode="1"
           android:versionName="1.0">
+
+  <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
+
   <application
       android:allowBackup="false"
       android:theme="@style/AppTheme"
diff --git a/platform_tools/android/apps/skqp/src/main/java/org/skia/skqp/SkQP.java b/platform_tools/android/apps/skqp/src/main/java/org/skia/skqp/SkQP.java
index 8ced43dbe2..1ebfa223da 100644
--- a/platform_tools/android/apps/skqp/src/main/java/org/skia/skqp/SkQP.java
+++ b/platform_tools/android/apps/skqp/src/main/java/org/skia/skqp/SkQP.java
@@ -25,21 +25,19 @@ public class SkQP {
 
     protected static final String kSkiaGM = "SkiaGM_";
     protected static final String kSkiaUnitTests = "Skia_UnitTests";
-    protected static final String LOG_PREFIX = "org.skis.skqp";
+    protected static final String LOG_PREFIX = "org.skia.skqp";
 
     static {
       System.loadLibrary("skqp_app");
     }
 
     protected void runTests(Context context, String outputDirPath) {
-        Log.w(LOG_PREFIX, "Output Dir: " + outputDirPath);
+        Log.i(LOG_PREFIX, "Output Dir: " + outputDirPath);
         File outputDir = new File(outputDirPath);
-        if (outputDir.exists()) {
-          try {
-              deleteDirectoryContents(outputDir);
-          } catch (IOException e) {
-              Log.w(LOG_PREFIX, "DeleteDirectoryContents: " + e.getMessage());
-          }
+        try {
+            ensureEmtpyDirectory(outputDir);
+        } catch (IOException e) {
+            Log.e(LOG_PREFIX, "ensureEmtpyDirectory:" + e.getMessage());
         }
 
         // Note: nInit will initialize the mGMs, mBackends and mUnitTests fields.
@@ -68,28 +66,40 @@ public class SkQP {
                 // Record success for this test.
               }
           }
+        }
+        for (int unitTest = 0; unitTest < mUnitTests.length; unitTest++) {
+            String testName = kSkiaUnitTests + "_" + mUnitTests[unitTest];
+            Log.w(LOG_PREFIX, "Running: " + testName);
+            String[] errors = this.nExecuteUnitTest(unitTest);
+            if (errors != null && errors.length > 0) {
+                for (String error : errors) {
+                  Log.w(LOG_PREFIX, "Error running " + testName + ":" + error);
+                }
+            } else {
+              Log.i(LOG_PREFIX, "Test: " + testName + " finished successfully.");
+            }
+        }
+        Log.i(LOG_PREFIX, "Finished running all tests.");
+        nMakeReport();
+    }
+
+    protected static void ensureEmtpyDirectory(File f) throws IOException {
+      if (f.exists()) {
+        delete(f);
       }
-      for (int unitTest = 0; unitTest < mUnitTests.length; unitTest++) {
-          String testName = kSkiaUnitTests + "_" + mUnitTests[unitTest];
-          Log.w(LOG_PREFIX, "Running: " + testName);
-          String[] errors = this.nExecuteUnitTest(unitTest);
-          if (errors != null && errors.length > 0) {
-              for (String error : errors) {
-                // Record unit test failures.
-              }
-          } else {
-            // Record success.
-          }
+      if (!f.mkdirs()) {
+        throw new IOException("Unable to create directory:" + f.getAbsolutePath());
       }
-      nMakeReport();
     }
 
-    protected static void deleteDirectoryContents(File f) throws IOException {
-      for (File s : f.listFiles()) {
-          if (s.isDirectory()) {
-              deleteDirectoryContents(s);
-          }
-          s.delete();
+    protected static void delete(File f) throws IOException {
+      if (f.isDirectory()) {
+        for (File s : f.listFiles()) {
+          delete(s);
+        }
+      }
+      if (!f.delete()) {
+        throw new IOException("Unable to delete:" + f.getAbsolutePath());
       }
     }
 }
diff --git a/platform_tools/android/apps/skqp/src/main/java/org/skia/skqp/SkQPRunner.java b/platform_tools/android/apps/skqp/src/main/java/org/skia/skqp/SkQPRunner.java
index dc9aea3f19..e7fc49950e 100644
--- a/platform_tools/android/apps/skqp/src/main/java/org/skia/skqp/SkQPRunner.java
+++ b/platform_tools/android/apps/skqp/src/main/java/org/skia/skqp/SkQPRunner.java
@@ -35,9 +35,9 @@ public class SkQPRunner extends Runner {
         Context context = InstrumentationRegistry.getTargetContext();
         File filesDir = context.getFilesDir();
         try {
-            SkQP.deleteDirectoryContents(filesDir);
+            SkQP.ensureEmtpyDirectory(filesDir);
         } catch (IOException e) {
-            Log.w("org.skis.skqp", "DeleteDirectoryContents: " + e.getMessage());
+            Log.w("org.skis.skqp", "ensureEmtpyDirectory: " + e.getMessage());
         }
 
         Resources resources = context.getResources();
diff --git a/tools/skqp/gm_knowledge.cpp b/tools/skqp/gm_knowledge.cpp
index 7670aa9ba7..05a6a5ccd0 100644
--- a/tools/skqp/gm_knowledge.cpp
+++ b/tools/skqp/gm_knowledge.cpp
@@ -126,7 +126,8 @@ bool IsGoodGM(const char* name, skqp::AssetManager* assetManager) {
         && asset_exists(assetManager, SkOSPath::Join(name, PATH_MIN_PNG).c_str());
 }
 
-// Assumes that for each GM foo, asset_manager has files foo/{max,min}.png
+// Assumes that for each GM foo, asset_manager has files foo/{max,min}.png and
+// that the report_directory_path already exists on disk.
 float Check(const uint32_t* pixels,
             int width,
             int height,
@@ -174,7 +175,6 @@ float Check(const uint32_t* pixels,
         gErrors.push_back(Run{SkString(backend), SkString(name), 0, 0});
     }
     if (report_directory_path && badness > 0 && report_directory_path[0] != '\0') {
-        sk_mkdir(report_directory_path);
         if (!backend) {
             backend = "skia";
         }