Refactored auto_bisect bot:

 - Extracted BisectPrinter, which contains everything related to printing
 - Extracted BisectState and RevisionState, which represent bisect-in-progress
 - Rewrote BisectResults - in particular split up GetRevisionDict, which is now its constructor
 - Added tests for BisectResults, fixed bugs in _FindOtherRegressions
 - Added tests for BisectState

R=qyearsley@chromium.org

Review URL: https://codereview.chromium.org/664753002

Cr-Commit-Position: refs/heads/master@{#300677}

tools/auto_bisect/bisect_perf_regression_test.py

@@ -12,7 +12,8 @@ SRC = os.path.join(os.path.dirname(__file__), os.path.pardir, os.path.pardir)
 sys.path.append(os.path.join(SRC, 'third_party', 'pymock'))
 
 import bisect_perf_regression
-import bisect_results
+import bisect_printer
+import bisect_utils
 import mock
 import source_control
 
@@ -26,15 +27,13 @@ DEFAULT_OPTIONS = {
     'metric': 'fake/metric',
     'good_revision': 280000,
     'bad_revision': 280005,
-  }
+}
 
 
 def _GetBisectPerformanceMetricsInstance(options_dict):
   """Returns an instance of the BisectPerformanceMetrics class."""
-  bisect_options = bisect_perf_regression.BisectOptions.FromDict(options_dict)
-  bisect_instance = bisect_perf_regression.BisectPerformanceMetrics(
-      bisect_options)
-  return bisect_instance
+  opts = bisect_perf_regression.BisectOptions.FromDict(options_dict)
+  return bisect_perf_regression.BisectPerformanceMetrics(opts)
 
 
 def _GetExtendedOptions(d, f):
@@ -61,13 +60,14 @@ def _GenericDryRun(options, print_results=False):
   try:
     shutil.rmtree = lambda path, onerror: None
     bisect_instance = _GetBisectPerformanceMetricsInstance(options)
-    results = bisect_instance.Run(bisect_instance.opts.command,
-                                  bisect_instance.opts.bad_revision,
-                                  bisect_instance.opts.good_revision,
-                                  bisect_instance.opts.metric)
+    results = bisect_instance.Run(
+        bisect_instance.opts.command, bisect_instance.opts.bad_revision,
+        bisect_instance.opts.good_revision, bisect_instance.opts.metric)
 
     if print_results:
-      bisect_instance.FormatAndPrintResults(results)
+      printer = bisect_printer.BisectPrinter(bisect_instance.opts,
+                                             bisect_instance.depot_registry)
+      printer.FormatAndPrintResults(results)
 
     return results
   finally:
@@ -85,74 +85,6 @@ class BisectPerfRegressionTest(unittest.TestCase):
   def tearDown(self):
     os.chdir(self.cwd)
 
-  def _AssertConfidence(self, score, bad_values, good_values):
-    """Checks whether the given sets of values have a given confidence score.
-
-    The score represents our confidence that the two sets of values wouldn't
-    be as different as they are just by chance; that is, that some real change
-    occurred between the two sets of values.
-
-    Args:
-      score: Expected confidence score.
-      bad_values: First list of numbers.
-      good_values: Second list of numbers.
-    """
-    # ConfidenceScore takes a list of lists but these lists are flattened
-    # inside the function.
-    confidence = bisect_results.ConfidenceScore(
-        [[v] for v in bad_values],
-        [[v] for v in good_values])
-    self.assertEqual(score, confidence)
-
-  def testConfidenceScore_ZeroConfidence(self):
-    # The good and bad sets contain the same values, so the confidence that
-    # they're different should be zero.
-    self._AssertConfidence(0.0, [4, 5, 7, 6, 8, 7], [8, 7, 6, 7, 5, 4])
-
-  def testConfidenceScore_MediumConfidence(self):
-    self._AssertConfidence(80.0, [0, 1, 1, 1, 2, 2], [1, 1, 1, 3, 3, 4])
-
-  def testConfidenceScore_HighConfidence(self):
-    self._AssertConfidence(95.0, [0, 1, 1, 1, 2, 2], [1, 2, 2, 3, 3, 4])
-
-  def testConfidenceScore_VeryHighConfidence(self):
-    # Confidence is high if the two sets of values have no internal variance.
-    self._AssertConfidence(99.9, [1, 1, 1, 1], [1.2, 1.2, 1.2, 1.2])
-    self._AssertConfidence(99.9, [1, 1, 1, 1], [1.01, 1.01, 1.01, 1.01])
-
-  def testConfidenceScore_UnbalancedSampleSize(self):
-    # The second set of numbers only contains one number, so confidence is 0.
-    self._AssertConfidence(0.0, [1.1, 1.2, 1.1, 1.2, 1.0, 1.3, 1.2], [1.4])
-
-  def testConfidenceScore_EmptySample(self):
-    # Confidence is zero if either or both samples are empty.
-    self._AssertConfidence(0.0, [], [])
-    self._AssertConfidence(0.0, [], [1.1, 1.2, 1.1, 1.2, 1.0, 1.3, 1.2, 1.3])
-    self._AssertConfidence(0.0, [1.1, 1.2, 1.1, 1.2, 1.0, 1.3, 1.2, 1.3], [])
-
-  def testConfidenceScore_FunctionalTestResults(self):
-    self._AssertConfidence(80.0, [1, 1, 0, 1, 1, 1, 0, 1], [0, 0, 1, 0, 1, 0])
-    self._AssertConfidence(99.9, [1, 1, 1, 1, 1, 1, 1, 1], [0, 0, 0, 0, 0, 0])
-
-  def testConfidenceScore_RealWorldCases(self):
-    """This method contains a set of data from actual bisect results.
-
-    The confidence scores asserted below were all copied from the actual
-    results, so the purpose of this test method is mainly to show what the
-    results for real cases are, and compare when we change the confidence
-    score function in the future.
-    """
-    self._AssertConfidence(80, [133, 130, 132, 132, 130, 129], [129, 129, 125])
-    self._AssertConfidence(99.5, [668, 667], [498, 498, 499])
-    self._AssertConfidence(80, [67, 68], [65, 65, 67])
-    self._AssertConfidence(0, [514], [514])
-    self._AssertConfidence(90, [616, 613, 607, 615], [617, 619, 619, 617])
-    self._AssertConfidence(0, [3.5, 5.8, 4.7, 3.5, 3.6], [2.8])
-    self._AssertConfidence(90, [3, 3, 3], [2, 2, 2, 3])
-    self._AssertConfidence(0, [1999004, 1999627], [223355])
-    self._AssertConfidence(90, [1040, 934, 961], [876, 875, 789])
-    self._AssertConfidence(90, [309, 305, 304], [302, 302, 299, 303, 298])
-
   def testParseDEPSStringManually(self):
     """Tests DEPS parsing."""
     deps_file_contents = """
@@ -249,7 +181,7 @@ class BisectPerfRegressionTest(unittest.TestCase):
         bisect_options)
     bisect_instance.opts.target_platform = target_platform
     git_revision = source_control.ResolveToRevision(
-        revision, 'chromium', bisect_perf_regression.DEPOT_DEPS_NAME, 100)
+        revision, 'chromium', bisect_utils.DEPOT_DEPS_NAME, 100)
     depot = 'chromium'
     command = bisect_instance.GetCompatibleCommand(
         original_command, git_revision, depot)
@@ -320,7 +252,6 @@ class BisectPerfRegressionTest(unittest.TestCase):
     results = _GenericDryRun(_GetExtendedOptions(1, -100))
     self.assertIsNone(results.error)
 
-
   def testGetCommitPosition(self):
     cp_git_rev = '7017a81991de983e12ab50dfc071c70e06979531'
     self.assertEqual(291765, source_control.GetCommitPosition(cp_git_rev))
@@ -366,18 +297,18 @@ class DepotDirectoryRegistryTest(unittest.TestCase):
   def setUp(self):
     self.old_chdir = os.chdir
     os.chdir = self.mockChdir
-    self.old_depot_names = bisect_perf_regression.DEPOT_NAMES
-    bisect_perf_regression.DEPOT_NAMES = ['mock_depot']
-    self.old_depot_deps_name = bisect_perf_regression.DEPOT_DEPS_NAME
-    bisect_perf_regression.DEPOT_DEPS_NAME = {'mock_depot': {'src': 'src/foo'}}
+    self.old_depot_names = bisect_utils.DEPOT_NAMES
+    bisect_utils.DEPOT_NAMES = ['mock_depot']
+    self.old_depot_deps_name = bisect_utils.DEPOT_DEPS_NAME
+    bisect_utils.DEPOT_DEPS_NAME = {'mock_depot': {'src': 'src/foo'}}
 
     self.registry = bisect_perf_regression.DepotDirectoryRegistry('/mock/src')
     self.cur_dir = None
 
   def tearDown(self):
     os.chdir = self.old_chdir
-    bisect_perf_regression.DEPOT_NAMES = self.old_depot_names
-    bisect_perf_regression.DEPOT_DEPS_NAME = self.old_depot_deps_name
+    bisect_utils.DEPOT_NAMES = self.old_depot_names
+    bisect_utils.DEPOT_DEPS_NAME = self.old_depot_deps_name
 
   def mockChdir(self, new_dir):
     self.cur_dir = new_dir

tools/auto_bisect/bisect_state.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.
+
+
+class RevisionState(object):
+  """Contains bisect state for a given revision.
+
+  Properties:
+    depot: The depot that this revision is from (e.g. WebKit).
+    revision: Revision number (Git hash or SVN number).
+    index: Position of the state in the list of all revisions.
+    value: Value(s) returned from the test.
+    perf_time: Time that a test took.
+    build_time: Time that a build took.
+    passed: Represents whether the performance test was successful at that
+        revision. Possible values include: 1 (passed), 0 (failed),
+        '?' (skipped), 'F' (build failed).
+    external: If the revision is a 'src' revision, 'external' contains the
+        revisions of each of the external libraries.
+  """
+
+  def __init__(self, depot, revision, index):
+    self.depot = depot
+    self.revision = revision
+    self.index = index
+    self.value = None
+    self.perf_time = 0
+    self.build_time = 0
+    self.passed = '?'
+    self.external = None
+
+  # TODO(sergiyb): Update() to parse run_results from the RunTest.
+
+
+class BisectState(object):
+  """Represents a state of the bisect as a collection of revision states."""
+
+  def __init__(self, depot, revisions):
+    """Initializes a new BisectState object with a set of revision states.
+
+    Args:
+      depot: Name of the depot used for initial set of revision states.
+      revisions: List of revisions used for initial set of revision states.
+    """
+    self.revision_states = []
+    self.revision_index = {}
+
+    index = 0
+    for revision in revisions:
+      new_state = self._InitRevisionState(depot, revision, index)
+      self.revision_states.append(new_state)
+      index += 1
+
+  @staticmethod
+  def _RevisionKey(depot, revision):
+    return "%s:%s" % (depot, revision)
+
+  def _InitRevisionState(self, depot, revision, index):
+    key = self._RevisionKey(depot, revision)
+    self.revision_index[key] = index
+    return RevisionState(depot, revision, index)
+
+  def GetRevisionState(self, depot, revision):
+    """Returns a mutable revision state."""
+    key = self._RevisionKey(depot, revision)
+    index = self.revision_index.get(key)
+    return self.revision_states[index] if index else None
+
+  def CreateRevisionStatesAfter(self, depot, revisions, reference_depot,
+                                reference_revision):
+    """Creates a set of new revision states after a specified reference state.
+
+    Args:
+      depot: Name of the depot for the new revision states.
+      revisions: List of revisions for the new revision states.
+      reference_depot: Name of the depot for the reference revision state.
+      reference_revision: Revision for the reference revision state.
+
+    Returns:
+      A list containing all created revision states in order as they were added.
+    """
+    ref_key = self._RevisionKey(reference_depot, reference_revision)
+    ref_index = self.revision_index[ref_key]
+    num_new_revisions = len(revisions)
+    for entry in self.revision_states:
+      if entry.index > ref_index:
+        entry.index += num_new_revisions
+
+    first_index = ref_index + 1
+    for index, revision in enumerate(revisions, start=first_index):
+      new_state = self._InitRevisionState(depot, revision, index)
+      self.revision_states.insert(index, new_state)
+
+    return self.revision_states[first_index:first_index + num_new_revisions]
+
+  def GetRevisionStates(self):
+    """Returns a copy of the list of the revision states."""
+    return list(self.revision_states)

tools/auto_bisect/bisect_state_test.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 unittest
+
+from bisect_state import BisectState
+
+
+class BisectStateTest(unittest.TestCase):
+
+  def testCreatesRevisionsStateAfterAReferenceRevision(self):
+    bisect_state = BisectState('chromium', ['a', 'b', 'c', 'd'])
+    bisect_state.CreateRevisionStatesAfter('webkit', [1, 2, 3], 'chromium', 'b')
+    bisect_state.CreateRevisionStatesAfter('v8', [100, 200], 'webkit', 2)
+
+    actual_revisions = bisect_state.GetRevisionStates()
+    expected_revisions = [('chromium', 'a'), ('chromium', 'b'), ('webkit', 1),
+                          ('webkit', 2), ('v8', 100), ('v8', 200),
+                          ('webkit', 3), ('chromium', 'c'), ('chromium', 'd')]
+    self.assertEqual(len(expected_revisions), len(actual_revisions))
+    for i in xrange(len(actual_revisions)):
+      self.assertEqual(i, actual_revisions[i].index)
+      self.assertEqual(expected_revisions[i][0], actual_revisions[i].depot)
+      self.assertEqual(expected_revisions[i][1], actual_revisions[i].revision)
+
+  # TODO(sergiyb): More tests for the remaining functions.
+
+
+if __name__ == '__main__':
+  unittest.main()

tools/auto_bisect/bisect_utils.py

@@ -87,6 +87,116 @@ REPO_PARAMS = [
 # Bisect working directory.
 BISECT_DIR = 'bisect'
 
+# The percentage at which confidence is considered high.
+HIGH_CONFIDENCE = 95
+
+# Below is the map of "depot" names to information about each depot. Each depot
+# is a repository, and in the process of bisecting, revision ranges in these
+# repositories may also be bisected.
+#
+# Each depot information dictionary may contain:
+#   src: Path to the working directory.
+#   recurse: True if this repository will get bisected.
+#   depends: A list of other repositories that are actually part of the same
+#       repository in svn. If the repository has any dependent repositories
+#       (e.g. skia/src needs skia/include and skia/gyp to be updated), then
+#       they are specified here.
+#   svn: URL of SVN repository. Needed for git workflow to resolve hashes to
+#       SVN revisions.
+#   from: Parent depot that must be bisected before this is bisected.
+#   deps_var: Key name in vars variable in DEPS file that has revision
+#       information.
+DEPOT_DEPS_NAME = {
+    'chromium': {
+        'src': 'src',
+        'recurse': True,
+        'depends': None,
+        'from': ['cros', 'android-chrome'],
+        'viewvc':
+            'http://src.chromium.org/viewvc/chrome?view=revision&revision=',
+        'deps_var': 'chromium_rev'
+    },
+    'webkit': {
+        'src': 'src/third_party/WebKit',
+        'recurse': True,
+        'depends': None,
+        'from': ['chromium'],
+        'viewvc':
+            'http://src.chromium.org/viewvc/blink?view=revision&revision=',
+        'deps_var': 'webkit_revision'
+    },
+    'angle': {
+        'src': 'src/third_party/angle',
+        'src_old': 'src/third_party/angle_dx11',
+        'recurse': True,
+        'depends': None,
+        'from': ['chromium'],
+        'platform': 'nt',
+        'deps_var': 'angle_revision'
+    },
+    'v8': {
+        'src': 'src/v8',
+        'recurse': True,
+        'depends': None,
+        'from': ['chromium'],
+        'custom_deps': GCLIENT_CUSTOM_DEPS_V8,
+        'viewvc': 'https://code.google.com/p/v8/source/detail?r=',
+        'deps_var': 'v8_revision'
+    },
+    'v8_bleeding_edge': {
+        'src': 'src/v8_bleeding_edge',
+        'recurse': True,
+        'depends': None,
+        'svn': 'https://v8.googlecode.com/svn/branches/bleeding_edge',
+        'from': ['v8'],
+        'viewvc': 'https://code.google.com/p/v8/source/detail?r=',
+        'deps_var': 'v8_revision'
+    },
+    'skia/src': {
+        'src': 'src/third_party/skia/src',
+        'recurse': True,
+        'svn': 'http://skia.googlecode.com/svn/trunk/src',
+        'depends': ['skia/include', 'skia/gyp'],
+        'from': ['chromium'],
+        'viewvc': 'https://code.google.com/p/skia/source/detail?r=',
+        'deps_var': 'skia_revision'
+    },
+    'skia/include': {
+        'src': 'src/third_party/skia/include',
+        'recurse': False,
+        'svn': 'http://skia.googlecode.com/svn/trunk/include',
+        'depends': None,
+        'from': ['chromium'],
+        'viewvc': 'https://code.google.com/p/skia/source/detail?r=',
+        'deps_var': 'None'
+    },
+    'skia/gyp': {
+        'src': 'src/third_party/skia/gyp',
+        'recurse': False,
+        'svn': 'http://skia.googlecode.com/svn/trunk/gyp',
+        'depends': None,
+        'from': ['chromium'],
+        'viewvc': 'https://code.google.com/p/skia/source/detail?r=',
+        'deps_var': 'None'
+    }
+}
+
+DEPOT_NAMES = DEPOT_DEPS_NAME.keys()
+
+# The possible values of the --bisect_mode flag, which determines what to
+# use when classifying a revision as "good" or "bad".
+BISECT_MODE_MEAN = 'mean'
+BISECT_MODE_STD_DEV = 'std_dev'
+BISECT_MODE_RETURN_CODE = 'return_code'
+
+
+def AddAdditionalDepotInfo(depot_info):
+  """Adds additional depot info to the global depot variables."""
+  global DEPOT_DEPS_NAME
+  global DEPOT_NAMES
+  DEPOT_DEPS_NAME = dict(DEPOT_DEPS_NAME.items() + depot_info.items())
+  DEPOT_NAMES = DEPOT_DEPS_NAME.keys()
+
 
 def OutputAnnotationStepStart(name):
   """Outputs annotation to signal the start of a step to a try bot.