Add step to process Render Test results.

scripts/slave/recipe_modules/chromium_tests/steps.py

Index: scripts/slave/recipe_modules/chromium_tests/steps.py
diff --git a/scripts/slave/recipe_modules/chromium_tests/steps.py b/scripts/slave/recipe_modules/chromium_tests/steps.py
index d0697c0fc2d9f3f4ee81580ebc25c0e61ff3148e..1569d6be1458ef9625cd7fb990a82f74cd074437 100644
--- a/scripts/slave/recipe_modules/chromium_tests/steps.py
+++ b/scripts/slave/recipe_modules/chromium_tests/steps.py
@@ -1486,27 +1486,33 @@ class AndroidTest(Test):
 
   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
+
+          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')
+          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
@@ -1532,7 +1538,7 @@ class AndroidJunitTest(AndroidTest):
 
   #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))
@@ -1589,7 +1595,8 @@ class AndroidInstrumentationTest(AndroidTest):
                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)
@@ -1617,6 +1624,7 @@ class AndroidInstrumentationTest(AndroidTest):
     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):
@@ -1624,7 +1632,7 @@ class AndroidInstrumentationTest(AndroidTest):
 
   #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),
@@ -1640,6 +1648,7 @@ class AndroidInstrumentationTest(AndroidTest):
         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))