[Android] Split uiautomator test runner from instrumentation.

Also, some minor refactoring.

BUG=

Review URL: https://codereview.chromium.org/13989007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@193967 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 193 insertions, 121 deletions

diff --git a/build/android/pylib/android_commands.py b/build/android/pylib/android_commands.py
index 1d80642..2fa9d5e0 100644
--- a/build/android/pylib/android_commands.py
+++ b/build/android/pylib/android_commands.py
@@ -1272,6 +1272,9 @@ class AndroidCommands(object):
     # convention and doesn't terminate with INSTRUMENTATION_CODE.
     # Just assume the first result is valid.
     (test_results, _) = am_instrument_parser.ParseAmInstrumentOutput(output)
+    if not test_results:
+      raise errors.InstrumentationError(
+          'no test results... device setup correctly?')
     return test_results[0]
 
 
diff --git a/build/android/pylib/base/base_test_runner.py b/build/android/pylib/base/base_test_runner.py
index 19bf4f3..9316a0f 100644
--- a/build/android/pylib/base/base_test_runner.py
+++ b/build/android/pylib/base/base_test_runner.py
@@ -2,6 +2,8 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+"""Base class for running tests on a single device."""
+
 import contextlib
 import httplib
 import logging
diff --git a/build/android/pylib/base/shard.py b/build/android/pylib/base/shard.py
index 03c72ef..61820ef 100644
--- a/build/android/pylib/base/shard.py
+++ b/build/android/pylib/base/shard.py
@@ -276,6 +276,7 @@ def ShardAndRunTests(runner_factory, devices, tests, build_type='Debug',
   Returns:
     A base_test_result.TestRunResults object.
   """
+  logging.info('Will run %d tests: %s', len(tests), str(tests))
   forwarder.Forwarder.KillHost(build_type)
   runners = _CreateRunners(runner_factory, devices, setup_timeout)
   try:
diff --git a/build/android/pylib/host_driven/run_python_tests.py b/build/android/pylib/host_driven/run_python_tests.py
index 5031cb2..f21962a 100644
--- a/build/android/pylib/host_driven/run_python_tests.py
+++ b/build/android/pylib/host_driven/run_python_tests.py
@@ -71,13 +71,12 @@ def DispatchPythonTests(options):
   logging.debug('All available tests: ' + str(test_names))
 
   available_tests = test_collection.GetAvailableTests(
-      options.annotation, options.test_filter)
+      options.annotations, options.test_filter)
 
   if not available_tests:
     logging.warning('No Python tests to run with current args.')
     return base_test_result.TestRunResults()
 
-  available_tests *= options.number_of_runs
   test_names = [t.qualified_name for t in available_tests]
   logging.debug('Final list of tests to run: ' + str(test_names))
 
diff --git a/build/android/pylib/instrumentation/dispatch.py b/build/android/pylib/instrumentation/dispatch.py
index 53c9d82..ff4e367 100644
--- a/build/android/pylib/instrumentation/dispatch.py
+++ b/build/android/pylib/instrumentation/dispatch.py
@@ -10,7 +10,6 @@ import os
 from pylib import android_commands
 from pylib.base import base_test_result
 from pylib.base import shard
-from pylib.uiautomator import test_package as uiautomator_package
 
 import test_package
 import test_runner
@@ -31,74 +30,29 @@ def Dispatch(options):
   Raises:
     Exception: when there are no attached devices.
   """
-  is_uiautomator_test = False
-  if hasattr(options, 'uiautomator_jar'):
-    test_pkg = uiautomator_package.TestPackage(
-        options.uiautomator_jar, options.uiautomator_info_jar)
-    is_uiautomator_test = True
-  else:
-    test_pkg = test_package.TestPackage(options.test_apk_path,
-                                        options.test_apk_jar_path)
-  # The default annotation for tests which do not have any sizes annotation.
-  default_size_annotation = 'SmallTest'
-
-  def _GetTestsMissingAnnotation(test_pkg):
-    test_size_annotations = frozenset(['Smoke', 'SmallTest', 'MediumTest',
-                                       'LargeTest', 'EnormousTest', 'FlakyTest',
-                                       'DisabledTest', 'Manual', 'PerfTest'])
-    tests_missing_annotations = []
-    for test_method in test_pkg.GetTestMethods():
-      annotations = frozenset(test_pkg.GetTestAnnotations(test_method))
-      if (annotations.isdisjoint(test_size_annotations) and
-          not test_pkg.IsPythonDrivenTest(test_method)):
-        tests_missing_annotations.append(test_method)
-    return sorted(tests_missing_annotations)
-
-  if options.annotation:
-    available_tests = test_pkg.GetAnnotatedTests(options.annotation)
-    if options.annotation.count(default_size_annotation) > 0:
-      tests_missing_annotations = _GetTestsMissingAnnotation(test_pkg)
-      if tests_missing_annotations:
-        logging.warning('The following tests do not contain any annotation. '
-                        'Assuming "%s":\n%s',
-                        default_size_annotation,
-                        '\n'.join(tests_missing_annotations))
-        available_tests += tests_missing_annotations
-  else:
-    available_tests = [m for m in test_pkg.GetTestMethods()
-                       if not test_pkg.IsPythonDrivenTest(m)]
-
-  tests = []
-  if options.test_filter:
-    # |available_tests| are in adb instrument format: package.path.class#test.
-    filter_without_hash = options.test_filter.replace('#', '.')
-    tests = [t for t in available_tests
-             if filter_without_hash in t.replace('#', '.')]
-  else:
-    tests = available_tests
-
+  test_pkg = test_package.TestPackage(options.test_apk_path,
+                                      options.test_apk_jar_path)
+  tests = test_pkg._GetAllMatchingTests(
+      options.annotations, options.test_filter)
   if not tests:
     logging.warning('No instrumentation tests to run with current args.')
     return base_test_result.TestRunResults()
 
-  tests *= options.number_of_runs
-
   attached_devices = android_commands.GetAttachedDevices()
-
   if not attached_devices:
     raise Exception('There are no devices online.')
+
   if options.device:
+    assert options.device in attached_devices
     attached_devices = [options.device]
 
-  logging.info('Will run: %s', str(tests))
-
   if len(attached_devices) > 1 and options.wait_for_debugger:
     logging.warning('Debugger can not be sharded, using first available device')
     attached_devices = attached_devices[:1]
 
   def TestRunnerFactory(device, shard_index):
     return test_runner.TestRunner(
-        options, device, shard_index, test_pkg, [], is_uiautomator_test)
+        options, device, shard_index, test_pkg, [])
 
   return shard.ShardAndRunTests(TestRunnerFactory, attached_devices, tests,
                                 options.build_type)
diff --git a/build/android/pylib/instrumentation/test_jar.py b/build/android/pylib/instrumentation/test_jar.py
index ef01656..e116a1b 100644
--- a/build/android/pylib/instrumentation/test_jar.py
+++ b/build/android/pylib/instrumentation/test_jar.py
@@ -19,22 +19,27 @@ PICKLE_FORMAT_VERSION = 1
 
 
 class TestJar(object):
+  _ANNOTATIONS = frozenset(
+      ['Smoke', 'SmallTest', 'MediumTest', 'LargeTest', 'EnormousTest',
+       'FlakyTest', 'DisabledTest', 'Manual', 'PerfTest'])
+  _DEFAULT_ANNOTATION = 'SmallTest'
+  _PROGUARD_CLASS_RE = re.compile(r'\s*?- Program class:\s*([\S]+)$')
+  _PROGUARD_METHOD_RE = re.compile(r'\s*?- Method:\s*(\S*)[(].*$')
+  _PROGUARD_ANNOTATION_RE = re.compile(r'\s*?- Annotation \[L(\S*);\]:$')
+  _PROGUARD_ANNOTATION_CONST_RE = (
+      re.compile(r'\s*?- Constant element value.*$'))
+  _PROGUARD_ANNOTATION_VALUE_RE = re.compile(r'\s*?- \S+? \[(.*)\]$')
+
   def __init__(self, jar_path):
+    if not os.path.exists(jar_path):
+      raise Exception('%s not found, please build it' % jar_path)
+
     sdk_root = os.getenv('ANDROID_SDK_ROOT', constants.ANDROID_SDK_ROOT)
     self._PROGUARD_PATH = os.path.join(sdk_root,
                                        'tools/proguard/bin/proguard.sh')
     if not os.path.exists(self._PROGUARD_PATH):
       self._PROGUARD_PATH = os.path.join(os.environ['ANDROID_BUILD_TOP'],
                                          'external/proguard/bin/proguard.sh')
-    self._PROGUARD_CLASS_RE = re.compile(r'\s*?- Program class:\s*([\S]+)$')
-    self._PROGUARD_METHOD_RE = re.compile(r'\s*?- Method:\s*(\S*)[(].*$')
-    self._PROGUARD_ANNOTATION_RE = re.compile(r'\s*?- Annotation \[L(\S*);\]:$')
-    self._PROGUARD_ANNOTATION_CONST_RE = (
-        re.compile(r'\s*?- Constant element value.*$'))
-    self._PROGUARD_ANNOTATION_VALUE_RE = re.compile(r'\s*?- \S+? \[(.*)\]$')
-
-    if not os.path.exists(jar_path):
-      raise Exception('%s not found, please build it' % jar_path)
     self._jar_path = jar_path
     self._annotation_map = collections.defaultdict(list)
     self._pickled_proguard_name = self._jar_path + '-proguard.pickle'
@@ -158,6 +163,52 @@ class TestJar(object):
     """Returns a list of all test methods in this apk as Class#testMethod."""
     return self._test_methods
 
+  def _GetTestsMissingAnnotation(self):
+    """Get a list of test methods with no known annotations."""
+    tests_missing_annotations = []
+    for test_method in self.GetTestMethods():
+      annotations_ = frozenset(self.GetTestAnnotations(test_method))
+      if (annotations_.isdisjoint(self._ANNOTATIONS) and
+          not self.IsPythonDrivenTest(test_method)):
+        tests_missing_annotations.append(test_method)
+    return sorted(tests_missing_annotations)
+
+  def _GetAllMatchingTests(self, annotation_filter_list, test_filter):
+    """Get a list of tests matching any of the annotations and the filter.
+
+    Args:
+      annotation_filter_list: List of test annotations. A test must have at
+        least one of the these annotations. A test without any annotations
+        is considered to be SmallTest.
+      test_filter: Filter used for partial matching on the test method names.
+
+    Returns:
+      List of all matching tests.
+    """
+    if annotation_filter_list:
+      available_tests = self.GetAnnotatedTests(annotation_filter_list)
+      # Include un-annotated tests in SmallTest.
+      if annotation_filter_list.count(self._DEFAULT_ANNOTATION) > 0:
+        for test in self._GetTestsMissingAnnotation():
+          logging.warning(
+              '%s has no annotations. Assuming "%s".', test,
+              self._DEFAULT_ANNOTATION)
+          available_tests.append(test)
+    else:
+      available_tests = [m for m in self.GetTestMethods()
+                         if not self.IsPythonDrivenTest(m)]
+
+    tests = []
+    if test_filter:
+      # |available_tests| are in adb instrument format: package.path.class#test.
+      filter_without_hash = test_filter.replace('#', '.')
+      tests = [t for t in available_tests
+               if filter_without_hash in t.replace('#', '.')]
+    else:
+      tests = available_tests
+
+    return tests
+
   @staticmethod
   def IsPythonDrivenTest(test):
     return 'pythonDrivenTests' in test
diff --git a/build/android/pylib/instrumentation/test_runner.py b/build/android/pylib/instrumentation/test_runner.py
index e6b6968..1dc164c 100644
--- a/build/android/pylib/instrumentation/test_runner.py
+++ b/build/android/pylib/instrumentation/test_runner.py
@@ -2,7 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-"""Runs the Java tests. See more information on run_instrumentation_tests.py."""
+"""Class for running instrumentation tests on a single device."""
 
 import logging
 import os
@@ -53,8 +53,7 @@ class TestRunner(base_test_runner.BaseTestRunner):
                                        '/chrome-profile*')
   _DEVICE_HAS_TEST_FILES = {}
 
-  def __init__(self, options, device, shard_index, test_pkg,
-               ports_to_forward, is_uiautomator_test=False):
+  def __init__(self, options, device, shard_index, test_pkg, ports_to_forward):
     """Create a new TestRunner.
 
     Args:
@@ -72,7 +71,6 @@ class TestRunner(base_test_runner.BaseTestRunner):
       test_pkg: A TestPackage object.
       ports_to_forward: A list of port numbers for which to set up forwarders.
                         Can be optionally requested by a test case.
-      is_uiautomator_test: Whether this is a uiautomator test.
     """
     super(TestRunner, self).__init__(device, options.tool, options.build_type)
     self._lighttp_port = constants.LIGHTTPD_RANDOM_PORT_FIRST + shard_index
@@ -85,15 +83,10 @@ class TestRunner(base_test_runner.BaseTestRunner):
     self.disable_assertions = options.disable_assertions
     self.test_pkg = test_pkg
     self.ports_to_forward = ports_to_forward
-    self.is_uiautomator_test = is_uiautomator_test
-    if self.is_uiautomator_test:
-      self.package_name = options.package_name
-    else:
-      self.install_apk = options.install_apk
-
+    self.install_apk = options.install_apk
     self.forwarder = None
 
-  #override.
+  #override
   def PushDependencies(self):
     # TODO(frankf): Implement a general approach for copying/installing
     # once across test runners.
@@ -120,7 +113,7 @@ class TestRunner(base_test_runner.BaseTestRunner):
         self.adb.PushIfNeeded(host_test_files_path,
                               self.adb.GetExternalStorage() + '/' +
                               TestRunner._DEVICE_DATA_DIR + '/' + dst_layer)
-    if self.is_uiautomator_test or self.install_apk:
+    if self.install_apk:
       self.test_pkg.Install(self.adb)
     self.tool.CopyFiles()
     TestRunner._DEVICE_HAS_TEST_FILES[self.device] = True
@@ -310,25 +303,12 @@ class TestRunner(base_test_runner.BaseTestRunner):
       return 3 * 60
     return 1 * 60
 
-  def _RunUIAutomatorTest(self, test, timeout):
-    """Runs a single uiautomator test.
-
-    Args:
-      test: Test class/method.
-      timeout: Timeout time in seconds.
+  def _RunTest(self, test, timeout):
+    return self.adb.RunInstrumentationTest(
+        test, self.test_pkg.GetPackageName(),
+        self._GetInstrumentationArgs(), timeout)
 
-    Returns:
-      An instance of am_instrument_parser.TestResult object.
-    """
-    self.adb.ClearApplicationState(self.package_name)
-    if 'Feature:FirstRunExperience' in self.test_pkg.GetTestAnnotations(test):
-      self.flags.RemoveFlags(['--disable-fre'])
-    else:
-      self.flags.AddFlags(['--disable-fre'])
-    return self.adb.RunUIAutomatorTest(
-        test, self.test_pkg.GetPackageName(), timeout)
-
-  #override.
+  #override
   def RunTest(self, test):
     raw_result = None
     start_date_ms = None
@@ -339,14 +319,7 @@ class TestRunner(base_test_runner.BaseTestRunner):
     try:
       self.TestSetup(test)
       start_date_ms = int(time.time()) * 1000
-
-      if self.is_uiautomator_test:
-        raw_result = self._RunUIAutomatorTest(test, timeout)
-      else:
-        raw_result = self.adb.RunInstrumentationTest(
-            test, self.test_pkg.GetPackageName(),
-            self._GetInstrumentationArgs(), timeout)
-
+      raw_result = self._RunTest(test, timeout)
       duration_ms = int(time.time()) * 1000 - start_date_ms
       status_code = raw_result.GetStatusCode()
       if status_code:
diff --git a/build/android/pylib/uiautomator/dispatch.py b/build/android/pylib/uiautomator/dispatch.py
new file mode 100644
index 0000000..7dd6990
--- /dev/null
+++ b/build/android/pylib/uiautomator/dispatch.py
@@ -0,0 +1,54 @@
+# 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 the uiautomator tests."""
+
+import logging
+import os
+
+from pylib import android_commands
+from pylib.base import base_test_result
+from pylib.base import shard
+
+import test_package
+import test_runner
+
+
+def Dispatch(options):
+  """Dispatches uiautomator tests onto connected device(s).
+
+  If possible, this method will attempt to shard the tests to
+  all connected devices. Otherwise, dispatch and run tests on one device.
+
+  Args:
+    options: Command line options.
+
+  Returns:
+    A TestRunResults object holding the results of the Java tests.
+
+  Raises:
+    Exception: when there are no attached devices.
+  """
+  test_pkg = test_package.TestPackage(
+      options.uiautomator_jar, options.uiautomator_info_jar)
+  tests = test_pkg._GetAllMatchingTests(
+      options.annotations, options.test_filter)
+  if not tests:
+    logging.warning('No uiautomator tests to run with current args.')
+    return base_test_result.TestRunResults()
+
+  attached_devices = android_commands.GetAttachedDevices()
+  if not attached_devices:
+    raise Exception('There are no devices online.')
+
+  if options.device:
+    assert options.device in attached_devices
+    attached_devices = [options.device]
+
+  def TestRunnerFactory(device, shard_index):
+    return test_runner.TestRunner(
+        options, device, shard_index, test_pkg, [])
+
+  return shard.ShardAndRunTests(TestRunnerFactory, attached_devices, tests,
+                                options.build_type)
diff --git a/build/android/pylib/uiautomator/test_runner.py b/build/android/pylib/uiautomator/test_runner.py
new file mode 100644
index 0000000..c35ac97
--- /dev/null
+++ b/build/android/pylib/uiautomator/test_runner.py
@@ -0,0 +1,40 @@
+# 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.
+
+"""Class for running uiautomator tests on a single device."""
+
+from pylib.instrumentation import test_runner as instr_test_runner
+
+
+class TestRunner(instr_test_runner.TestRunner):
+  """Responsible for running a series of tests connected to a single device."""
+
+  def __init__(self, options, device, shard_index, test_pkg, ports_to_forward):
+    """Create a new TestRunner.
+
+    Args:
+      options: An options object similar to the one in parent class plus:
+        - package_name: Application package name under test.
+    """
+    options.ensure_value('install_apk', True)
+    options.ensure_value('wait_for_debugger', False)
+    super(TestRunner, self).__init__(
+        options, device, shard_index, test_pkg, ports_to_forward)
+
+    self.package_name = options.package_name
+
+  #override
+  def PushDependencies(self):
+      self.test_pkg.Install(self.adb)
+
+  #override
+  def _RunTest(self, test, timeout):
+    self.adb.ClearApplicationState(self.package_name)
+    if 'Feature:FirstRunExperience' in self.test_pkg.GetTestAnnotations(test):
+      self.flags.RemoveFlags(['--disable-fre'])
+    else:
+      self.flags.AddFlags(['--disable-fre'])
+    return self.adb.RunUIAutomatorTest(
+        test, self.test_pkg.GetPackageName(), timeout)
+
diff --git a/build/android/pylib/utils/test_options_parser.py b/build/android/pylib/utils/test_options_parser.py
index 98e5c15..4ddc32f 100644
--- a/build/android/pylib/utils/test_options_parser.py
+++ b/build/android/pylib/utils/test_options_parser.py
@@ -134,26 +134,19 @@ def AddCommonInstrumentationOptions(option_parser):
   """Decorates OptionParser with base instrumentation tests options."""
 
   AddTestRunnerOptions(option_parser)
-  option_parser.add_option('-w', '--wait_debugger', dest='wait_for_debugger',
-                           action='store_true', help='Wait for debugger.')
   option_parser.add_option('-f', '--test_filter',
                            help='Test filter (if not fully qualified, '
                            'will run all matches).')
-  option_parser.add_option('-A', '--annotation', dest='annotation_str',
-                           help=('Run only tests with any of the given '
-                                 'annotations. '
-                                 'An annotation can be either a key or a '
-                                 'key-values pair. '
-                                 'A test that has no annotation is '
-                                 'considered "SmallTest".'))
+  option_parser.add_option(
+      '-A', '--annotation', dest='annotation_str',
+      help=('Comma-separated list of annotations. Run only tests with any of '
+            'the given annotations. An annotation can be either a key or a '
+            'key-values pair. A test that has no annotation is considered '
+            '"SmallTest".'))
   option_parser.add_option('-j', '--java_only', action='store_true',
                            help='Run only the Java tests.')
   option_parser.add_option('-p', '--python_only', action='store_true',
                            help='Run only the Python tests.')
-  option_parser.add_option('-n', '--run_count', type='int',
-                           dest='number_of_runs', default=1,
-                           help=('How many times to run each test, regardless '
-                                 'of the result. (Default is 1)'))
   option_parser.add_option('--screenshot', dest='screenshot_failures',
                            action='store_true',
                            help='Capture screenshots of test failures')
@@ -194,6 +187,8 @@ def AddInstrumentationOptions(option_parser):
   """Decorates OptionParser with instrumentation tests options."""
 
   AddCommonInstrumentationOptions(option_parser)
+  option_parser.add_option('-w', '--wait_debugger', dest='wait_for_debugger',
+                           action='store_true', help='Wait for debugger.')
   option_parser.add_option('-I', dest='install_apk',
                            help='Install APK.', action='store_true')
   option_parser.add_option(
@@ -234,11 +229,11 @@ def ValidateCommonInstrumentationOptions(option_parser, options, args):
     options.run_java_tests = False
 
   if options.annotation_str:
-    options.annotation = options.annotation_str.split()
+    options.annotations = options.annotation_str.split(',')
   elif options.test_filter:
-    options.annotation = []
+    options.annotations = []
   else:
-    options.annotation = ['Smoke', 'SmallTest', 'MediumTest', 'LargeTest']
+    options.annotations = ['Smoke', 'SmallTest', 'MediumTest', 'LargeTest']
 
 
 def ValidateInstrumentationOptions(option_parser, options, args):
diff --git a/build/android/run_instrumentation_tests.py b/build/android/run_instrumentation_tests.py
index 089c04c..598331b 100755
--- a/build/android/run_instrumentation_tests.py
+++ b/build/android/run_instrumentation_tests.py
@@ -51,7 +51,7 @@ def DispatchInstrumentationTests(options):
       results=all_results,
       test_type='Instrumentation',
       test_package=os.path.basename(options.test_apk),
-      annotation=options.annotation,
+      annotation=options.annotations,
       build_type=options.build_type,
       flakiness_server=options.flakiness_dashboard_server)
 
diff --git a/build/android/run_uiautomator_tests.py b/build/android/run_uiautomator_tests.py
index 63480f4..ad6035b 100755
--- a/build/android/run_uiautomator_tests.py
+++ b/build/android/run_uiautomator_tests.py
@@ -16,7 +16,7 @@ from pylib import constants
 from pylib import ports
 from pylib.base import base_test_result
 from pylib.host_driven import run_python_tests
-from pylib.instrumentation import dispatch
+from pylib.uiautomator import dispatch
 from pylib.utils import report_results
 from pylib.utils import run_tests_helper
 from pylib.utils import test_options_parser
@@ -53,7 +53,7 @@ def DispatchUIAutomatorTests(options):
       results=all_results,
       test_type='UIAutomator',
       test_package=os.path.basename(options.test_jar),
-      annotation=options.annotation,
+      annotation=options.annotations,
       build_type=options.build_type,
       flakiness_server=options.flakiness_dashboard_server)