[Android] Move modules_unittest to isolate.

build/android/pylib/gtest/dispatch.py

 # Copyright (c) 2013 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Dispatches GTests."""
 
 import copy
 import fnmatch
 import glob
 import logging
@@ skipping 17 matching lines @@
 # TODO(frankf): Add more test targets here after making sure we don't
 # blow up the dependency size (and the world).
 _ISOLATE_FILE_PATHS = {
     'base_unittests': 'base/base_unittests.isolate',
     'breakpad_unittests': 'breakpad/breakpad_unittests.isolate',
     'cc_perftests': 'cc/cc_perftests.isolate',
     'components_unittests': 'components/components_unittests.isolate',
     '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',
 }
 
 # Used for filtering large data deps at a finer grain than what's allowed in
 # isolate files since pushing deps to devices is expensive.
 # Wildcards are allowed.
 _DEPS_EXCLUSION_LIST = [
     'chrome/test/data/extensions/api_test',
@@ skipping 17 matching lines @@
 _ISOLATE_SCRIPT = os.path.join(
     constants.DIR_SOURCE_ROOT, 'tools', 'swarm_client', 'isolate.py')
 
 
 def _GenerateDepsDirUsingIsolate(test_suite, build_type):
   """Generate the dependency dir for the test suite using isolate.
 
   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.

 
   isolate_abs_path = os.path.join(constants.DIR_SOURCE_ROOT, isolate_rel_path)
   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',
       '--isolate', isolate_abs_path,
       '--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)
 
   # We're relying on the fact that timestamps are preserved
   # 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:
         raise Exception('isolate remap command did not use hardlinks.')
 
   # 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',
                    excluded_paths)
     for p in excluded_paths:
       if os.path.isdir(p):
         shutil.rmtree(p)
       else:
         os.remove(p)
   finally:
     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):
   """Get a list of absolute paths to test suite targets.
 
   Args:
     exe: if True, use the executable-based test runner.
     option_test_suite: the test_suite specified as an option.
     build_type: 'Release' or 'Debug'.
 
@@ skipping 109 matching lines @@
     attached_devices = android_commands.GetAttachedDevices()
 
   if not attached_devices:
     raise Exception('A device must be attached and online.')
 
   # Reset the test port allocation. It's important to do it before starting
   # to dispatch any tests.
   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):
     return test_runner.TestRunner(
         device,
         options.test_suite,
         options.test_arguments,
         options.timeout,
         options.cleanup_test_files,
         options.tool,
         options.build_type,
         options.webkit,
         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:
     all_tests = [t for t in options.test_filter.split(':') if t]
   else:
     all_tests = GetAllEnabledTests(RunnerFactory, attached_devices)
   num_devices = len(attached_devices)
   tests = [':'.join(all_tests[i::num_devices]) for i in xrange(num_devices)]
   tests = [t for t in tests if t]
 
   # Run tests.
   test_results, exit_code = shard.ShardAndRunTests(
       RunnerFactory, attached_devices, tests, options.build_type,
       test_timeout=None, num_retries=options.num_retries)
 
   report_results.LogFull(
       results=test_results,
       test_type='Unit test',
       test_package=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()
 
   return (test_results, exit_code)
 
 
 def _ListTestSuites():
   """Display a list of available test suites."""
   print 'Available test suites are:'
   for test_suite in gtest_config.STABLE_TEST_SUITES:
@@ skipping 31 matching lines @@
     test_options.test_suite = suite_path
     test_results, test_exit_code = _RunATestSuite(test_options, suite_name)
     results.AddTestRunResults(test_results)
     if test_exit_code and exit_code != constants.ERROR_EXIT_CODE:
       exit_code = test_exit_code
 
   if options.use_xvfb:
     framebuffer.Stop()
 
   return (results, exit_code)