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

base/test/test_support_android.cc

Index: base/test/test_support_android.cc
diff --git a/base/test/test_support_android.cc b/base/test/test_support_android.cc
index f8441bd1ba5f9d75fe8e52258c6d339f24763514..35e2965ed555b595747d731d3a8162feb52c08e5 100644
--- a/base/test/test_support_android.cc
+++ b/base/test/test_support_android.cc
@@ -18,6 +18,8 @@
 
 namespace {
 
+base::FilePath* g_test_data_dir = nullptr;

[jbudorick, 2016/05/18 14:23:38]

I'm really not crazy about introducing a global here or about introducing an
implicit ordering requirement between GetTestProviderPath and InitPathProvider,
although I suppose it was doing that before (in a less explicit way) for
DIR_ANDROID_APP_DATA.

[agrieve, 2016/05/19 02:11:53]

I think GetTestProvider being a non-exported function makes this okay. Clients
can't mess it up. 

[jbudorick, 2016/05/24 19:13:52]

ah, I didn't catch that before.

+
 struct RunState {
   RunState(base::MessagePump::Delegate* delegate, int run_depth)
       : delegate(delegate),
@@ -132,13 +134,14 @@ std::unique_ptr<base::MessagePump> CreateMessagePumpForUIStub() {
   return std::unique_ptr<base::MessagePump>(new MessagePumpForUIStub());
 };
 
-// Provides the test path for DIR_MODULE and DIR_ANDROID_APP_DATA.
+// Provides the test path for DIR_SOURCE_ROOT and DIR_ANDROID_APP_DATA.
 bool GetTestProviderPath(int key, base::FilePath* result) {
   switch (key) {
-    case base::DIR_ANDROID_APP_DATA: {
-      // For tests, app data is put in external storage.
-      return base::android::GetExternalStorageDirectory(result);
-    }
+    case base::DIR_ANDROID_APP_DATA:
+    case base::DIR_SOURCE_ROOT:
+      CHECK(g_test_data_dir != nullptr);

[jbudorick, 2016/05/18 14:23:38]

Can this just be CHECK(g_test_data_dir) ?

It's been a while since I've written C++, and consequently I haven't worked with
nullptr much :|

[agrieve, 2016/05/19 02:11:53]

That would work too, but I don't love the implicit pointer -> boolean cast that
that does. Couldn't find anything in the style guide that talks about it though.

[jbudorick, 2016/05/24 19:13:52]

Acknowledged.

+      *result = *g_test_data_dir;
+      return true;
     default:
       return false;
   }
@@ -166,8 +169,9 @@ void InitAndroidTestLogging() {
                        false);   // Tick count
 }
 
-void InitAndroidTestPaths() {
-  InitPathProvider(DIR_MODULE);
+void InitAndroidTestPaths(const FilePath& test_data_dir) {
+  g_test_data_dir = new FilePath(test_data_dir);
+  InitPathProvider(DIR_SOURCE_ROOT);
   InitPathProvider(DIR_ANDROID_APP_DATA);
 }
 
@@ -179,7 +183,6 @@ void InitAndroidTestMessageLoop() {
 void InitAndroidTest() {
   if (!base::AndroidIsChildProcess()) {
     InitAndroidTestLogging();
-    InitAndroidTestPaths();
   }
   InitAndroidTestMessageLoop();
 }