Updating swarming api to check for chartjson results from a finished

scripts/slave/recipe_modules/swarming/api.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 logging
 import datetime
 import functools
 
 from recipe_engine import recipe_api
 
 
 # Minimally supported version of swarming.py script (reported by --version).
 MINIMAL_SWARMING_VERSION = (0, 8, 6)
 
 
@@ skipping 385 matching lines @@
     extra_args.append(
         '--test-launcher-summary-output=${ISOLATED_OUTDIR}/output.json')
 
     # Make a task, configure it to be collected through shim script.
     task = self.task(title, isolated_hash, extra_args=extra_args,
                      cipd_packages=cipd_packages, **kwargs)
     task.collect_step = lambda *args, **kw: (
         self._gtest_collect_step(test_launcher_summary_output, *args, **kw))
     return task
 
+  def _check_and_set_output_flag(self, extra_args, flag, output_file_name):
+    extra_args = list(extra_args or [])
+    # Ensure flag is not already passed. We are going to overwrite it.
+    flag_value = '--%s=' % flag
+    bad_args = any(x.startswith(flag_value) for x in extra_args)
+    if bad_args:  # pragma: no cover

[Ken Russell (switch to Gerrit), 2016/09/19 23:41:19]

It would be better to write a test for this code path -- is that not possible?

[eyaich1, 2016/09/20 13:56:59]

So this was copy/pasted code so I didn't notice it before.  You can easily add a
test to simulate a bad flag, but what do you do with the raised error in the
test suite?  If you notice in this file there is other untested code for
exceptions being thrown and not caught, can the expectations suite handle
uncaught exceptions?  When I run the test suite it fails on the thrown
exception.

+      error = '--%s should not be used' % flag
+      raise ValueError(error)
+
+    # Append it.
+    output_arg = '--%s=${ISOLATED_OUTDIR}/%s' % (flag, output_file_name)
+    extra_args.append(output_arg)
+    return extra_args
+
   def isolated_script_task(self, title, isolated_hash, extra_args=None,
                            idempotent=False, **kwargs):
     """Returns a new SwarmingTask to run an isolated script test on Swarming.
 
     Swarming recipe module knows how collect JSON file with test execution
     summary produced by isolated script tests launcher. Since isolated script
     tests do not support sharding, no merging of the results is performed.
     Parsed JSON summary is returned from the collect step.
 
     For meaning of the rest of the arguments see 'task' method.
     """
-    extra_args = list(extra_args or [])
 
-    # Ensure --isolated-script-test-output is not already passed. We are going
-    # to overwrite it.
-    bad_args = any(
-        x.startswith('--isolated-script-test-output=') for x in extra_args)
-    if bad_args:  # pragma: no cover
-      raise ValueError('--isolated-script-test-output should not be used.')
-
-    # Append it. output.json name is expected by collect_gtest_task.py.
-    extra_args.append(
-        '--isolated-script-test-output=${ISOLATED_OUTDIR}/output.json')
+    # Ensure output flags are not already passed. We are going
+    # to overwrite them.
+    # output.json name is expected by collect_gtest_task.py.
+    extra_args = self._check_and_set_output_flag( \
+      extra_args, 'isolated-script-test-output', 'output.json')
+    # chartjson-output.json name is expected by benchmarks generating chartjson
+    # output
+    extra_args = self._check_and_set_output_flag(
+      extra_args,
+      'isolated-script-test-chartjson-output',
+      'chartjson-output.json')
 
     task = self.task(title, isolated_hash, extra_args=extra_args,
                      idempotent=idempotent, **kwargs)
     task.collect_step = self._isolated_script_collect_step
     return task
 
   def check_client_version(self, step_test_data=None):
     """Yields steps to verify compatibility with swarming_client version."""
     return self.m.swarming_client.ensure_script_version(
         'swarming.py', MINIMAL_SWARMING_VERSION, step_test_data)
@@ skipping 267 matching lines @@
         # Show any remaining isolated outputs (such as logcats).
         # Note that collect_gtest_task.py uses the default summary.json, which
         # only has 'outputs_ref' instead of the deprecated 'isolated_out'.
         for index, shard in enumerate(swarming_summary.get('shards', [])):
           outputs_ref = shard.get('outputs_ref')
           if outputs_ref:
             link_name = 'shard #%d isolated out' % index
             p.links[link_name] = outputs_ref['view_url']
 
 
+  def _merge_isolated_script_chartjson_ouput_shards(self, task, step_result):
+    # Taken from third_party/catapult/telemetry/telemetry/internal/results/
+    # chart_json_output_formatter.py, the json entries are as follows:
+    # result_dict = {
+    #   'format_version': '0.1',
+    #   'next_version': '0.2',
+    #   'benchmark_name': benchmark_metadata.name,
+    #   'benchmark_description': benchmark_metadata.description,
+    #   'trace_rerun_options': benchmark_metadata.rerun_options,
+    #   'benchmark_metadata': benchmark_metadata.AsDict(),
+    #   'charts': charts,
+    # }
+    #
+    # Therefore, all entries should be the same and we should only need to merge
+    # the chart from each shard.
+    merged_results = {}
+    seen_first_shard = False
+    for i in xrange(task.shards):
+      path = self.m.path.join(str(i), 'chartjson-output.json')
+      if path not in step_result.raw_io.output_dir:
+        # chartjson results were not written for this shard, not an error,
+        # just return
+        continue
+      results_raw = step_result.raw_io.output_dir[path]
+      try:
+        chartjson_results_json = self.m.json.loads(results_raw)
+      except Exception as e:
+        raise Exception('error decoding JSON results from shard #%d' % i)
+      if not seen_first_shard:
+        merged_results = chartjson_results_json
+        seen_first_shard = True
+        continue
+      for key in chartjson_results_json:
+        if key == 'charts':
+          merged_results[key].extend(chartjson_results_json[key])
+    return merged_results
+
+
   def _merge_isolated_script_shards(self, task, step_result):
     # This code is unfortunately specialized to the "simplified"
     # JSON format that used to be the standard for recipes. The
     # isolated scripts should be changed to use the now-standard
     # Chromium JSON test results format:
     # https://www.chromium.org/developers/the-json-test-results-format
     # . Note that gtests, above, don't seem to conform to this
     # format yet, so it didn't seem like a good prerequisite to
     # switch the isolated tests over when adding sharding support.
     #
@@ skipping 62 matching lines @@
         # in recipe runs.)
         links = step_result.presentation.links
         for index in xrange(task.shards):
           url = task.get_shard_view_url(index)
           if url:
             links['shard #%d' % index] = url
 
         step_result.isolated_script_results = \
           self._merge_isolated_script_shards(task, step_result)
 
+        # Obtain chartjson results if present
+        step_result.isolated_script_chartjson_results = \
+          self._merge_isolated_script_chartjson_ouput_shards(task, step_result)
+
         self._display_pending(summary, step_result.presentation)
       except Exception as e:
         self.m.step.active_result.presentation.logs['no_results_exc'] = [str(e)]
         self.m.step.active_result.isolated_script_results = None
 
   def _get_step_name(self, prefix, task):
     """SwarmingTask -> name of a step of a waterfall.
 
     Will take a task title (+ step name prefix) and append OS dimension to it.
 
@@ skipping 177 matching lines @@
 
   def get_shard_view_url(self, index):
     """Returns URL of HTML page with shard details or None if not available.
 
     Works only after the task has been successfully triggered.
     """
     if self._trigger_output and self._trigger_output.get('tasks'):
       for shard_dict in self._trigger_output['tasks'].itervalues():
         if shard_dict['shard_index'] == index:
           return shard_dict['view_url']