Android: Cleaned up python setup steps for gtests.

build/android/pylib/gtest/setup.py

 # Copyright 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.
 
 """Generates test runner factory and tests for GTests."""
 # pylint: disable=W0212
 
 import fnmatch
 import glob
 import logging
@@ skipping 200 matching lines @@
     return '*'
 
   filters = [x for x in [x.strip() for x in file(filter_file_path).readlines()]
              if x and x[0] != '#']
   disabled_filter = '*-%s' % ':'.join(filters)
   logging.info('Applying filter "%s" obtained from %s',
                disabled_filter, filter_file_path)
   return disabled_filter
 
 
+# A helper class for scheduling setup-related tasks on devices.
+class _TestSetupRunner(test_runner.TestRunner):
+  def SetUp(self):
+    pass
+
+  def TearDown(self):
+    pass
+
+  def DeviceSteps(self):
+    """ Steps to be run for device. Should be implemented in subclasses.
+
+    Returns:
+      A value that will be put into 'setup_result' field of corresponding
+      test result.
+    """

[bulach, 2014/06/11 16:59:11]

nit: raise NotImplementedError()

[Victor Starodub, 2014/07/02 14:48:17]

Done.

+
+  def RunTest(self, _test):
+    result = base_test_result.BaseTestResult(
+        "dummy", base_test_result.ResultType.PASS)

[bulach, 2014/06/11 16:59:11]

nit: s/"/'/

[Victor Starodub, 2014/07/02 14:48:17]

Done.

+
+    result.setup_result = self.DeviceSteps()
+    results = base_test_result.TestRunResults()
+    results.AddResult(result)
+    return results, None
+
+
+def _InstallPackage(test_options, test_package, devices):
+  def TestInstallerRunnerFactory(device, _shard_index):
+    class TestInstallerRunner(_TestSetupRunner):
+      def DeviceSteps(self):
+        self.test_package.Install(self.device)
+    return TestInstallerRunner(test_options, device, test_package)
+
+  test_dispatcher.RunTests(['setup'], TestInstallerRunnerFactory, devices,
+      shard=False)
+
+
 def _GetTests(test_options, test_package, devices):
   """Get a list of tests.
 
   Args:
     test_options: A GTestOptions object.
     test_package: A TestPackageApk object.
     devices: A list of attached devices.
 
   Returns:
     A list of all the tests in the test suite.
   """
   def TestListerRunnerFactory(device, _shard_index):
-    class TestListerRunner(test_runner.TestRunner):
-      def RunTest(self, _test):
-        result = base_test_result.BaseTestResult(
-            'gtest_list_tests', base_test_result.ResultType.PASS)
-        self.test_package.Install(self.device)
-        result.test_list = self.test_package.GetAllTests(self.device)
-        results = base_test_result.TestRunResults()
-        results.AddResult(result)
-        return results, None
+    class TestListerRunner(_TestSetupRunner):

[jbudorick, 2014/06/16 18:16:51]

One other thought, since we were talking about this: what's would be the
difference in execution time between this change and just adding noop overrides
of SetUp and TearDown to the existing TestListerRunner (i.e., what you did in
_TestSetupRunner above)?

[Victor Starodub, 2014/07/02 14:48:17]

Second install attempt would be ~2.5s if the package is already installed (1
device), at least ~15s for a 30Mb package if the package is not installed (2nd
device).

+      def DeviceSteps(self):
+        return self.test_package.GetAllTests(self.device)
     return TestListerRunner(test_options, device, test_package)
 
   results, _no_retry = test_dispatcher.RunTests(
-      ['gtest_list_tests'], TestListerRunnerFactory, devices)
+      ['setup'], TestListerRunnerFactory, devices)
   tests = []
   for r in results.GetAll():
-    tests.extend(r.test_list)
+    tests.extend(r.setup_result)
   return tests
 
 
 def _FilterTestsUsingPrefixes(all_tests, pre=False, manual=False):
   """Removes tests with disabled prefixes.
 
   Args:
     all_tests: List of tests to filter.
     pre: If True, include tests with PRE_ prefix.
     manual: If True, include tests with MANUAL_ prefix.
@@ skipping 57 matching lines @@
         test_options.suite_name)
     if not os.path.exists(test_package.suite_path):
       raise Exception(
           'Did not find %s target. Ensure it has been built.'
           % test_options.suite_name)
   logging.warning('Found target %s', test_package.suite_path)
 
   _GenerateDepsDirUsingIsolate(test_options.suite_name,
                                test_options.isolate_file_path)
 
+  _InstallPackage(test_options, test_package, devices)

[jbudorick, 2014/06/11 18:22:58]

I believe this is the "complex setup" you're referring to. We're still pushing
the same data (and we have to). The only difference is that it's no longer in
_GetTests. Since _GetTests _depends_ on the apk or executable being on the
device, I'm not sure that's a great idea.

[Victor Starodub, 2014/06/11 19:13:29]

We're just installing the package, we don't try push data here or bring up http
server as we did, so that's the "complex setup" we don't do.

Since it's the code of the 'Setup' method, I think relying on the fact that the
package will be installed inside it is ok. Also it looks fine to me that we do
depend on the order of setup steps being correct.

A lot of things during test run can depend on the package being installed, but I
don't think it necessary means that we need to put self.test_package.Install()
calls everywhere.

   tests = _GetTests(test_options, test_package, devices)
 
   # Constructs a new TestRunner with the current options.
   def TestRunnerFactory(device, _shard_index):
     return test_runner.TestRunner(
         test_options,
         device,
         test_package)
 
   if test_options.run_disabled:
     test_options = test_options._replace(
         test_arguments=('%s --gtest_also_run_disabled_tests' %
                         test_options.test_arguments))
   else:
     tests = _FilterDisabledTests(tests, test_options.suite_name,
                                  bool(test_options.gtest_filter))
   if test_options.gtest_filter:
     tests = unittest_util.FilterTestNames(tests, test_options.gtest_filter)
 
   # Coalesce unit tests into a single test per device
   if test_options.suite_name != 'content_browsertests':
     num_devices = len(devices)
     tests = [':'.join(tests[i::num_devices]) for i in xrange(num_devices)]
     tests = [t for t in tests if t]
 
   return (TestRunnerFactory, tests)