Add option to push files to device using isolate for instrumentation tests.

build/android/pylib/instrumentation/setup.py

Index: build/android/pylib/instrumentation/setup.py
diff --git a/build/android/pylib/instrumentation/setup.py b/build/android/pylib/instrumentation/setup.py
index 57286e2f793dd644ecc9c9b57353f1a725fb049d..55b20f295ec2b6b511cf242ea68273f80e4293cc 100644
--- a/build/android/pylib/instrumentation/setup.py
+++ b/build/android/pylib/instrumentation/setup.py
@@ -7,11 +7,60 @@
 import logging
 import os
 
+from pylib import constants
+from pylib import valgrind_tools
+
+from pylib.base import base_setup
+from pylib.device import device_utils
 from pylib.instrumentation import test_package
 from pylib.instrumentation import test_runner
 
+DEVICE_DATA_DIR = 'chrome/test/data'
+
+ISOLATE_FILE_PATHS = {
+    'AndroidWebViewTest': 'android_webview/android_webview_test_apk.isolate',
+    'ChromeShellTest': 'chrome/chrome_shell_test_apk.isolate',
+    'ContentShellTest': 'content/content_shell_test_apk.isolate',
+    'MojoTest': 'mojo/mojo_test_apk.isolate',

[jbudorick, 2014/11/03 15:52:32]

I'll talk to you about this today, but it's probably safest to handle Mojo in a
separate CL. It moved into its own repository that gets synced into chromium a
little while back, and it's got its own bots.

[mikecase (-- gone --), 2014/11/03 19:20:05]

Acknowledged.

+}
+
+DEPS_EXCLUSION_LIST = []
+
+def _PushExtraSuiteDataDeps(device, test_apk):
+  """Pushes some extra data files/dirs needed by some test suite.
+
+  Args:
+    test_apk: The test suite basename for which to return file paths.
+  """
+  if test_apk in ['ChromeTest', 'ContentShellTest']:
+    test_files = 'net/data/ssl/certificates'

[jbudorick, 2014/11/03 15:52:33]

Why isn't this directory handled in the .isolate files?

[mikecase (-- gone --), 2014/11/03 19:20:05]

I would prefer to only have 1 function that does any data pushing. The reason I
have _PushExtraSuiteDataDeps is because for example with the
ContentShellTests...

src/content/test/data/android/device_files/ 
to end up in
sdcard/CHROME/TEST/DATA/content/test/data/android/device_files

BUT

src/net/data/ssl/certificates/
to end up in 
sdcard/net/data/sll/certificates/

This means that if we used isolate for everything, we would have to push half of
ISOLATE_DEPS_DIR to sdcard/chrome/test/data/ and the other half of
ISOLATE_DEPS_DIR to sdcard/

Three solution I can think of but none are clean.

1. Leave _PushExtraSuiteDataDeps as is and don't use isolate to push
src/net/data/sll/certificates/

2. After running isolate remap, go into the ISOLATE_DEPS_DIR and change its
structure, prepending chrome/test/data/ to some file paths.

3. Have everything pushed to the base sdcard/ directory. This would include a
good number of changes to individual tests that hardcode where they grab the
dependencies from. But mostly would require lots of LGTMs. 

+    host_device_file_tuple = [
+        (os.path.join(constants.DIR_SOURCE_ROOT, test_files),
+         os.path.join(device.GetExternalStoragePath(), test_files))]
+    device.PushChangedFiles(host_device_file_tuple)
+
+
+def _PushDataDeps(device, test_options):
+  valgrind_tools.PushFilesForTool(test_options.tool, device)
+
+  host_device_file_tuples = []
+  for dest_host_pair in test_options.test_data:
+    dst_src = dest_host_pair.split(':', 1)
+    dst_layer = dst_src[0]
+    host_src = dst_src[1]
+    host_test_files_path = os.path.join(constants.DIR_SOURCE_ROOT, host_src)
+    if os.path.exists(host_test_files_path):
+      host_device_file_tuples += [(
+          host_test_files_path,
+          '%s/%s/%s' % (
+              device.GetExternalStoragePath(),
+              DEVICE_DATA_DIR,
+              dst_layer))]
+  if host_device_file_tuples:
+    device.PushChangedFiles(host_device_file_tuples)
+
 
-def Setup(test_options):
+def Setup(test_options, devices):
   """Create and return the test runner factory and tests.
 
   Args:
@@ -34,6 +83,26 @@ def Setup(test_options):
   if not tests:
     logging.error('No instrumentation tests to run with current args.')
 
+  # TODO(mikecase): Remove this function once everything uses
+  # base_setup.PushDataDeps instead.
+  if test_options.test_data:

[jbudorick, 2014/11/03 15:52:33]

This is only staying in here until you switch the bots over, right?

[mikecase (-- gone --), 2014/11/03 19:20:05]

Yes, once the recipes switch from using --test-data to isolate I will remove the
function _PushDataDeps. 

_PushExtraSuiteDataDeps will probably have to stay for the time being (read
comment above).

+    device_utils.DeviceUtils.parallel(devices).pMap(
+        _PushDataDeps, test_options)
+  else:
+    base_setup.GenerateDepsDirUsingIsolate(test_options.test_apk,
+                                           test_options.isolate_file_path,
+                                           ISOLATE_FILE_PATHS,
+                                           DEPS_EXCLUSION_LIST)
+    def DeviceDir(device):
+      return os.path.join(device.GetExternalStoragePath(), DEVICE_DATA_DIR)

[jbudorick, 2014/11/03 15:52:32]

What happened to trying to get rid of DEVICE_DATA_DIR?

[mikecase (-- gone --), 2014/11/03 19:20:05]

If I do it, it will be in a different CL. It will require a lot of owner
reviews, and I would prefer to get this in first. 

+
+    device_utils.DeviceUtils.parallel(devices).pMap(
+        (lambda d, opts: base_setup.PushDataDeps(d, DeviceDir(d), opts)),

[jbudorick, 2014/11/03 15:52:32]

again w.r.t defining DeviceDir vs defining the lambda.

[mikecase (-- gone --), 2014/11/03 19:20:05]

Done.

+        test_options)
+
+  device_utils.DeviceUtils.parallel(devices).pMap(
+      _PushExtraSuiteDataDeps, test_options.test_apk)
+
   def TestRunnerFactory(device, shard_index):
     return test_runner.TestRunner(test_options, device, shard_index,
                                   test_pkg)