[Android] Move modules_unittest to isolate.

build/android/pylib/gtest/dispatch.py

Index: build/android/pylib/gtest/dispatch.py
diff --git a/build/android/pylib/gtest/dispatch.py b/build/android/pylib/gtest/dispatch.py
index fb1b7999738b0e7b06b7697b1cb06f8e061a23f7..ef82c6a47b1f015d7b721490e197f887d44b30f6 100644
--- a/build/android/pylib/gtest/dispatch.py
+++ b/build/android/pylib/gtest/dispatch.py
@@ -35,6 +35,7 @@ _ISOLATE_FILE_PATHS = {
     'content_browsertests': 'content/content_browsertests.isolate',
     'content_unittests': 'content/content_unittests.isolate',
     'media_unittests': 'media/media_unittests.isolate',
+    'modules_unittests': 'third_party/webrtc/modules/modules_unittests.isolate',

[hellner1, 2013/07/15 20:43:41]

We are still hard coding the .isolate-path. Will this go away? E.g. WebRTC has
more than one unit test target and it would be nice to keep them as they are but
without having an explicit reference in Chromium. The way it now I would
probably need to create one webrtc target for all WebRTC unit tests (maybe that
makes sense regardless?).

[frankf, 2013/07/15 21:46:34]

So, all these targets have identical dependencies? Will this be true in the
future? If so, it might make sense to have a single isolate files. 

Regarding hardcoding these here: If there are many isolate files, and there's
churn in number/location (which shouldn't be), then I can add a command-line
option to take the path to the isolate files. But we like to avoid adding
options if at all possible.


On 2013/07/15 20:43:41, hellner1 wrote:

[hellner1, 2013/07/15 21:57:14]

Can the script use the .isolate paths provided in the .gyp[i]-files? Then it
would be more similar to how it works for PC.

     'net_unittests': 'net/net_unittests.isolate',
     'ui_unittests': 'ui/ui_unittests.isolate',
     'unit_tests': 'chrome/unit_tests.isolate',
@@ -72,13 +73,13 @@ def _GenerateDepsDirUsingIsolate(test_suite, build_type):
   Args:
     test_suite: The test suite basename (e.g. base_unittests).
     build_type: Release/Debug
-
-  Returns:
-    If an isolate file exists, returns path to dependency dir on the host.
-    Otherwise, returns False.
   """
   product_dir = os.path.join(cmd_helper.OutDirectory.get(), build_type)
   assert os.path.isabs(product_dir)
+
+  if os.path.isdir(constants.ISOLATE_DEPS_DIR):
+    shutil.rmtree(constants.ISOLATE_DEPS_DIR)
+
   isolate_rel_path = _ISOLATE_FILE_PATHS.get(test_suite)
   if not isolate_rel_path:
     return False

[hellner1, 2013/07/15 20:43:41]

Function still seems to have a return value. Should this be removed?

[hellner1, 2013/07/15 21:57:14]

Please don't forget this one.

[frankf, 2013/07/15 22:06:06]

Done.

@@ -87,9 +88,6 @@ def _GenerateDepsDirUsingIsolate(test_suite, build_type):
   isolated_abs_path = os.path.join(
       product_dir, '%s.isolated' % test_suite)
   assert os.path.exists(isolate_abs_path)
-  deps_dir = os.path.join(product_dir, 'isolate_deps_dir')
-  if os.path.isdir(deps_dir):
-    shutil.rmtree(deps_dir)
   isolate_cmd = [
       'python', _ISOLATE_SCRIPT,
       'remap',
@@ -97,7 +95,7 @@ def _GenerateDepsDirUsingIsolate(test_suite, build_type):
       '--isolated', isolated_abs_path,
       '-V', 'PRODUCT_DIR=%s' % product_dir,
       '-V', 'OS=android',
-      '--outdir', deps_dir,
+      '--outdir', constants.ISOLATE_DEPS_DIR,
   ]
   assert not cmd_helper.RunCmd(isolate_cmd)
 
@@ -105,12 +103,12 @@ def _GenerateDepsDirUsingIsolate(test_suite, build_type):
   # by the remap command (hardlinked). Otherwise, all the data
   # will be pushed to the device once we move to using time diff
   # instead of md5sum. Perform a sanity check here.
-  for root, _, filenames in os.walk(deps_dir):
+  for root, _, filenames in os.walk(constants.ISOLATE_DEPS_DIR):
     if filenames:
       linked_file = os.path.join(root, filenames[0])
       orig_file = os.path.join(
           constants.DIR_SOURCE_ROOT,
-          os.path.relpath(linked_file, deps_dir))
+          os.path.relpath(linked_file, constants.ISOLATE_DEPS_DIR))
       if os.stat(linked_file).st_ino == os.stat(orig_file).st_ino:
         break
       else:
@@ -119,7 +117,7 @@ def _GenerateDepsDirUsingIsolate(test_suite, build_type):
   # Delete excluded files as defined by _DEPS_EXCLUSION_LIST.
   old_cwd = os.getcwd()
   try:
-    os.chdir(deps_dir)
+    os.chdir(constants.ISOLATE_DEPS_DIR)
     excluded_paths = [x for y in _DEPS_EXCLUSION_LIST for x in glob.glob(y)]
     if excluded_paths:
       logging.info('Excluding the following from dependency list: %s',
@@ -133,21 +131,20 @@ def _GenerateDepsDirUsingIsolate(test_suite, build_type):
     os.chdir(old_cwd)
 
   # On Android, all pak files need to be in the top-level 'paks' directory.
-  paks_dir = os.path.join(deps_dir, 'paks')
+  paks_dir = os.path.join(constants.ISOLATE_DEPS_DIR, 'paks')
   os.mkdir(paks_dir)
-  for root, _, filenames in os.walk(os.path.join(deps_dir, 'out')):
+  for root, _, filenames in os.walk(os.path.join(constants.ISOLATE_DEPS_DIR,
+                                                 'out')):
     for filename in fnmatch.filter(filenames, '*.pak'):
       shutil.move(os.path.join(root, filename), paks_dir)
 
   # Move everything in PRODUCT_DIR to top level.
-  deps_product_dir = os.path.join(deps_dir, 'out', build_type)
+  deps_product_dir = os.path.join(constants.ISOLATE_DEPS_DIR, 'out', build_type)
   if os.path.isdir(deps_product_dir):
     for p in os.listdir(deps_product_dir):
-      shutil.move(os.path.join(deps_product_dir, p), deps_dir)
+      shutil.move(os.path.join(deps_product_dir, p), constants.ISOLATE_DEPS_DIR)
     os.rmdir(deps_product_dir)
-    os.rmdir(os.path.join(deps_dir, 'out'))
-
-  return deps_dir
+    os.rmdir(os.path.join(constants.ISOLATE_DEPS_DIR, 'out'))
 
 
 def _FullyQualifiedTestSuites(exe, option_test_suite, build_type):
@@ -277,7 +274,7 @@ def _RunATestSuite(options, suite_name):
   if not ports.ResetTestServerPortAllocation():
     raise Exception('Failed to reset test server port.')
 
-  deps_dir = _GenerateDepsDirUsingIsolate(suite_name, options.build_type)
+  _GenerateDepsDirUsingIsolate(suite_name, options.build_type)
 
   # Constructs a new TestRunner with the current options.
   def RunnerFactory(device, shard_index):
@@ -293,8 +290,7 @@ def _RunATestSuite(options, suite_name):
         options.push_deps,
         constants.GTEST_TEST_PACKAGE_NAME,
         constants.GTEST_TEST_ACTIVITY_NAME,
-        constants.GTEST_COMMAND_LINE_FILE,
-        deps_dir=deps_dir)
+        constants.GTEST_COMMAND_LINE_FILE)
 
   # Get tests and split them up based on the number of devices.
   if options.test_filter:
@@ -317,6 +313,9 @@ def _RunATestSuite(options, suite_name):
       build_type=options.build_type,
       flakiness_server=options.flakiness_dashboard_server)
 
+  if os.path.isdir(constants.ISOLATE_DEPS_DIR):
+    shutil.rmtree(constants.ISOLATE_DEPS_DIR)
+
   for buildbot_emulator in buildbot_emulators:
     buildbot_emulator.Shutdown()