Refactor stages 1, 2 and test_api overhaul.

scripts/slave/recipe_modules/auto_bisect/bisector.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 json
 import re
 import time
 import urllib
 
 from . import config_validation
@@ skipping 28 matching lines @@
     'LO_FINAL_CONF',  # The bisect completed without a culprit.
 )
 
 # When we look for the next revision to build, we search nearby revisions
 # looking for a revision that's already been archived. Since we don't want
 # to move *too* far from the original revision, we'll cap the search at 25%.
 DEFAULT_SEARCH_RANGE_PERCENTAGE = 0.25
 
 # How long to re-test the initial good-bad range for until significant
 # difference is established.
-REGRESSION_CHECK_TIMEOUT = 20 * 60 * 60  # 20 hours. A build times out after 24.
-# If we reach this number of samples on the reference range and have not
-# achieved statistical significance, bail.
-MAX_REQUIRED_SAMPLES = 15
-
-# Significance level to use for determining difference between revisions via
-# hypothesis testing.
-SIGNIFICANCE_LEVEL = 0.01
+REGRESSION_CHECK_TIMEOUT = 2 * 60 * 60
 
 _FAILED_INITIAL_CONFIDENCE_ABORT_REASON = (
     'The metric values for the initial "good" and "bad" revisions '
     'do not represent a clear regression.')
 
 _DIRECTION_OF_IMPROVEMENT_ABORT_REASON = (
     'The metric values for the initial "good" and "bad" revisions match the '
     'expected direction of improvement. Thus, likely represent an improvement '
     'and not a regression.')
 
@@ skipping 81 matching lines @@
 
     Returns:
       A 40-digit git commit hash string.
     """
     if self._is_sha1(rev):  # pragma: no cover
       return rev
     if rev.isdigit():
       commit_position = self._api.m.commit_position.construct(
           branch='refs/heads/master', value=rev)
       try:
-        return self._api.m.crrev.to_commit_hash(commit_position)
+        return self._api.m.crrev.to_commit_hash(
+            commit_position,
+            step_test_data=lambda: self.api._test_data['hash_cp_map'][rev])
       except self.api.m.step.StepFailure:  # pragma: no cover
         self.surface_result('BAD_REV')
         raise
     self.surface_result('BAD_REV')  # pragma: no cover
     raise self.api.m.step.StepFailure(
         'Invalid input revision: %r' % (rev,))  # pragma: no cover
 
   @staticmethod
   def _is_sha1(s):
     return bool(re.match('^[0-9A-Fa-f]{40}$', s))
 
-  def significantly_different(
-      self, list_a, list_b,
-      significance_level=SIGNIFICANCE_LEVEL):  # pragma: no cover
-    """Uses an external script to run hypothesis testing with scipy.
+  def compare_revisions(self, revision_a, revision_b):
+    """
+    Returns:

[prasadv, 2016/09/09 18:52:30]

I think it is better to have a consistent datatype value to return. 
In this case if there might be a confusion to evaluate False and None.
May be we should use some kind of enum here.

Same for the "needMoreData"

[RobertoCN, 2016/09/13 22:11:40]

Done.

+      True if the samples are significantly different.
+      None if there is not enough data to tell.
+      False if there's enough data but still can't tell the samples apart.
+    """
+    output_format = 'chartjson'
+    values_a = revision_a.chartjson_paths
+    values_b = revision_b.chartjson_paths
+    if revision_a.valueset_paths and revision_b.valueset_paths:
+      output_format = 'valueset'
 
-    The reason why we need an external script is that scipy is not available to
-    the default python installed in all platforms. We instead rely on an
-    anaconda environment to provide those packages.
+    result = self.api.stat_compare(
+        values_a,
+        values_b,
+        self.bisect_config['metric'],
+        output_format=output_format,
+        step_test_data=lambda: self.api.test_api.compare_samples_data(
+            self.api._test_data.get('revision_data'), revision_a, revision_b))
 
-    Args:
-      list_a, list_b: Two lists representing samples to be compared.
-      significance_level: Self-describing. As a decimal fraction.
+    revision_a.debug_values = result['sample_a']['debug_values']
+    revision_b.debug_values = result['sample_b']['debug_values']
+    revision_a.mean = result['sample_a']['mean']
+    revision_b.mean = result['sample_b']['mean']
+    revision_a.std_dev = result['sample_a']['std_dev']
+    revision_b.std_dev = result['sample_b']['std_dev']
 
-    Returns:
-      A boolean indicating whether the null hypothesis ~(that the lists are
-      samples from the same population) can be rejected at the specified
-      significance level.
-    """
-    step_result = self.api.m.python(
-        'Checking sample difference',
-        self.api.resource('significantly_different.py'),
-        [json.dumps(list_a), json.dumps(list_b), str(significance_level)],
-        stdout=self.api.m.json.output())
-    results = step_result.stdout
-    if results is None:
-      assert self.dummy_builds
-      return True
-    significantly_different = results['significantly_different']
-    step_result.presentation.logs[str(significantly_different)] = [
-        'See json.output for details']
-    return significantly_different
+    if result['result'] == 'needMoreData':
+      return None
+    return bool(result['result'])
 
   def config_step(self):
     """Yields a step that prints the bisect config."""
     api = self.api
 
     # bisect_config may come as a FrozenDict (which is not serializable).
     bisect_config = dict(self.bisect_config)
 
     def fix_windows_backslashes(s):
       backslash_regex = re.compile(r'(?<!\\)\\(?!\\)')
@@ skipping 17 matching lines @@
     except config_validation.ValidationFail as error:
       self.surface_result('BAD_CONFIG')
       self.api.m.halt(error.message)
       raise self.api.m.step.StepFailure(error.message)
 
   @property
   def api(self):
     return self._api
 
   def compute_relative_change(self):
-    old_value = float(self.good_rev.mean_value)
-    new_value = float(self.bad_rev.mean_value)
+    old_value = float(self.good_rev.mean or 0)
+    new_value = float(self.bad_rev.mean or 0)
 
     if new_value and not old_value:  # pragma: no cover
       self.relative_change = ZERO_TO_NON_ZERO
       return
 
     rel_change = self.api.m.math_utils.relative_change(old_value, new_value)
     self.relative_change = '%.2f%%' % (100 * rel_change)
 
   def make_deps_sha_file(self, deps_sha):
     """Make a diff patch that creates DEPS.sha.
@@ skipping 16 matching lines @@
         file is to be written to.
       commit_hash (str): An identifier for the step.
 
     Returns:
       A string containing the hash of the interned object.
     """
     cmd = 'hash-object -t blob -w --stdin'.split(' ')
     stdin = self.api.m.raw_io.input(file_contents)
     stdout = self.api.m.raw_io.output()
     step_name = 'Hashing modified DEPS file with revision ' + commit_hash
-    step_result = self.api.m.git(*cmd, cwd=cwd, stdin=stdin, stdout=stdout,
-                                 name=step_name)
+    step_result = self.api.m.git(
+        *cmd, cwd=cwd, stdin=stdin, stdout=stdout, name=step_name,
+        step_test_data=lambda:
+            self.api.m.raw_io.test_api.stream_output(commit_hash))
     hash_string = step_result.stdout.splitlines()[0]
     try:
       if hash_string:
         int(hash_string, 16)
         return hash_string
     except ValueError:  # pragma: no cover
       reason = 'Git did not output a valid hash for the interned file.'
       self.api.m.halt(reason)
       raise self.api.m.step.StepFailure(reason)
 
@@ skipping 14 matching lines @@
     Returns:
       A string containing the diff patch as produced by the 'git diff' command.
     """
     # The prefixes used in the command below are used to find and replace the
     # tree-ish git object id's on the diff output more easily.
     cmd = 'diff %s %s --src-prefix=IAMSRC: --dst-prefix=IAMDST:'
     cmd %= (git_object_a, git_object_b)
     cmd = cmd.split(' ')
     stdout = self.api.m.raw_io.output()
     step_name = 'Generating patch for %s to %s' % (git_object_a, deps_rev)
-    step_result = self.api.m.git(*cmd, cwd=cwd, stdout=stdout, name=step_name)
+    step_result = self.api.m.git(
+        *cmd, cwd=cwd, stdout=stdout, name=step_name,
+        step_test_data=lambda: self.api._test_data['diff_patch'])
     patch_text = step_result.stdout
     src_string = 'IAMSRC:' + git_object_a
     dst_string = 'IAMDST:' + git_object_b
     patch_text = patch_text.replace(src_string, src_alias)
     patch_text = patch_text.replace(dst_string, dst_alias)
     return patch_text
 
   def make_deps_patch(self, base_revision, base_file_contents,
                       depot, new_commit_hash):
     """Make a diff patch that updates a specific dependency revision.
@@ skipping 42 matching lines @@
         interned_deps_hash, deps_file, deps_file,
         cwd=cwd,
         deps_rev=new_commit_hash)
     return patch_text, patched_contents
 
   def _expand_initial_revision_range(self):
     """Sets the initial contents of |self.revisions|."""
     with self.api.m.step.nest('Expanding revision range'):
       good_hash = self.good_rev.commit_hash
       bad_hash = self.bad_rev.commit_hash
+      depot = self.good_rev.depot_name
       step_name = 'for revisions %s:%s' % (good_hash, bad_hash)
       revisions = self._revision_range(
           start=good_hash,
           end=bad_hash,
           depot_name=self.base_depot,
           step_name=step_name,
-          exclude_end=True)
+          exclude_end=True,
+          step_test_data=lambda: self.api._test_data['revision_list'][depot]
+      )
       self.revisions = [self.good_rev] + revisions + [self.bad_rev]
       self._update_revision_list_indexes()
 
   def _revision_range(self, start, end, depot_name, base_revision=None,
-                      step_name=None, exclude_end=False):
+                      step_name=None, exclude_end=False, **kwargs):
     """Returns a list of RevisionState objects between |start| and |end|.
 
     When expanding the initial revision range we want to exclude the last
     revision, since both good and bad have already been created and tested.
     When bisecting into a roll on the other hand, we want to include the last
     revision in the roll, because although the code should be equivalent to
     the roll, we want to blame the right culprit and not the roll.
 
     Args:
       start (str): Start commit hash.
       end (str): End commit hash.
       depot_name (str): Short string name of repo, e.g. chromium or v8.
       base_revision (str): Base revision in the downstream repo (e.g. chromium).
       step_name (str): Optional step name.
       exclude_end (bool): Whether to exclude the last revision in the range,
           i.e. the revision given as end.
 
     Returns:
       A list of RevisionState objects.
     """
     if self.internal_bisect:  # pragma: no cover
       return self._revision_range_with_gitiles(
           start, end, depot_name, base_revision, step_name)
     try:
       step_result = self.api.m.python(
           step_name,
           self.api.resource('fetch_intervening_revisions.py'),
           [start, end, depot_config.DEPOT_DEPS_NAME[depot_name]['url']],
-          stdout=self.api.m.json.output())
+          stdout=self.api.m.json.output(), **kwargs)
     except self.api.m.step.StepFailure:  # pragma: no cover
       self.surface_result('BAD_REV')
       raise
     revisions = []
     revision_hashes = step_result.stdout
     if exclude_end:
       revision_hashes = revision_hashes[:-1]
     for commit_hash, _ in revision_hashes:
       revisions.append(self.revision_class(
           bisector=self,
@@ skipping 88 matching lines @@
           dep_revision_max = max_revision.deps[depot_name]
           if (dep_revision_min and dep_revision_max and
               dep_revision_min != dep_revision_max):
             step_name = ('Expanding revision range for revision %s'
                          ' on depot %s' % (dep_revision_max, depot_name))
             rev_list = self._revision_range(
                 start=dep_revision_min,
                 end=dep_revision_max,
                 depot_name=depot_name,
                 base_revision=min_revision,
-                step_name=step_name)
+                step_name=step_name,
+                step_test_data=lambda:
+                    self.api._test_data['revision_list'][depot_name])
             new_revisions = self.revisions[:max_revision.list_index]
             new_revisions += rev_list
             new_revisions += self.revisions[max_revision.list_index:]
             self.revisions = new_revisions
             self._update_revision_list_indexes()
             return True
     except RuntimeError:  # pragma: no cover
       warning_text = ('Could not expand dependency revisions for ' +
                       revision_to_expand.commit_hash)
       self.surface_result('BAD_REV')
@@ skipping 16 matching lines @@
 
     The change between the test results obtained for the given 'good' and
     'bad' revisions is expected to be considered a regression. The
     `improvement_direction` attribute is positive if a larger number is
     considered better, and negative if a smaller number is considered better.
 
     Returns:
       True if the check passes (i.e. no problem), False if the change is not
       a regression according to the improvement direction.
     """
-    good = self.good_rev.mean_value
-    bad = self.bad_rev.mean_value
+    good = self.good_rev.mean
+    bad = self.bad_rev.mean
 
     if self.is_return_code_mode():
       return True
 
     direction = self.improvement_direction
     if direction is None:
       return True
     if (bad > good and direction > 0) or (bad < good and direction < 0):
       self._set_failed_direction_results()
       return False
@@ skipping 14 matching lines @@
     """Checks that the initial range presents a clear enough regression.
 
     We ensure that the good and bad revisions produce significantly different
     results, increasing the sample size until MAX_REQUIRED_SAMPLES is reached
     or REGRESSION_CHECK_TIMEOUT seconds have elapsed.
 
     Returns: True if the revisions produced results that differ from each
     other in a statistically significant manner. False if such difference could
     not be established in the time or sample size allowed.
     """
-    if self.test_type == 'return_code':
+    if self.is_return_code_mode():
       return (self.good_rev.overall_return_code !=
               self.bad_rev.overall_return_code)
 
     if self.bypass_stats_check:
-      dummy_result = self.good_rev.values != self.bad_rev.values
+      self.compare_revisions(self.good_rev, self.bad_rev)
+      dummy_result = self.good_rev.mean != self.bad_rev.mean
       if not dummy_result:
         self._set_insufficient_confidence_warning()
       return dummy_result
 
+    # TODO(robertocn): This step should not be necessary in some cases.
     with self.api.m.step.nest('Re-testing reference range'):
       expiration_time = time.time() + REGRESSION_CHECK_TIMEOUT
       while time.time() < expiration_time:
-        if len(self.good_rev.values) >= 5 and len(self.bad_rev.values) >= 5:
-          if self.significantly_different(self.good_rev.values,
-                                          self.bad_rev.values):
+        if (self.good_rev.test_run_count >= 5
+            and self.bad_rev.test_run_count >= 5):
+          if self.compare_revisions(self.good_rev, self.bad_rev):
             return True
-        if len(self.good_rev.values) == len(self.bad_rev.values):
+        if self.good_rev.test_run_count == self.bad_rev.test_run_count:
           revision_to_retest = self.last_tested_revision
         else:
           revision_to_retest = min(self.good_rev, self.bad_rev,
-                                   key=lambda x: len(x.values))
-        if len(revision_to_retest.values) < MAX_REQUIRED_SAMPLES:
-          revision_to_retest.retest()
-        else:
-          break
+                                   key=lambda x: x.test_run_count)
+        revision_to_retest._do_test()
+
       self._set_insufficient_confidence_warning()
       return False
 
 
   def get_exception(self):
     raise NotImplementedError()  # pragma: no cover
     # TODO: should return an exception with the details of the failure.
 
   def _set_insufficient_confidence_warning(
       self):  # pragma: no cover
     """Adds a warning about the lack of initial regression confidence."""
     self.failed_initial_confidence = True
     self.surface_result('LO_INIT_CONF')
     self.warnings.append(
         'Bisect failed to reproduce the regression with enough confidence.')
 
   def _results_debug_message(self):
     """Returns a string with values used to debug a bisect result."""
     result = 'bisector.lkgr: %r\n' % self.lkgr
     result += 'bisector.fkbr: %r\n\n' % self.fkbr
     result += self._revision_value_table()
-    if (self.lkgr and self.lkgr.values and self.fkbr and self.fkbr.values):
-      result += '\n' + self._t_test_results()
+    if (self.lkgr and self.lkgr.test_run_count and self.fkbr and
+        self.fkbr.test_run_count):
+      result += '\n' + '\n'.join([
+          'LKGR values: %r' % list(self.lkgr.debug_values),
+          'FKBR values: %r' % list(self.fkbr.debug_values),
+      ])
     return result
 
   def _revision_value_table(self):
     """Returns a string table showing revisions and their values."""
     header = [['Revision', 'Values']]
-    rows = [[r.revision_string(), str(r.values)] for r in self.revisions]
+    rows = [[r.revision_string(), str(r.debug_values)] for r in self.revisions]
     return self._pretty_table(header + rows)
 
   def _pretty_table(self, data):
     results = []
     for row in data:
       results.append('%-15s' * len(row) % tuple(row))
     return '\n'.join(results)
 
-  def _t_test_results(self):
-    """Returns a string showing t-test results for lkgr and fkbr."""
-    t, df, p = self.api.m.math_utils.welchs_t_test(
-        self.lkgr.values, self.fkbr.values)
-    lines = [
-        'LKGR values: %r' % self.lkgr.values,
-        'FKBR values: %r' % self.fkbr.values,
-        't-statistic: %r' % t,
-        'deg. of freedom:  %r' % df,
-        'p-value: %r' % p,
-        'Confidence score: %r' % (100 * (1 - p))
-    ]
-    return '\n'.join(lines)
-
   def print_result_debug_info(self):
     """Prints extra debug info at the end of the bisect process."""
     lines = self._results_debug_message().splitlines()
     # If we emit a null step then add a log to it, the log should be kept
     # longer than 7 days (which is often needed to debug some issues).
     self.api.m.step('Debug Info', [])
     self.api.m.step.active_result.presentation.logs['Debug Info'] = lines
 
   def post_result(self, halt_on_failure=False):
     """Posts bisect results to Perf Dashboard."""
     self.api.m.perf_dashboard.set_default_config()
     self.api.m.perf_dashboard.post_bisect_results(
         self.get_result(), halt_on_failure)
 
   def get_revision_to_eval(self):
     """Gets the next RevisionState object in the candidate range.
 
     Returns:
        The next Revision object in a list.
     """
     self._update_candidate_range()
     candidate_range = [revision for revision in
                        self.revisions[self.lkgr.list_index + 1:
                                       self.fkbr.list_index]
-                       if not revision.tested and not revision.failed]
+                       if not revision.failed]
     if len(candidate_range) == 1:
       return candidate_range[0]
     if len(candidate_range) == 0:
       return None
 
     default_revision = candidate_range[len(candidate_range) / 2]
 
     with self.api.m.step.nest(
         'Wiggling revision ' + default_revision.revision_string()):
       # We'll search up to 25% of the range (in either direction) to try and
@@ skipping 22 matching lines @@
     return False
 
   def check_bisect_finished(self, revision):
     """Checks if this revision completes the bisection process.
 
     In this case 'finished' refers to finding one revision considered 'good'
     immediately preceding a revision considered 'bad' where the 'bad' revision
     does not contain a DEPS change.
     """
     if (revision.bad and revision.previous_revision and
-        revision.previous_revision.good):  # pragma: no cover
+        revision.previous_revision.good):
       if revision.deps_change() and self._expand_deps_revisions(revision):
         return False
       self.culprit = revision
       return True
     if (revision.good and revision.next_revision and
         revision.next_revision.bad):
       if (revision.next_revision.deps_change()
           and self._expand_deps_revisions(revision.next_revision)):
         return False
       self.culprit = revision.next_revision
       return True
-    return False
-
-  def wait_for_all(self, revision_list):
-    """Waits for all revisions in list to finish."""
-    for r in revision_list:
-      self.wait_for(r)
-
-  def wait_for(self, revision, nest_check=True):
-    """Waits for the revision to finish its job."""
-    if nest_check and not self.flags.get(
-        'do_not_nest_wait_for_revision'):  # pragma: no cover
-      with self.api.m.step.nest('Waiting for ' + revision.revision_string()):
-        return self.wait_for(revision, nest_check=False)
-    while True:
-      revision.update_status()
-      if revision.in_progress:
-        self.api.m.python.inline(
-            'sleeping',
-            """
-            import sys
-            import time
-            time.sleep(20*60)
-            sys.exit(0)
-            """)
-      else:
-        break
+    # We'll never get here because revision adjacency is checked before this
+    # function is called.
+    assert False  # pragma: no cover
 
   def _update_candidate_range(self):
     """Updates lkgr and fkbr (last known good/first known bad) revisions.
 
     lkgr and fkbr are 'pointers' to the appropriate RevisionState objects in
     bisectors.revisions."""
     for r in self.revisions:
-      if r.tested:
+      if r.test_run_count:
         if r.good:
           self.lkgr = r
         elif r.bad:
           self.fkbr = r
           break
     assert self.lkgr and self.fkbr
 
   def get_perf_tester_name(self):
     """Gets the name of the tester bot (on tryserver.chromium.perf) to use.
 
@@ skipping 17 matching lines @@
 
     # TODO(prasadv): Refactor this code to remove hard coded values.
     bot_name = self.get_perf_tester_name()
     if 'win' in bot_name:
       if any(b in bot_name for b in ['x64', 'gpu']):
         return 'winx64_bisect_builder'
       return 'win_perf_bisect_builder'
 
     # TODO(prasadv): Refactor this code to remove hard coded values and use
     # target_bit from the bot config. crbug.com/640287
-    if 'android' in bot_name:
+    if 'android' in bot_name:  # pragma: no cover
       if any(b in bot_name for b in ['arm64', 'nexus9', 'nexus5X']):
         return 'android_arm64_perf_bisect_builder'
       return 'android_perf_bisect_builder'
 
     if 'mac' in bot_name:
       return 'mac_perf_bisect_builder'
 
     return 'linux_perf_bisect_builder'
 
   def get_platform_gs_prefix(self):
     """Returns the prefix of a GS URL where a build can be found.
 
     This prefix includes the schema, bucket, directory and beginning
     of filename. It is joined together with the part of the filename
     that includes the revision and the file extension to form the
     full GS URL.
     """
     if self.api.buildurl_gs_prefix:  # pragma: no cover
       return self.api.buildurl_gs_prefix
 
     # TODO(prasadv): Refactor this code to remove hard coded values.
     bot_name = self.get_perf_tester_name()
     if 'win' in bot_name:
       if any(b in bot_name for b in ['x64', 'gpu']):
         return 'gs://chrome-perf/Win x64 Builder/full-build-win32_'
       return 'gs://chrome-perf/Win Builder/full-build-win32_'
 
     # TODO(prasadv): Refactor this code to remove hard coded values and use
     # target_bit from the bot config. crbug.com/640287
-    if 'android' in bot_name:
+    if 'android' in bot_name: #pragma: no cover
       if any(b in bot_name for b in ['arm64', 'nexus9', 'nexus5X']):
         return 'gs://chrome-perf/Android arm64 Builder/full-build-linux_'
       return 'gs://chrome-perf/Android Builder/full-build-linux_'
 
     if 'mac' in bot_name:
       return 'gs://chrome-perf/Mac Builder/full-build-mac_'
 
     return 'gs://chrome-perf/Linux Builder/full-build-linux_'
 
   def ensure_sync_master_branch(self):
     """Make sure the local master is in sync with the fetched origin/master.
 
     We have seen on several occasions that the local master branch gets reset
     to previous revisions and also detached head states. Running this should
     take care of either situation.
     """
     # TODO(robertocn): Investigate what causes the states mentioned in the
     # docstring in the first place.
     self.api.m.git('update-ref', 'refs/heads/master',
                    'refs/remotes/origin/master')
     self.api.m.git('checkout', 'master', cwd=self.api.m.path['checkout'])
 
   def is_return_code_mode(self):
     """Checks whether this is a bisect on the test's exit code."""
-    return self.bisect_config.get('test_type') == 'return_code'
+    return self.test_type == 'return_code'
 
   def surface_result(self, result_string):
     assert result_string in VALID_RESULT_CODES
     prefix = 'B4T_'  # To avoid collision. Stands for bisect (abbr. `a la i18n).
     result_code = prefix + result_string
     assert len(result_code) <= 20
     if result_code not in self.result_codes:
       self.result_codes.add(result_code)
       properties = self.api.m.step.active_result.presentation.properties
       properties['extra_result_code'] = sorted(self.result_codes)
 
   def get_result(self):
     """Returns the results as a jsonable object."""
     config = self.bisect_config
-    results_confidence = 0
-    if self.culprit:
-      results_confidence = self.api.m.math_utils.confidence_score(
-          self.lkgr.values, self.fkbr.values)
 
     if self.failed:
       status = 'failed'
     elif self.bisect_over:
       status = 'completed'
     else:
       status = 'started'
 
     aborted_reason = None
     if self.failed_initial_confidence:
       aborted_reason = _FAILED_INITIAL_CONFIDENCE_ABORT_REASON
     elif self.failed_direction:
       aborted_reason = _DIRECTION_OF_IMPROVEMENT_ABORT_REASON
     return {
         'try_job_id': config.get('try_job_id'),
         'bug_id': config.get('bug_id'),
         'status': status,
         'buildbot_log_url': self._get_build_url(),
         'bisect_bot': self.get_perf_tester_name(),
         'command': config['command'],
         'test_type': config['test_type'],
-        'metric': config['metric'],
+        'metric': config.get('metric'),
         'change': self.relative_change,
-        'score': results_confidence,
         'good_revision': self.good_rev.commit_hash,
         'bad_revision': self.bad_rev.commit_hash,
         'warnings': self.warnings,
         'aborted_reason': aborted_reason,
         'culprit_data': self._culprit_data(),
         'revision_data': self._revision_data()
     }
 
   def _culprit_data(self):
     culprit = self.culprit
@@ skipping 11 matching lines @@
         'email': culprit_info['email'],
         'cl_date': culprit_info['date'],
         'commit_info': culprit_info['body'],
         'revisions_links': [],
         'cl': culprit.commit_hash
     }
 
   def _revision_data(self):
     revision_rows = []
     for r in self.revisions:
-      if r.tested or r.aborted:
+      if r.test_run_count:
         revision_rows.append({
             'depot_name': r.depot_name,
             'commit_hash': r.commit_hash,
             'revision_string': r.revision_string(),
-            'mean_value': r.mean_value,
+            'mean_value': r.mean,
             'std_dev': r.std_dev,
-            'values': r.values,
+            'values': r.debug_values,
             'result': 'good' if r.good else 'bad' if r.bad else 'unknown',
         })
     return revision_rows
 
   def _get_build_url(self):
     properties = self.api.m.properties
     bot_url = properties.get('buildbotURL',
                              'http://build.chromium.org/p/chromium/')
     builder_name = urllib.quote(properties.get('buildername', ''))
     builder_number = str(properties.get('buildnumber', ''))
     return '%sbuilders/%s/builds/%s' % (bot_url, builder_name, builder_number)