👔 Move side-loaded test data /sdcard -> /sdcard/gtestdata

base/base_paths_android.cc

Index: base/base_paths_android.cc
diff --git a/base/base_paths_android.cc b/base/base_paths_android.cc
index 62b07dde07340f4d5a85071ff56d87f461ee0d2b..8a875d3a18e4a5572eda6c4d8356b40e80f5c7fe 100644
--- a/base/base_paths_android.cc
+++ b/base/base_paths_android.cc
@@ -40,7 +40,10 @@ bool PathProviderAndroid(int key, FilePath* result) {
       return base::android::GetNativeLibraryDirectory(result);
     case base::DIR_SOURCE_ROOT:
       // This const is only used for tests.
-      return base::android::GetExternalStorageDirectory(result);
+      if (!base::android::GetExternalStorageDirectory(result))
+        return false;
+      *result = result->Append(FILE_PATH_LITERAL("gtestdata"));

[jbudorick, 2016/04/27 19:59:01]

What's the motivation behind putting this in "gtestdata" rather than using the
external storage directory directly? This, tbh, is the scariest part of this
change.

[jbudorick, 2016/04/27 20:00:30]

(also, in the event we keep it, "gtestdata" is a bad name. Instrumentation tests
use this, too.)

[agrieve, 2016/04/27 20:26:31]

chromium_test_root?

[agrieve, 2016/04/27 20:26:31]

It's to avoid wiping the entire /sdcard when running tests. Alternatively, we
could have some custom code that deletes all files except directories that start
with a capital letter. 

Still feels like just putting the test data in a custom directory is the nicer
way to go though. I've on occasion used my personal android phone to run a test,
and I'd be upset if it wiped my sdcard.

[jbudorick, 2016/04/27 20:41:36]

eh, I'd rather have our own sdcard subdirectory than something like this.
I would generally advise against this. I do try to warn people whenever I tell
them to do something that will nuke their SD card (and the test runner doesn't
do so, obviously).

+      return true;
     case base::DIR_USER_DESKTOP:
       // Android doesn't support GetUserDesktop.
       NOTIMPLEMENTED();