Refactor stages 1, 2 and test_api overhaul.

scripts/slave/recipe_modules/bisect_tester/perf_test.py

 # Copyright 2015 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 os
 import re
 import time
 
 from . import parse_metric
 
@@ skipping 61 matching lines @@
   """Attempts to discern whether or not a given command is running telemetry."""
   return 'run_benchmark' in command
 
 
 def run_perf_test(api, test_config, **kwargs):
   """Runs the command N times and parses a metric from the output."""
   # TODO(prasadv):  Consider extracting out the body of the for loop into
   # a helper method, or extract the metric-extraction to make this more
   # cleaner.
   limit = test_config['max_time_minutes'] * kwargs.get('time_multiplier', 1)
-  run_results = {'measured_values': [], 'errors': set()}
-  values = run_results['measured_values']
+  results = {'valueset_paths': [], 'chartjson_paths': [], 'errors': set(),
+             'retcodes': [], 'values': [], 'output': []}
   metric = test_config.get('metric')
-  retcodes = []
-  output_for_all_runs = []
   temp_dir = None
   repeat_cnt = test_config['repeat_count']
 
   command = test_config['command']
   use_chartjson = bool('chartjson' in command)
+  use_valueset = bool('valueset' in command)
   is_telemetry = _is_telemetry_command(command)
   start_time = time.time()
 
   if api.m.chromium.c.TARGET_PLATFORM == 'android' and is_telemetry:
     device_serial_number = api.device_to_test;
     if device_serial_number:
       command += ' --device ' + device_serial_number # pragma: no cover
 
   for i in range(repeat_cnt):
     elapsed_minutes = (time.time() - start_time) / 60.0
     # A limit of 0 means 'no timeout set'.
     if limit and elapsed_minutes >= limit:  # pragma: no cover
       break
     if is_telemetry:
       if i == 0 and kwargs.get('reset_on_first_run'):
         command += ' --reset-results'
       if i == repeat_cnt - 1 and kwargs.get('upload_on_last_run'):
         command += ' --upload-results'
       if kwargs.get('results_label'):
         command += ' --results-label=%s' % kwargs.get('results_label')
-    if use_chartjson:  # pragma: no cover
+    if use_chartjson or use_valueset:  # pragma: no cover
       temp_dir = api.m.path.mkdtemp('perf-test-output')
       command = _set_output_dir(command, str(temp_dir))
-      results_path = temp_dir.join('results-chart.json')
+      chartjson_path = temp_dir.join('results-chart.json')
+      valueset_path = temp_dir.join('results-valueset.json')
 
     step_name = "Performance Test%s %d of %d" % (
         ' (%s)' % kwargs['name'] if 'name' in kwargs else '', i + 1, repeat_cnt)
     if api.m.platform.is_linux:
       os.environ['CHROME_DEVEL_SANDBOX'] = api.m.path.join(
           '/opt', 'chromium', 'chrome_sandbox')
-    out, err, retcode = _run_command(api, command, step_name)
+    out, err, retcode = _run_command(api, command, step_name, **kwargs)
+    results['output'].append(out or '')
 
     if out is None and err is None:
       # dummy value when running test TODO: replace with a mock
-      values.append(0)
+      results['values'].append(0)
     elif metric:  # pragma: no cover
       if use_chartjson:
-        step_result = api.m.json.read(
-            'Reading chartjson results', results_path)
-        has_valid_value, value = find_values(
-            step_result.json.output, Metric(metric))
-      else:
+        try:
+          step_result = api.m.json.read(
+              'Reading chartjson results', chartjson_path)
+        except api.m.step.StepFailure:
+          pass
+        else:
+          if step_result.json.output:
+            results['chartjson_paths'].append(chartjson_path)
+      if use_valueset:
+        try:
+          step_result = api.m.json.read(
+              'Reading valueset results', valueset_path,
+              step_test_data=lambda: api.m.json.test_api.output(
+                  {'dummy':'dict'}))
+        except api.m.step.StepFailure:
+          pass
+        else:
+          if step_result.json.output:
+            results['valueset_paths'].append(valueset_path)
+      if not use_valueset and not use_chartjson:
         has_valid_value, value = parse_metric.parse_metric(
             out, err, metric.split('/'))
-      output_for_all_runs.append(out)
-      if has_valid_value:
-        values.extend(value)
-      else:
-        # This means the metric was not found in the output.
-        if not retcode:
-          # If all tests passed, but the metric was not found, this means that
-          # something changed on the test, or the given metric name was
-          # incorrect, we need to surface this on the bisector.
-          run_results['errors'].add('MISSING_METRIC')
-    else:
-      output_for_all_runs.append(out)
-    retcodes.append(retcode)
+        if has_valid_value:
+          results['values'].extend(value)
+        else:
+          # This means the metric was not found in the output.
+          if not retcode:
+            # If all tests passed, but the metric was not found, this means that
+            # something changed on the test, or the given metric name was
+            # incorrect, we need to surface this on the bisector.
+            results['errors'].add('MISSING_METRIC')
+    results['retcodes'].append(retcode)
 
-  return run_results, output_for_all_runs, retcodes
-
+  return results
 
 def find_values(results, metric):  # pragma: no cover
   """Tries to extract the given metric from the given results.
 
   This method tries several different possible chart names depending
   on the given metric.
 
   Args:
     results: The chartjson dict.
     metric: A Metric instance.
@@ skipping 33 matching lines @@
   """
   if (file_path.startswith('src/') or file_path.startswith('./src/')):
     return api.m.path['checkout'].join(
         *file_path.split('src', 1)[1].split('/')[1:])
   elif (file_path.startswith('src\\') or
         file_path.startswith('.\\src\\')):  # pragma: no cover
     return api.m.path['checkout'].join(
         *file_path.split('src', 1)[1].split('\\')[1:])
   return file_path
 
-def _run_command(api, command, step_name):
+def _run_command(api, command, step_name, **kwargs):
   command_parts = command.split()
   stdout = api.m.raw_io.output()
   stderr = api.m.raw_io.output()
 
   # TODO(prasadv): Remove this once bisect runs are no longer running
   # against revisions from February 2016 or earlier.
-  kwargs = {}
   if 'android-chrome' in command:  # pragma: no cover
     kwargs['env'] = {'CHROMIUM_OUTPUT_DIR': api.m.chromium.output_dir}
 
   # By default, we assume that the test to run is an executable binary. In the
   # case of python scripts, runtest.py will guess based on the extension.
   python_mode = False
   if command_parts[0] == 'python':  # pragma: no cover
     # Dashboard prepends the command with 'python' when on windows, however, it
     # is not necessary to pass this along to the runtest.py invocation.
     # TODO(robertocn): Remove this clause when dashboard stops sending python as
@@ skipping 18 matching lines @@
     step_result.presentation.logs['Captured Output'] = (
         step_result.stdout or '').splitlines()
   except api.m.step.StepFailure as sf:
     sf.result.presentation.logs['Failure Output'] = (
         sf.result.stdout or '').splitlines()
     if sf.result.stderr:  # pragma: no cover
       sf.result.presentation.logs['stderr'] = (
         sf.result.stderr).splitlines()
     return sf.result.stdout, sf.result.stderr, sf.result.retcode
   return step_result.stdout, step_result.stderr, step_result.retcode