[Instrumentation Test Speed] Install apks in parallel with concurrent_adb enabled

build/android/pylib/local/device/local_device_instrumentation_test_run.py

Index: build/android/pylib/local/device/local_device_instrumentation_test_run.py
diff --git a/build/android/pylib/local/device/local_device_instrumentation_test_run.py b/build/android/pylib/local/device/local_device_instrumentation_test_run.py
index a8d9f5e6737828ccf5010f9070d4cdb553f407dd..37ecc9873b665bf2d2d66437c2792e19976bb83f 100644
--- a/build/android/pylib/local/device/local_device_instrumentation_test_run.py
+++ b/build/android/pylib/local/device/local_device_instrumentation_test_run.py
@@ -76,29 +76,44 @@ class LocalDeviceInstrumentationTestRun(
     @trace_event.traced
     def individual_device_set_up(dev, host_device_tuples):
       def install_apk():

[jbudorick, 2017/04/05 16:32:29]

This function should no longer exist. We should just create steps earlier in
individual_device_set_up's scope and append to it directly.

[shenghuazhang, 2017/04/05 18:57:05]

Done.

+        steps = []
+        def install_helper(apk, permissions):
+          return lambda: dev.Install(apk, permissions=permissions)
         if self._test_instance.apk_under_test:
           if self._test_instance.apk_under_test_incremental_install_script:
-            local_device_test_run.IncrementalInstall(
-                dev,
-                self._test_instance.apk_under_test,
-                self._test_instance.apk_under_test_incremental_install_script)
+            steps.append(lambda: local_device_test_run.IncrementalInstall(
+                                     dev,
+                                     self._test_instance.apk_under_test,
+                                     self._test_instance.
+                                     apk_under_test_incremental_install_script))
           else:
             permissions = self._test_instance.apk_under_test.GetPermissions()
-            dev.Install(self._test_instance.apk_under_test,
-                        permissions=permissions)
+            steps.append(install_helper(self._test_instance.apk_under_test,

[shenghuazhang, 2017/04/05 03:00:16]

Seems like python doesn't create closure binding the value 'permissions' to
dev.Install(apk, permissions=permissions), and if pass this:

steps.append(lambda: dev.Install(..., permissions=permission))

it will throw SecurityException e.g. 'Package org.chromium.chrome has not
requested permission android.permission.READ_LOGS'.

So use the install_helper here and below.

+                                        permissions))
 
         if self._test_instance.test_apk_incremental_install_script:
-          local_device_test_run.IncrementalInstall(
-              dev,
-              self._test_instance.test_apk,
-              self._test_instance.test_apk_incremental_install_script)
+          steps.append(lambda: local_device_test_run.IncrementalInstall(
+                                   dev,
+                                   self._test_instance.test_apk,
+                                   self._test_instance.
+                                   test_apk_incremental_install_script))
         else:
           permissions = self._test_instance.test_apk.GetPermissions()
-          dev.Install(self._test_instance.test_apk, permissions=permissions)
+          steps.append(install_helper(self._test_instance.test_apk,
+                                      permissions))
 
-        for apk in self._test_instance.additional_apks:
-          dev.Install(apk)
+        steps.extend([install_helper(apk, ())
+                          for apk in self._test_instance.additional_apks])

[mikecase (-- gone --), 2017/04/05 15:46:10]

nit: some of the indentation looks off.

[shenghuazhang, 2017/04/05 18:57:05]

Done.

 
+        if self._env.concurrent_adb:

[jbudorick, 2017/04/05 16:32:29]

Again: we shouldn't need to branch based on this here.

[shenghuazhang, 2017/04/05 18:57:05]

Done.

+          return steps
+        else:
+          def install_apk_func():

[mikecase (-- gone --), 2017/04/05 15:46:10]

Would rename to something like...

sequential_install_function

...just so its purpose is clearer.

[jbudorick, 2017/04/05 16:32:29]

As mentioned, this shouldn't exist. Use the same steps.

[shenghuazhang, 2017/04/05 18:57:05]

Done.

+            for step in steps:
+              step()
+          return [install_apk_func]
+
+      def set_debug_app():
         # Set debug app in order to enable reading command line flags on user
         # builds
         if self._test_instance.flags:
@@ -165,8 +180,9 @@ class LocalDeviceInstrumentationTestRun(
         valgrind_tools.SetChromeTimeoutScale(
             dev, self._test_instance.timeout_scale)
 
-      steps = (install_apk, edit_shared_prefs, push_test_data,
-               create_flag_changer)
+      install_apk_steps = install_apk()
+      steps = install_apk_steps + [set_debug_app, edit_shared_prefs,

[mikecase (-- gone --), 2017/04/05 15:46:10]

Could something go wrong is set_debug_app step is run before the apk is
installed? Unsure if the command work if package not installed yet. It might.

[mikecase (-- gone --), 2017/04/05 16:29:06]

Tested it. Seems to work fine.

[shenghuazhang, 2017/04/05 18:57:05]

Thanks for bringing this up! I just do a quick research about it. The
'--persistent' option of the set-debug-app command would 'retain this value',
s.t., this option survives app re-installations and even uninstall-install
sequence. So I think it would be fine to run set_debug_app step before apk
installed.

Referred from the blog:
http://android-dev-life.blogspot.com/2015/02/do-you-adb-shell-am-set-debug-ap...

+                                   push_test_data, create_flag_changer]
       if self._env.concurrent_adb:
         reraiser_thread.RunAsync(steps)
       else: