Add step to process Render Test results.

scripts/slave/recipe_modules/chromium_tests/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 datetime
 import json
 import re
 import string
 
 
@@ skipping 1466 matching lines @@
   def run_tests(self, api, suffix, json_results_file):
     """Runs the Android test suite and outputs the json results to a file.
 
     Args:
       api: Caller's API.
       suffix: Suffix added to the test name.
       json_results_file: File to output the test results.
     """
     raise NotImplementedError()  # pragma: no cover
 
   def run(self, api, suffix, test_filter=None):

[jbudorick, 2016/11/03 18:34:36]

Implementing this in here will not work on the bots that use swarming -- i.e.,
the trybots or the main waterfall.

[mikecase (-- gone --), 2016/11/03 18:41:37]

I don't think that is a problem. The logic to run the render test stuff is in
chromium_android/api.py. These changes are just to adapt to me removing the
copy_gtest_results function.

[jbudorick, 2016/11/07 21:58:56]

I mean, it's ok for now, but if you want to use this on the try bots, it'll need
to be usable on swarming.

(The same applies to the results processing logic, though.)

     assert api.chromium.c.TARGET_PLATFORM == 'android'
-    json_results_file = api.test_utils.gtest_results(add_json_log=False)
-    try:
-      self.run_tests(api, suffix, json_results_file)
-    finally:
-      step_result = api.step.active_result
-      self._test_runs[suffix] = {'valid': False}
-      if (hasattr(step_result, 'test_utils') and
-          hasattr(step_result.test_utils, 'gtest_results')):
-        gtest_results = step_result.test_utils.gtest_results
 
-        failures = gtest_results.failures
-        self._test_runs[suffix] = {'valid': True, 'failures': failures}
-        step_result.presentation.step_text += (
-            api.test_utils.format_step_text([['failures:', failures]]))
+    nested_step_name = '%s%s' % (self._name, ' (%s)' % suffix if suffix else '')
+    with api.step.nest(nested_step_name) as nested_step:
+      json_results_file = api.test_utils.gtest_results(add_json_log=False)
+      try:
+        step_result = self.run_tests(api, suffix, json_results_file)
+      except api.step.StepFailure as f:
+        step_result = f.result
+        raise
+      finally:
+        nested_step.presentation.status = step_result.presentation.status
+        self._test_runs[suffix] = {'valid': False}
+        if (hasattr(step_result, 'test_utils') and
+            hasattr(step_result.test_utils, 'gtest_results')):
+          gtest_results = step_result.test_utils.gtest_results
 
-        api.test_results.upload(
-            api.json.input(gtest_results.raw),
-            test_type=self.name,
-            chrome_revision=api.bot_update.last_returned_properties.get(
-                'got_revision_cp', 'x@{#0}'),
-            test_results_server='test-results.appspot.com')
+          failures = gtest_results.failures
+          self._test_runs[suffix] = {'valid': True, 'failures': failures}
+          nested_step.presentation.step_text += (
+              api.test_utils.format_step_text([['failures:', failures]]))

[mikecase (-- gone --), 2016/11/03 18:56:33]

This is basically the only important line I added to this file.

Before

- run_instrumentation_test() function copies test results from the test_runner
step to the last step that was run. This might not be the test_runner step and
could be anything which is kinda awkward
- This function grabs test results from the active_result (last step that was
run). It then prints out the results to the active_result's step text (again,
this step might not be the test_runner step so the results could get printed
somewhere awkward and inconsistent)

Now

- run_instrumentation_test() function DOES NOT copy test results to last step.
Instead, it explicitly returns the test runner step.
- This function grabs the test results from the test_runner step. However, it
might not be able to modify that step's step_text because it is not guaranteed
to be the last step to have run. Instead, it print the results to the new outer
nested_step I added.

Alternatively, we could have run_instrumentation_tests() function print the
results to the step_text which might work

[jbudorick, 2016/11/07 21:58:56]

Ah, yeah, I see what you mean now.

+
+          api.test_results.upload(
+              api.json.input(gtest_results.raw),
+              test_type=self.name,
+              chrome_revision=api.bot_update.last_returned_properties.get(
+                  'got_revision_cp', 'x@{#0}'),
+              test_results_server='test-results.appspot.com')
 
   def compile_targets(self, _):
     return self._compile_targets
 
   def has_valid_results(self, api, suffix):
     if suffix not in self._test_runs:
       return False  # pragma: no cover
     return self._test_runs[suffix]['valid']
 
   def failures(self, api, suffix):
     assert self.has_valid_results(api, suffix)
     return self._test_runs[suffix]['failures']
 
 
 class AndroidJunitTest(AndroidTest):
   def __init__(self, name):
     super(AndroidJunitTest, self).__init__(name, compile_targets=[name],
         isolate_file_path=None)
 
   @property
   def uses_local_devices(self):
     return False
 
   #override
   def run_tests(self, api, suffix, json_results_file):
-    api.chromium_android.run_java_unit_test_suite(
+    return api.chromium_android.run_java_unit_test_suite(
         self.name, verbose=True, suffix=suffix,
         json_results_file=json_results_file,
         step_test_data=lambda: api.test_utils.test_api.canned_gtest_output(False))
 
 
 class AndroidInstrumentationTest(AndroidTest):
   _DEFAULT_SUITES = {
     'AndroidWebViewTest': {
       'compile_target': 'android_webview_test_apk',
     },
@@ skipping 36 matching lines @@
     'mojo_test_apk': _DEFAULT_SUITES['MojoTest'],
     'remoting_test_apk': _DEFAULT_SUITES['ChromotingTest'],
     'system_webview_shell_layout_test_apk':
         _DEFAULT_SUITES['SystemWebViewShellLayoutTest'],
   }
 
   def __init__(self, name, compile_targets=None, apk_under_test=None,
                test_apk=None, isolate_file_path=None, timeout_scale=None,
                annotation=None, except_annotation=None, screenshot=False,
                verbose=True, tool=None, additional_apks=None,
-               store_tombstones=False, result_details=False):
+               store_tombstones=False, result_details=False,
+               render_results_dir=None):
     suite_defaults = (
         AndroidInstrumentationTest._DEFAULT_SUITES.get(name)
         or AndroidInstrumentationTest._DEFAULT_SUITES_BY_TARGET.get(name)
         or {})
     if not compile_targets:
       compile_targets = [suite_defaults.get('compile_target', name)]
       compile_targets.extend(
           suite_defaults.get('additional_compile_targets', []))
 
     super(AndroidInstrumentationTest, self).__init__(
         name,
         compile_targets,
         isolate_file_path or suite_defaults.get('isolate_file_path'))
     self._additional_apks = (
         additional_apks or suite_defaults.get('additional_apks'))
     self._annotation = annotation
     self._apk_under_test = (
         apk_under_test or suite_defaults.get('apk_under_test'))
     self._except_annotation = except_annotation
     self._screenshot = screenshot
     self._test_apk = test_apk or suite_defaults.get('test_apk')
     self._timeout_scale = timeout_scale
     self._tool = tool
     self._verbose = verbose
     self._wrapper_script_suite_name = compile_targets[0]
     self._store_tombstones = store_tombstones
     self._result_details = result_details
+    self._render_results_dir = render_results_dir
 
   @property
   def uses_local_devices(self):
     return True
 
   #override
   def run_tests(self, api, suffix, json_results_file):
-    api.chromium_android.run_instrumentation_suite(
+    return api.chromium_android.run_instrumentation_suite(
         self.name,
         test_apk=api.chromium_android.apk_path(self._test_apk),
         apk_under_test=api.chromium_android.apk_path(self._apk_under_test),
         additional_apks=[
             api.chromium_android.apk_path(a)
             for a in self._additional_apks or []],
         suffix=suffix,
         isolate_file_path=self.isolate_file_path,
         annotation=self._annotation, except_annotation=self._except_annotation,
         screenshot=self._screenshot, verbose=self._verbose, tool=self._tool,
         json_results_file=json_results_file,
         timeout_scale=self._timeout_scale,
         result_details=self._result_details,
         store_tombstones=self._store_tombstones,
         wrapper_script_suite_name=self._wrapper_script_suite_name,
+        render_results_dir=self._render_results_dir,
         step_test_data=lambda: api.test_utils.test_api.canned_gtest_output(False))
 
 
 class BlinkTest(Test):
   # TODO(dpranke): This should be converted to a PythonBasedTest, although it
   # will need custom behavior because we archive the results as well.
   def __init__(self, extra_args=None):
     super(BlinkTest, self).__init__()
     self._extra_args = extra_args
 
@@ skipping 232 matching lines @@
           args=args)
       api.gsutil.upload(
           temp_output_dir.join(
               '%s-android-chrome.json' % timestamp_string),
           'chromium-annotated-tests', 'android')
 
 GOMA_TESTS = [
   GTestTest('base_unittests'),
   GTestTest('content_unittests'),
 ]