Android: Cleaned up python setup steps for gtests.

build/android/pylib/gtest/setup.py

Index: build/android/pylib/gtest/setup.py
diff --git a/build/android/pylib/gtest/setup.py b/build/android/pylib/gtest/setup.py
index c691e412cbdb2e6e68d8e273a736fb799f2ef238..e88d535978aa5f5c3bd30ab944f33fe210dcb688 100644
--- a/build/android/pylib/gtest/setup.py
+++ b/build/android/pylib/gtest/setup.py
@@ -218,6 +218,43 @@ def _GetDisabledTestsFilterFromFile(suite_name):
   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.
 
@@ -230,22 +267,16 @@ def _GetTests(test_options, test_package, devices):
     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
 
 
@@ -323,6 +354,7 @@ def Setup(test_options, devices):
   _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.