Merged Android and non-Android code paths to reuse code in GTestTest

scripts/slave/recipe_modules/chromium/steps.py

 # Copyright 2014 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.
 
 import re
 
 
 class Test(object):
   """
   Base class for tests that can be retried after deapplying a previously
@@ skipping 206 matching lines @@
       return [self.target_name + '_apk']
 
     # On iOS we rely on 'All' target being compiled instead of using
     # individual targets.
     if api.chromium.c.TARGET_PLATFORM == 'ios':
       return []
 
     return [self.target_name]
 
   def run(self, api, suffix):
-    if api.chromium.c.TARGET_PLATFORM == 'android':
-      isolate_file_path = None
-      if self._android_isolate_path:
-        isolate_file_path = api.path['checkout'].join(
-            self._android_isolate_path)
-      return api.chromium_android.run_test_suite(
-          self.target_name,
-          flakiness_dashboard='http://test-results.appspot.com',
-          isolate_file_path=isolate_file_path)
-
     # Copy the list because run can be invoked multiple times and we modify
     # the local copy.
     args = self._args[:]
 
     if suffix == 'without patch':
       args.append(api.chromium.test_launcher_filter(
           self.failures(api, 'with patch')))
 
+    gtest_results_file = api.json.gtest_results(add_json_log=False)
+    step_test_data = lambda: api.json.test_api.canned_gtest_output(True)
+
     kwargs = {}
     kwargs['name'] = self._step_name(suffix)
-    kwargs['xvfb'] = True
-    kwargs['test_type'] = self.name
-    kwargs['annotate'] = 'gtest'
     kwargs['args'] = args
-    kwargs['step_test_data'] = lambda: api.json.test_api.canned_gtest_output(
-        True)
-    kwargs['test_launcher_summary_output'] = api.json.gtest_results(
-        add_json_log=False)
-    kwargs.update(self._runtest_kwargs)
+    kwargs['step_test_data'] = step_test_data
+
+    if api.chromium.c.TARGET_PLATFORM == 'android':
+      # TODO(sergiyb): Figure out if we can reuse isolate module for running
+      # isolated Android tests, rather than using custom solution in Android
+      # test launcher.
+      if self._android_isolate_path:
+        isolate_fpath = api.path['checkout'].join(self._android_isolate_path)

[Paweł Hajdan Jr., 2015/01/29 20:19:12]

nit: Instead of somewhat weird isolate_fpath why not just isolate_path (without
the "f")?

[Sergiy Byelozyorov, 2015/01/29 20:24:22]

Done.

+        kwargs['isolate_file_path'] = isolate_fpath
+      kwargs['json_results_file'] = gtest_results_file

[Sergiy Byelozyorov, 2015/01/29 17:09:58]

Since Android test launcher tries to return test results in the same format as
gTest tests, we should be able to parse these results with the same placeholder
type. Since Android instrumentation tests are using the same launcher, we can
probably also replace custom parsing code there with api.json.gtest_results too,
but I'll do this in a separate CL.

[Paweł Hajdan Jr., 2015/01/29 20:19:12]

Have you verified this?

Both are JSON, but Android might be emulating the LayoutTests format instead of
our C++ gtest launcher. Just making sure it's been empirically checked rather
just being an assumption that since both are JSON they must be compatible.

[Sergiy Byelozyorov, 2015/01/29 20:24:22]

(sorry for not replying directly to the comment first time)

I based my assumption on the fact that keys have the same names and this
comment:
https://code.google.com/p/chromium/codesearch#chromium/src/build/android/pyli....
AFAIK, GTest uses this method too.

+      kwargs['flakiness_dashboard'] = 'http://test-results.appspot.com'
+    else:
+      kwargs['xvfb'] = True
+      kwargs['test_type'] = self.name
+      kwargs['annotate'] = 'gtest'
+      kwargs['test_launcher_summary_output'] = gtest_results_file
+      kwargs.update(self._runtest_kwargs)
 
     try:
-      if self._use_isolate:
+      if api.chromium.c.TARGET_PLATFORM == 'android':
+        api.chromium_android.run_test_suite(self.target_name, **kwargs)
+      elif self._use_isolate:
         api.isolate.runtest(self.target_name, self._revision,
                             self._webkit_revision, **kwargs)
       else:
         api.chromium.runtest(self.target_name, revision=self._revision,
                              webkit_revision=self._webkit_revision, **kwargs)
     finally:
       step_result = api.step.active_result
       self._test_runs[suffix] = step_result
 
       r = step_result.json.gtest_results
       p = step_result.presentation
 
       if r.valid:
         p.step_text += api.test_utils.format_step_text([
             ['failures:', r.failures]
         ])
+
     return step_result
 
   def has_valid_results(self, api, suffix):
     gtest_results = self._test_runs[suffix].json.gtest_results
     if not gtest_results.valid:  # pragma: no cover
       return False
     global_tags = gtest_results.raw.get('global_tags', [])
     return 'UNRELIABLE_RESULTS' not in global_tags
 
   def failures(self, api, suffix):
@@ skipping 816 matching lines @@
   GTestTest('crypto_unittests'),
   GTestTest('gfx_unittests'),
   GTestTest('url_unittests'),
   GTestTest('content_unittests'),
   GTestTest('net_unittests'),
   GTestTest('ui_base_unittests'),
   GTestTest('ui_ios_unittests'),
   GTestTest('sync_unit_tests'),
   GTestTest('sql_unittests'),
 ]