llvm_tools: refactor LLVM bisection tool

BUG=chromium:1081457
TEST=Verified locally.

Change-Id: Ic662a7bb697efb920a83255d3da87a0031e694ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/2371502
Tested-by: Jian Cai <jiancai@google.com>
Reviewed-by: George Burgess <gbiv@chromium.org>

2 files changed, 274 insertions, 535 deletions

diff --git a/llvm_tools/llvm_bisection.py b/llvm_tools/llvm_bisection.py
index 2772ca48..37320baf 100755
--- a/llvm_tools/llvm_bisection.py
+++ b/llvm_tools/llvm_bisection.py
@@ -17,6 +17,7 @@ import sys
 
 import chroot
 import get_llvm_hash
+import git_llvm_rev
 import modify_a_tryjob
 import update_tryjob_status
 
@@ -28,11 +29,6 @@ class BisectionExitStatus(enum.Enum):
   BISECTION_COMPLETE = 126
 
 
-def is_file_and_json(json_file):
-  """Validates that the file exists and is a JSON file."""
-  return os.path.isfile(json_file) and json_file.endswith('.json')
-
-
 def GetCommandLineArgs():
   """Parses the command line for the command line arguments."""
 
@@ -124,27 +120,17 @@ def GetCommandLineArgs():
   args_output = parser.parse_args()
 
   assert args_output.start_rev < args_output.end_rev, (
-      'Start revision %d is >= end revision %d' % (args_output.start_rev,
-                                                   args_output.end_rev))
+      'Start revision %d is >= end revision %d' %
+      (args_output.start_rev, args_output.end_rev))
 
   if args_output.last_tested and not args_output.last_tested.endswith('.json'):
-    raise ValueError(
-        'Filed provided %s does not end in ".json"' % args_output.last_tested)
+    raise ValueError('Filed provided %s does not end in ".json"' %
+                     args_output.last_tested)
 
   return args_output
 
 
-def _ValidateStartAndEndAgainstJSONStartAndEnd(start, end, json_start,
-                                               json_end):
-  """Valides that the command line arguments are the same as the JSON."""
-
-  if start != json_start or end != json_end:
-    raise ValueError('The start %d or the end %d version provided is '
-                     'different than "start" %d or "end" %d in the .JSON '
-                     'file' % (start, end, json_start, json_end))
-
-
-def GetStartAndEndRevision(start, end, tryjobs):
+def GetRemainingRange(start, end, tryjobs):
   """Gets the start and end intervals in 'json_file'.
 
   Args:
@@ -230,145 +216,56 @@ def GetStartAndEndRevision(start, end, tryjobs):
   return good_rev, bad_rev, pending_revisions, skip_revisions
 
 
-def GetRevisionsBetweenBisection(start, end, parallel, src_path,
-                                 pending_revisions, skip_revisions):
-  """Gets the revisions between 'start' and 'end'.
-
-  Sometimes, the LLVM source tree's revisions do not increment by 1 (there is
-  a jump), so need to construct a list of all revisions that are NOT missing
-  between 'start' and 'end'. Then, the step amount (i.e. length of the list
-  divided by ('parallel' + 1)) will be used for indexing into the list.
-
-  Args:
-    start: The start revision.
-    end: The end revision.
-    parallel: The number of tryjobs to create between 'start' and 'end'.
-    src_path: The absolute path to the LLVM source tree to use.
-    pending_revisions: A set containing 'pending' revisions that are between
-    'start' and 'end'.
-    skip_revisions: A set containing revisions between 'start' and 'end' that
-    are to be skipped.
-
-  Returns:
-    A list of revisions between 'start' and 'end'.
-  """
-
-  valid_revisions = []
-
-  # Start at ('start' + 1) because 'start' is the good revision.
-  #
-  # FIXME: Searching for each revision from ('start' + 1) up to 'end' in the
-  # LLVM source tree is a quadratic algorithm. It's a good idea to optimize
-  # this.
-  for cur_revision in range(start + 1, end):
-    try:
-      if cur_revision not in pending_revisions and \
-          cur_revision not in skip_revisions:
-        # Verify that the current revision exists by finding its corresponding
-        # git hash in the LLVM source tree.
-        get_llvm_hash.GetGitHashFrom(src_path, cur_revision)
-        valid_revisions.append(cur_revision)
-    except ValueError:
-      # Could not find the git hash for the current revision.
-      continue
-
-  # ('parallel' + 1) so that the last revision in the list is not close to
-  # 'end' (have a bit more coverage).
-  index_step = len(valid_revisions) // (parallel + 1)
-
-  if not index_step:
-    index_step = 1
-
-  result = [valid_revisions[index] \
-            for index in range(0, len(valid_revisions), index_step)]
-
-  return result
-
-
-def GetRevisionsListAndHashList(start, end, parallel, src_path,
-                                pending_revisions, skip_revisions):
+def GetCommitsBetween(start, end, parallel, src_path, pending_revisions,
+                      skip_revisions):
   """Determines the revisions between start and end."""
 
-  new_llvm = get_llvm_hash.LLVMHash()
+  with get_llvm_hash.LLVMHash().CreateTempDirectory() as temp_dir:
+    # We have guaranteed contiguous revision numbers after this,
+    # and that guarnatee simplifies things considerably, so we don't
+    # support anything before it.
+    assert start >= git_llvm_rev.base_llvm_revision, f'{start} was too long ago'
 
-  with new_llvm.CreateTempDirectory() as temp_dir:
     with get_llvm_hash.CreateTempLLVMRepo(temp_dir) as new_repo:
       if not src_path:
         src_path = new_repo
-
-      # Get a list of revisions between start and end.
-      revisions = GetRevisionsBetweenBisection(
-          start, end, parallel, src_path, pending_revisions, skip_revisions)
-
+      index_step = (end - (start + 1)) // (parallel + 1)
+      if not index_step:
+        index_step = 1
+      revisions = [
+          rev for rev in range(start + 1, end, index_step)
+          if rev not in pending_revisions and rev not in skip_revisions
+      ]
       git_hashes = [
           get_llvm_hash.GetGitHashFrom(src_path, rev) for rev in revisions
       ]
+      return revisions, git_hashes
 
-  return revisions, git_hashes
-
-
-def DieWithNoRevisionsError(start, end, skip_revisions, pending_revisions):
-  """Raises a ValueError exception with useful information."""
-
-  no_revisions_message = ('No revisions between start %d and end '
-                          '%d to create tryjobs' % (start, end))
-
-  if pending_revisions:
-    no_revisions_message += '\nThe following tryjobs are pending:\n' \
-        + '\n'.join(str(rev) for rev in pending_revisions)
-
-  if skip_revisions:
-    no_revisions_message += '\nThe following tryjobs were skipped:\n' \
-        + '\n'.join(str(rev) for rev in skip_revisions)
 
-  raise ValueError(no_revisions_message)
-
-
-def CheckForExistingTryjobsInRevisionsToLaunch(revisions, jobs):
-  """Checks if a revision in 'revisions' exists in 'jobs' list."""
-
-  for rev in revisions:
-    if update_tryjob_status.FindTryjobIndex(rev, jobs) is not None:
-      raise ValueError('Revision %d exists already in "jobs"' % rev)
-
-
-def UpdateBisection(revisions, git_hashes, bisect_contents, last_tested,
-                    update_packages, chroot_path, patch_metadata_file,
-                    extra_change_lists, options, builder, verbose):
+def Bisect(revisions, git_hashes, bisect_state, last_tested, update_packages,
+           chroot_path, patch_metadata_file, extra_change_lists, options,
+           builder, verbose):
   """Adds tryjobs and updates the status file with the new tryjobs."""
 
   try:
     for svn_revision, git_hash in zip(revisions, git_hashes):
-      tryjob_dict = modify_a_tryjob.AddTryjob(
-          update_packages, git_hash, svn_revision, chroot_path,
-          patch_metadata_file, extra_change_lists, options, builder, verbose,
-          svn_revision)
+      tryjob_dict = modify_a_tryjob.AddTryjob(update_packages, git_hash,
+                                              svn_revision, chroot_path,
+                                              patch_metadata_file,
+                                              extra_change_lists, options,
+                                              builder, verbose, svn_revision)
 
-      bisect_contents['jobs'].append(tryjob_dict)
+      bisect_state['jobs'].append(tryjob_dict)
   finally:
     # Do not want to lose progress if there is an exception.
     if last_tested:
       new_file = '%s.new' % last_tested
       with open(new_file, 'w') as json_file:
-        json.dump(bisect_contents, json_file, indent=4, separators=(',', ': '))
+        json.dump(bisect_state, json_file, indent=4, separators=(',', ': '))
 
       os.rename(new_file, last_tested)
 
 
-def _NoteCompletedBisection(last_tested, src_path, end):
-  """Prints that bisection is complete."""
-
-  print('Finished bisecting for %s' % last_tested)
-
-  if src_path:
-    bad_llvm_hash = get_llvm_hash.GetGitHashFrom(src_path, end)
-  else:
-    bad_llvm_hash = get_llvm_hash.LLVMHash().GetLLVMHash(end)
-
-  print(
-      'The bad revision is %d and its commit hash is %s' % (end, bad_llvm_hash))
-
-
 def LoadStatusFile(last_tested, start, end):
   """Loads the status file for bisection."""
 
@@ -383,37 +280,36 @@ def LoadStatusFile(last_tested, start, end):
 
 
 def main(args_output):
-  """Bisects LLVM based off of a .JSON file.
+  """Bisects LLVM commits.
 
   Raises:
     AssertionError: The script was run inside the chroot.
   """
 
   chroot.VerifyOutsideChroot()
-
   update_packages = [
       'sys-devel/llvm', 'sys-libs/compiler-rt', 'sys-libs/libcxx',
       'sys-libs/libcxxabi', 'sys-libs/llvm-libunwind'
   ]
-
   patch_metadata_file = 'PATCHES.json'
-
   start = args_output.start_rev
   end = args_output.end_rev
 
-  bisect_contents = LoadStatusFile(args_output.last_tested, start, end)
-
-  _ValidateStartAndEndAgainstJSONStartAndEnd(
-      start, end, bisect_contents['start'], bisect_contents['end'])
+  bisect_state = LoadStatusFile(args_output.last_tested, start, end)
+  if start != bisect_state['start'] or end != bisect_state['end']:
+    raise ValueError(f'The start {start} or the end {end} version provided is '
+                     f'different than "start" {bisect_state["start"]} or "end" '
+                     f'{bisect_state["end"]} in the .JSON file')
 
   # Pending and skipped revisions are between 'start_revision' and
   # 'end_revision'.
   start_revision, end_revision, pending_revisions, skip_revisions = \
-      GetStartAndEndRevision(start, end, bisect_contents['jobs'])
+      GetRemainingRange(start, end, bisect_state['jobs'])
 
-  revisions, git_hashes = GetRevisionsListAndHashList(
-      start_revision, end_revision, args_output.parallel, args_output.src_path,
-      pending_revisions, skip_revisions)
+  revisions, git_hashes = GetCommitsBetween(start_revision, end_revision,
+                                            args_output.parallel,
+                                            args_output.src_path,
+                                            pending_revisions, skip_revisions)
 
   # No more revisions between 'start_revision' and 'end_revision', so
   # bisection is complete.
@@ -421,39 +317,48 @@ def main(args_output):
   # This is determined by finding all valid revisions between 'start_revision'
   # and 'end_revision' and that are NOT in the 'pending' and 'skipped' set.
   if not revisions:
-    # Successfully completed bisection where there are 2 cases:
-    # 1) 'start_revision' and 'end_revision' are back-to-back (example:
-    # 'start_revision' is 369410 and 'end_revision' is 369411).
-    #
-    # 2) 'start_revision' and 'end_revision' are NOT back-to-back, so there must
-    # be tryjobs in between which are labeled as 'skip' for their 'status'
-    # value.
-    #
-    # In either case, there are no 'pending' jobs.
-    if not pending_revisions:
-      _NoteCompletedBisection(args_output.last_tested, args_output.src_path,
-                              end_revision)
-
-      if skip_revisions:
-        skip_revisions_message = ('\nThe following revisions were skipped:\n' +
-                                  '\n'.join(str(rev) for rev in skip_revisions))
-
-        print(skip_revisions_message)
-
-      return BisectionExitStatus.BISECTION_COMPLETE.value
+    if pending_revisions:
+      # Some tryjobs are not finished which may change the actual bad
+      # commit/revision when those tryjobs are finished.
+      no_revisions_message = (f'No revisions between start {start_revision} '
+                              f'and end {end_revision} to create tryjobs\n')
 
-    # Some tryjobs are not finished which may change the actual bad
-    # commit/revision when those tryjobs are finished.
-    DieWithNoRevisionsError(start_revision, end_revision, skip_revisions,
-                            pending_revisions)
+      if pending_revisions:
+        no_revisions_message += (
+            'The following tryjobs are pending:\n' +
+            '\n'.join(str(rev) for rev in pending_revisions) + '\n')
 
-  CheckForExistingTryjobsInRevisionsToLaunch(revisions, bisect_contents['jobs'])
+      if skip_revisions:
+        no_revisions_message += ('The following tryjobs were skipped:\n' +
+                                 '\n'.join(str(rev) for rev in skip_revisions) +
+                                 '\n')
+
+      raise ValueError(no_revisions_message)
+
+    print(f'Finished bisecting for {args_output.last_tested}')
+    if args_output.src_path:
+      bad_llvm_hash = get_llvm_hash.GetGitHashFrom(args_output.src_path,
+                                                   end_revision)
+    else:
+      bad_llvm_hash = get_llvm_hash.LLVMHash().GetLLVMHash(end_revision)
+    print(f'The bad revision is {end_revision} and its commit hash is '
+          f'{bad_llvm_hash}')
+    if skip_revisions:
+      skip_revisions_message = ('\nThe following revisions were skipped:\n' +
+                                '\n'.join(str(rev) for rev in skip_revisions))
+      print(skip_revisions_message)
+
+    return BisectionExitStatus.BISECTION_COMPLETE.value
 
-  UpdateBisection(revisions, git_hashes, bisect_contents,
-                  args_output.last_tested, update_packages,
-                  args_output.chroot_path, patch_metadata_file,
-                  args_output.extra_change_lists, args_output.options,
-                  args_output.builder, args_output.verbose)
+  for rev in revisions:
+    if update_tryjob_status.FindTryjobIndex(rev,
+                                            bisect_state['jobs']) is not None:
+      raise ValueError(f'Revision {rev} exists already in "jobs"')
+
+  Bisect(revisions, git_hashes, bisect_state, args_output.last_tested,
+         update_packages, args_output.chroot_path, patch_metadata_file,
+         args_output.extra_change_lists, args_output.options,
+         args_output.builder, args_output.verbose)
 
 
 if __name__ == '__main__':
diff --git a/llvm_tools/llvm_bisection_unittest.py b/llvm_tools/llvm_bisection_unittest.py
index e730293b..8478f82e 100755
--- a/llvm_tools/llvm_bisection_unittest.py
+++ b/llvm_tools/llvm_bisection_unittest.py
@@ -16,45 +16,50 @@ import unittest.mock as mock
 
 import chroot
 import get_llvm_hash
+import git_llvm_rev
 import llvm_bisection
 import modify_a_tryjob
 import test_helpers
-import update_tryjob_status
 
 
 class LLVMBisectionTest(unittest.TestCase):
   """Unittests for LLVM bisection."""
 
-  def testStartAndEndDoNotMatchJsonStartAndEnd(self):
+  def testGetRemainingRangePassed(self):
     start = 100
     end = 150
 
-    json_start = 110
-    json_end = 150
-
-    # Verify the exception is raised when the start and end revision for LLVM
-    # bisection do not match the .JSON's 'start' and 'end' values.
-    with self.assertRaises(ValueError) as err:
-      llvm_bisection._ValidateStartAndEndAgainstJSONStartAndEnd(
-          start, end, json_start, json_end)
-
-    expected_error_message = ('The start %d or the end %d version provided is '
-                              'different than "start" %d or "end" %d in the '
-                              '.JSON file' % (start, end, json_start, json_end))
-
-    self.assertEqual(str(err.exception), expected_error_message)
-
-  def testStartAndEndMatchJsonStartAndEnd(self):
-    start = 100
-    end = 150
+    test_tryjobs = [{
+        'rev': 110,
+        'status': 'good',
+        'link': 'https://some_tryjob_1_url.com'
+    }, {
+        'rev': 120,
+        'status': 'good',
+        'link': 'https://some_tryjob_2_url.com'
+    }, {
+        'rev': 130,
+        'status': 'pending',
+        'link': 'https://some_tryjob_3_url.com'
+    }, {
+        'rev': 135,
+        'status': 'skip',
+        'link': 'https://some_tryjob_4_url.com'
+    }, {
+        'rev': 140,
+        'status': 'bad',
+        'link': 'https://some_tryjob_5_url.com'
+    }]
 
-    json_start = 100
-    json_end = 150
+    # Tuple consists of the new good revision, the new bad revision, a set of
+    # 'pending' revisions, and a set of 'skip' revisions.
+    expected_revisions_tuple = 120, 140, {130}, {135}
 
-    llvm_bisection._ValidateStartAndEndAgainstJSONStartAndEnd(
-        start, end, json_start, json_end)
+    self.assertEqual(
+        llvm_bisection.GetRemainingRange(start, end, test_tryjobs),
+        expected_revisions_tuple)
 
-  def testTryjobStatusIsMissing(self):
+  def testGetRemainingRangeFailedWithMissingStatus(self):
     start = 100
     end = 150
 
@@ -72,18 +77,14 @@ class LLVMBisectionTest(unittest.TestCase):
         'link': 'https://some_tryjob_3_url.com'
     }]
 
-    # Verify the exception is raised when a tryjob does not have a value for
-    # the 'status' key or the 'status' key is missing.
     with self.assertRaises(ValueError) as err:
-      llvm_bisection.GetStartAndEndRevision(start, end, test_tryjobs)
-
-    expected_error_message = (
-        '"status" is missing or has no value, please '
-        'go to %s and update it' % test_tryjobs[1]['link'])
+      llvm_bisection.GetRemainingRange(start, end, test_tryjobs)
 
-    self.assertEqual(str(err.exception), expected_error_message)
+    error_message = ('"status" is missing or has no value, please '
+                     'go to %s and update it' % test_tryjobs[1]['link'])
+    self.assertEqual(str(err.exception), error_message)
 
-  def testGoodRevisionGreaterThanBadRevision(self):
+  def testGetRemainingRangeFailedWithInvalidRange(self):
     start = 100
     end = 150
 
@@ -101,206 +102,68 @@ class LLVMBisectionTest(unittest.TestCase):
         'link': 'https://some_tryjob_3_url.com'
     }]
 
-    # Verify the exception is raised when the new 'start' revision is greater
-    # than the new 'bad' revision for bisection (i.e. bisection is broken).
     with self.assertRaises(AssertionError) as err:
-      llvm_bisection.GetStartAndEndRevision(start, end, test_tryjobs)
+      llvm_bisection.GetRemainingRange(start, end, test_tryjobs)
 
-    expected_error_message = (
-        'Bisection is broken because %d (good) is >= '
-        '%d (bad)' % (test_tryjobs[2]['rev'], test_tryjobs[0]['rev']))
+    expected_error_message = ('Bisection is broken because %d (good) is >= '
+                              '%d (bad)' %
+                              (test_tryjobs[2]['rev'], test_tryjobs[0]['rev']))
 
     self.assertEqual(str(err.exception), expected_error_message)
 
-  def testSuccessfullyGetNewStartAndNewEndRevision(self):
-    start = 100
-    end = 150
-
-    test_tryjobs = [{
-        'rev': 110,
-        'status': 'good',
-        'link': 'https://some_tryjob_1_url.com'
-    }, {
-        'rev': 120,
-        'status': 'good',
-        'link': 'https://some_tryjob_2_url.com'
-    }, {
-        'rev': 130,
-        'status': 'pending',
-        'link': 'https://some_tryjob_3_url.com'
-    }, {
-        'rev': 135,
-        'status': 'skip',
-        'link': 'https://some_tryjob_4_url.com'
-    }, {
-        'rev': 140,
-        'status': 'bad',
-        'link': 'https://some_tryjob_5_url.com'
-    }]
-
-    # Tuple consists of the new good revision, the new bad revision, a set of
-    # 'pending' revisions, and a set of 'skip' revisions.
-    expected_revisions_tuple = 120, 140, {130}, {135}
-
-    self.assertTupleEqual(
-        llvm_bisection.GetStartAndEndRevision(start, end, test_tryjobs),
-        expected_revisions_tuple)
-
-  @mock.patch.object(get_llvm_hash, 'GetGitHashFrom')
-  def testNoRevisionsBetweenStartAndEnd(self, mock_get_git_hash):
-    start = 100
-    end = 110
-
-    test_pending_revisions = {107}
-    test_skip_revisions = {101, 102, 103, 104, 108, 109}
-
-    # Simulate behavior of `GetGitHashFrom()` when the revision does not
-    # exist in the LLVM source tree.
-    def MockGetGitHashForRevisionRaiseException(_src_path, _revision):
-      raise ValueError('Revision does not exist')
-
-    mock_get_git_hash.side_effect = MockGetGitHashForRevisionRaiseException
-
-    parallel = 3
-
-    abs_path_to_src = '/abs/path/to/src'
-
-    self.assertListEqual(
-        llvm_bisection.GetRevisionsBetweenBisection(
-            start, end, parallel, abs_path_to_src, test_pending_revisions,
-            test_skip_revisions), [])
-
-  # Assume llvm_bisection module has imported GetGitHashFrom
   @mock.patch.object(get_llvm_hash, 'GetGitHashFrom')
-  def testSuccessfullyRetrievedRevisionsBetweenStartAndEnd(
-      self, mock_get_git_hash):
-
-    start = 100
-    end = 110
-
-    test_pending_revisions = set()
-    test_skip_revisions = {101, 102, 103, 104, 106, 108, 109}
-
+  def testGetCommitsBetweenPassed(self, mock_get_git_hash):
+    start = git_llvm_rev.base_llvm_revision
+    end = start + 10
+    test_pending_revisions = {start + 7}
+    test_skip_revisions = {
+        start + 1, start + 2, start + 4, start + 8, start + 9
+    }
     parallel = 3
-
     abs_path_to_src = '/abs/path/to/src'
 
-    # Valid revision that exist in the LLVM source tree between 'start' and
-    # 'end' and were not in the 'pending' set or 'skip' set.
-    expected_revisions_between_start_and_end = [105, 107]
-
-    self.assertListEqual(
-        llvm_bisection.GetRevisionsBetweenBisection(
-            start, end, parallel, abs_path_to_src, test_pending_revisions,
-            test_skip_revisions), expected_revisions_between_start_and_end)
-
-    self.assertEqual(mock_get_git_hash.call_count, 2)
+    revs = ['a123testhash3', 'a123testhash5']
+    mock_get_git_hash.side_effect = revs
 
-  # Simulate behavior of `GetGitHashFrom()` when successfully retrieved
-  # a list git hashes for each revision in the revisions list.
-  # Assume llvm_bisection module has imported GetGitHashFrom
-  @mock.patch.object(get_llvm_hash, 'GetGitHashFrom')
-  # Simulate behavior of `GetRevisionsBetweenBisection()` when successfully
-  # retrieved a list of valid revisions between 'start' and 'end'.
-  @mock.patch.object(llvm_bisection, 'GetRevisionsBetweenBisection')
-  # Simulate behavior of `CreatTempLLVMRepo()` when successfully created a
-  # worktree when a source path was not provided.
-  @mock.patch.object(get_llvm_hash, 'CreateTempLLVMRepo')
-  def testSuccessfullyGetRevisionsListAndHashList(
-      self, mock_create_temp_llvm_repo, mock_get_revisions_between_bisection,
-      mock_get_git_hash):
-
-    expected_revisions_and_hash_tuple = ([102, 105, 108], [
-        'a123testhash1', 'a123testhash2', 'a123testhash3'
-    ])
-
-    @test_helpers.CallCountsToMockFunctions
-    def MockGetGitHashForRevision(call_count, _src_path, _rev):
-      # Simulate retrieving the git hash for the revision.
-      if call_count < 3:
-        return expected_revisions_and_hash_tuple[1][call_count]
-
-      assert False, 'Called `GetGitHashFrom()` more than expected.'
-
-    temp_worktree = '/abs/path/to/tmpDir'
-
-    mock_create_temp_llvm_repo.return_value.__enter__.return_value.name = \
-        temp_worktree
-
-    # Simulate the valid revisions list.
-    mock_get_revisions_between_bisection.return_value = \
-        expected_revisions_and_hash_tuple[0]
-
-    # Simulate behavior of `GetGitHashFrom()` by using the testing
-    # function.
-    mock_get_git_hash.side_effect = MockGetGitHashForRevision
-
-    start = 100
-    end = 110
-    parallel = 3
-    src_path = None
-    pending_revisions = {103, 104}
-    skip_revisions = {101, 106, 107, 109}
+    git_hashes = [
+        git_llvm_rev.base_llvm_revision + 3, git_llvm_rev.base_llvm_revision + 5
+    ]
 
-    self.assertTupleEqual(
-        llvm_bisection.GetRevisionsListAndHashList(
-            start, end, parallel, src_path, pending_revisions, skip_revisions),
-        expected_revisions_and_hash_tuple)
-
-    mock_get_revisions_between_bisection.assert_called_once()
-
-    self.assertEqual(mock_get_git_hash.call_count, 3)
+    self.assertEqual(
+        llvm_bisection.GetCommitsBetween(start, end, parallel, abs_path_to_src,
+                                         test_pending_revisions,
+                                         test_skip_revisions),
+        (git_hashes, revs))
 
-  def testSuccessfullyDieWithNoRevisionsError(self):
+  def testLoadStatusFilePassedWithExistingFile(self):
     start = 100
-    end = 110
-
-    pending_revisions = {105, 108}
-    skip_revisions = {101, 102, 103, 104, 106, 107, 109}
-
-    expected_no_revisions_message = ('No revisions between start %d and end '
-                                     '%d to create tryjobs' % (start, end))
-
-    expected_no_revisions_message += '\nThe following tryjobs are pending:\n' \
-        + '\n'.join(str(rev) for rev in pending_revisions)
-
-    expected_no_revisions_message += '\nThe following tryjobs were skipped:\n' \
-        + '\n'.join(str(rev) for rev in skip_revisions)
-
-    # Verify that an exception is raised when there are no revisions to launch
-    # tryjobs for between 'start' and 'end' and some tryjobs are 'pending'.
-    with self.assertRaises(ValueError) as err:
-      llvm_bisection.DieWithNoRevisionsError(start, end, skip_revisions,
-                                             pending_revisions)
+    end = 150
 
-    self.assertEqual(str(err.exception), expected_no_revisions_message)
+    test_bisect_state = {'start': start, 'end': end, 'jobs': []}
 
-  # Simulate behavior of `FindTryjobIndex()` when the index of the tryjob was
-  # found.
-  @mock.patch.object(update_tryjob_status, 'FindTryjobIndex', return_value=0)
-  def testTryjobExistsInRevisionsToLaunch(self, mock_find_tryjob_index):
-    test_existing_jobs = [{'rev': 102, 'status': 'good'}]
+    # Simulate that the status file exists.
+    with test_helpers.CreateTemporaryJsonFile() as temp_json_file:
+      with open(temp_json_file, 'w') as f:
+        test_helpers.WritePrettyJsonFile(test_bisect_state, f)
 
-    revision_to_launch = [102]
+      self.assertEqual(
+          llvm_bisection.LoadStatusFile(temp_json_file, start, end),
+          test_bisect_state)
 
-    expected_revision_that_exists = 102
+  def testLoadStatusFilePassedWithoutExistingFile(self):
+    start = 200
+    end = 250
 
-    with self.assertRaises(ValueError) as err:
-      llvm_bisection.CheckForExistingTryjobsInRevisionsToLaunch(
-          revision_to_launch, test_existing_jobs)
+    expected_bisect_state = {'start': start, 'end': end, 'jobs': []}
 
-    expected_found_tryjob_index_error_message = (
-        'Revision %d exists already '
-        'in "jobs"' % expected_revision_that_exists)
+    last_tested = '/abs/path/to/file_that_does_not_exist.json'
 
     self.assertEqual(
-        str(err.exception), expected_found_tryjob_index_error_message)
-
-    mock_find_tryjob_index.assert_called_once()
+        llvm_bisection.LoadStatusFile(last_tested, start, end),
+        expected_bisect_state)
 
   @mock.patch.object(modify_a_tryjob, 'AddTryjob')
-  def testSuccessfullyUpdatedStatusFileWhenExceptionIsRaised(
-      self, mock_add_tryjob):
+  def testBisectPassed(self, mock_add_tryjob):
 
     git_hash_list = ['a123testhash1', 'a123testhash2', 'a123testhash3']
     revisions_list = [102, 104, 106]
@@ -343,11 +206,11 @@ class LLVMBisectionTest(unittest.TestCase):
       # Verify that the status file is updated when an exception happened when
       # attempting to launch a revision (i.e. progress is not lost).
       with self.assertRaises(ValueError) as err:
-        llvm_bisection.UpdateBisection(
-            revisions_list, git_hash_list, bisection_contents, temp_json_file,
-            packages, args_output.chroot_path, patch_file,
-            args_output.extra_change_lists, args_output.options,
-            args_output.builders, args_output.verbose)
+        llvm_bisection.Bisect(revisions_list, git_hash_list, bisection_contents,
+                              temp_json_file, packages, args_output.chroot_path,
+                              patch_file, args_output.extra_change_lists,
+                              args_output.options, args_output.builders,
+                              args_output.verbose)
 
       expected_bisection_contents = {
           'start':
@@ -368,121 +231,128 @@ class LLVMBisectionTest(unittest.TestCase):
       with open(temp_json_file) as f:
         json_contents = json.load(f)
 
-        self.assertDictEqual(json_contents, expected_bisection_contents)
+        self.assertEqual(json_contents, expected_bisection_contents)
 
     self.assertEqual(str(err.exception), 'Unable to launch tryjob')
 
     self.assertEqual(mock_add_tryjob.call_count, 3)
 
-  # Simulate behavior of `GetGitHashFrom()` when successfully retrieved
-  # the git hash of the bad revision. Assume llvm_bisection has imported
-  # GetGitHashFrom
   @mock.patch.object(
-      get_llvm_hash, 'GetGitHashFrom', return_value='a123testhash4')
-  def testCompletedBisectionWhenProvidedSrcPath(self, mock_get_git_hash):
-    last_tested = '/some/last/tested_file.json'
+      get_llvm_hash.LLVMHash, 'GetLLVMHash', return_value='a123testhash4')
+  @mock.patch.object(llvm_bisection, 'GetCommitsBetween')
+  @mock.patch.object(llvm_bisection, 'GetRemainingRange')
+  @mock.patch.object(llvm_bisection, 'LoadStatusFile')
+  @mock.patch.object(chroot, 'VerifyOutsideChroot', return_value=True)
+  def testMainPassed(self, mock_outside_chroot, mock_load_status_file,
+                     mock_get_range, mock_get_revision_and_hash_list,
+                     _mock_get_bad_llvm_hash):
 
-    src_path = '/abs/path/to/src/path'
+    start = 500
+    end = 502
+    cl = 1
 
-    # The bad revision.
-    end = 150
+    bisect_state = {
+        'start': start,
+        'end': end,
+        'jobs': [{
+            'rev': 501,
+            'status': 'bad',
+            'cl': cl
+        }]
+    }
 
-    llvm_bisection._NoteCompletedBisection(last_tested, src_path, end)
+    skip_revisions = {501}
+    pending_revisions = {}
 
-    mock_get_git_hash.assert_called_once()
+    mock_load_status_file.return_value = bisect_state
 
-  # Simulate behavior of `GetLLVMHash()` when successfully retrieved
-  # the git hash of the bad revision.
-  @mock.patch.object(
-      get_llvm_hash.LLVMHash, 'GetLLVMHash', return_value='a123testhash5')
-  def testCompletedBisectionWhenNotProvidedSrcPath(self, mock_get_git_hash):
-    last_tested = '/some/last/tested_file.json'
+    mock_get_range.return_value = (start, end, pending_revisions,
+                                   skip_revisions)
 
-    src_path = None
+    mock_get_revision_and_hash_list.return_value = [], []
 
-    # The bad revision.
-    end = 200
+    args_output = test_helpers.ArgsOutputTest()
+    args_output.start_rev = start
+    args_output.end_rev = end
+    args_output.parallel = 3
+    args_output.src_path = None
+    args_output.chroot_path = 'somepath'
 
-    llvm_bisection._NoteCompletedBisection(last_tested, src_path, end)
+    self.assertEqual(
+        llvm_bisection.main(args_output),
+        llvm_bisection.BisectionExitStatus.BISECTION_COMPLETE.value)
 
-    mock_get_git_hash.assert_called_once()
+    mock_outside_chroot.assert_called_once()
 
-  def testSuccessfullyLoadedStatusFile(self):
-    start = 100
-    end = 150
+    mock_load_status_file.assert_called_once()
 
-    test_bisect_contents = {'start': start, 'end': end, 'jobs': []}
+    mock_get_range.assert_called_once()
 
-    # Simulate that the status file exists.
-    with test_helpers.CreateTemporaryJsonFile() as temp_json_file:
-      with open(temp_json_file, 'w') as f:
-        test_helpers.WritePrettyJsonFile(test_bisect_contents, f)
+    mock_get_revision_and_hash_list.assert_called_once()
 
-      self.assertDictEqual(
-          llvm_bisection.LoadStatusFile(temp_json_file, start, end),
-          test_bisect_contents)
+  @mock.patch.object(llvm_bisection, 'LoadStatusFile')
+  @mock.patch.object(chroot, 'VerifyOutsideChroot', return_value=True)
+  def testMainFailedWithInvalidRange(self, mock_outside_chroot,
+                                     mock_load_status_file):
 
-  def testLoadedStatusFileThatDoesNotExist(self):
-    start = 200
-    end = 250
+    start = 500
+    end = 502
 
-    expected_bisect_contents = {'start': start, 'end': end, 'jobs': []}
+    bisect_state = {
+        'start': start - 1,
+        'end': end,
+    }
 
-    last_tested = '/abs/path/to/file_that_does_not_exist.json'
+    mock_load_status_file.return_value = bisect_state
 
-    self.assertDictEqual(
-        llvm_bisection.LoadStatusFile(last_tested, start, end),
-        expected_bisect_contents)
-
-  # Simulate behavior of `_NoteCompletedBisection()` when there are no more
-  # tryjobs to launch between start and end, so bisection is complete.
-  @mock.patch.object(llvm_bisection, '_NoteCompletedBisection')
-  @mock.patch.object(llvm_bisection, 'GetRevisionsListAndHashList')
-  @mock.patch.object(llvm_bisection, 'GetStartAndEndRevision')
-  # Simulate behavior of `_ValidateStartAndEndAgainstJSONStartAndEnd()` when
-  # both start and end revisions match.
-  @mock.patch.object(llvm_bisection,
-                     '_ValidateStartAndEndAgainstJSONStartAndEnd')
+    args_output = test_helpers.ArgsOutputTest()
+    args_output.start_rev = start
+    args_output.end_rev = end
+    args_output.parallel = 3
+    args_output.src_path = None
+
+    with self.assertRaises(ValueError) as err:
+      llvm_bisection.main(args_output)
+
+    error_message = (f'The start {start} or the end {end} version provided is '
+                     f'different than "start" {bisect_state["start"]} or "end" '
+                     f'{bisect_state["end"]} in the .JSON file')
+
+    self.assertEqual(str(err.exception), error_message)
+
+    mock_outside_chroot.assert_called_once()
+
+    mock_load_status_file.assert_called_once()
+
+  @mock.patch.object(llvm_bisection, 'GetCommitsBetween')
+  @mock.patch.object(llvm_bisection, 'GetRemainingRange')
   @mock.patch.object(llvm_bisection, 'LoadStatusFile')
-  # Simulate behavior of `VerifyOutsideChroot()` when successfully invoked the
-  # script outside of the chroot.
   @mock.patch.object(chroot, 'VerifyOutsideChroot', return_value=True)
-  def testSuccessfullyBisectedLLVM(
-      self, mock_outside_chroot, mock_load_status_file,
-      mock_validate_start_and_end, mock_get_start_and_end_revision,
-      mock_get_revision_and_hash_list, mock_note_completed_bisection):
+  def testMainFailedWithPendingBuilds(self, mock_outside_chroot,
+                                      mock_load_status_file, mock_get_range,
+                                      mock_get_revision_and_hash_list):
 
     start = 500
     end = 502
+    rev = 501
 
-    bisect_contents = {
+    bisect_state = {
         'start': start,
         'end': end,
         'jobs': [{
-            'rev': 501,
-            'status': 'skip'
+            'rev': rev,
+            'status': 'pending'
         }]
     }
 
-    skip_revisions = {501}
-    pending_revisions = {}
+    skip_revisions = {}
+    pending_revisions = {rev}
+
+    mock_load_status_file.return_value = bisect_state
+
+    mock_get_range.return_value = (start, end, pending_revisions,
+                                   skip_revisions)
 
-    # Simulate behavior of `LoadStatusFile()` when successfully loaded the
-    # status file.
-    mock_load_status_file.return_value = bisect_contents
-
-    # Simulate behavior of `GetStartAndEndRevision()` when successfully found
-    # the new start and end revision of the bisection.
-    #
-    # Returns new start revision, new end revision, a set of pending revisions,
-    # and a set of skip revisions.
-    mock_get_start_and_end_revision.return_value = (start, end,
-                                                    pending_revisions,
-                                                    skip_revisions)
-
-    # Simulate behavior of `GetRevisionsListAndHashList()` when successfully
-    # retrieved valid revisions (along with their git hashes) between start and
-    # end (in this case, none).
     mock_get_revision_and_hash_list.return_value = [], []
 
     args_output = test_helpers.ArgsOutputTest()
@@ -491,111 +361,75 @@ class LLVMBisectionTest(unittest.TestCase):
     args_output.parallel = 3
     args_output.src_path = None
 
-    self.assertEqual(
-        llvm_bisection.main(args_output),
-        llvm_bisection.BisectionExitStatus.BISECTION_COMPLETE.value)
+    with self.assertRaises(ValueError) as err:
+      llvm_bisection.main(args_output)
+
+    error_message = (f'No revisions between start {start} and end {end} to '
+                     'create tryjobs\nThe following tryjobs are pending:\n'
+                     f'{rev}\n')
+
+    self.assertEqual(str(err.exception), error_message)
 
     mock_outside_chroot.assert_called_once()
 
     mock_load_status_file.assert_called_once()
 
-    mock_validate_start_and_end.assert_called_once()
-
-    mock_get_start_and_end_revision.assert_called_once()
+    mock_get_range.assert_called_once()
 
     mock_get_revision_and_hash_list.assert_called_once()
 
-    mock_note_completed_bisection.assert_called_once()
-
-  @mock.patch.object(llvm_bisection, 'DieWithNoRevisionsError')
-  # Simulate behavior of `_NoteCompletedBisection()` when there are no more
-  # tryjobs to launch between start and end, so bisection is complete.
-  @mock.patch.object(llvm_bisection, 'GetRevisionsListAndHashList')
-  @mock.patch.object(llvm_bisection, 'GetStartAndEndRevision')
-  # Simulate behavior of `_ValidateStartAndEndAgainstJSONStartAndEnd()` when
-  # both start and end revisions match.
-  @mock.patch.object(llvm_bisection,
-                     '_ValidateStartAndEndAgainstJSONStartAndEnd')
+  @mock.patch.object(llvm_bisection, 'GetCommitsBetween')
+  @mock.patch.object(llvm_bisection, 'GetRemainingRange')
   @mock.patch.object(llvm_bisection, 'LoadStatusFile')
-  # Simulate behavior of `VerifyOutsideChroot()` when successfully invoked the
-  # script outside of the chroot.
   @mock.patch.object(chroot, 'VerifyOutsideChroot', return_value=True)
-  def testNoMoreTryjobsToLaunch(
-      self, mock_outside_chroot, mock_load_status_file,
-      mock_validate_start_and_end, mock_get_start_and_end_revision,
-      mock_get_revision_and_hash_list, mock_die_with_no_revisions_error):
+  def testMainFailedWithDuplicateBuilds(self, mock_outside_chroot,
+                                        mock_load_status_file, mock_get_range,
+                                        mock_get_revision_and_hash_list):
 
     start = 500
     end = 502
+    rev = 501
+    git_hash = 'a123testhash1'
 
-    bisect_contents = {
+    bisect_state = {
         'start': start,
         'end': end,
         'jobs': [{
-            'rev': 501,
+            'rev': rev,
             'status': 'pending'
         }]
     }
 
     skip_revisions = {}
-    pending_revisions = {501}
-
-    no_revisions_error_message = ('No more tryjobs to launch between %d and '
-                                  '%d' % (start, end))
-
-    def MockNoRevisionsErrorException(_start, _end, _skip, _pending):
-      raise ValueError(no_revisions_error_message)
-
-    # Simulate behavior of `LoadStatusFile()` when successfully loaded the
-    # status file.
-    mock_load_status_file.return_value = bisect_contents
-
-    # Simulate behavior of `GetStartAndEndRevision()` when successfully found
-    # the new start and end revision of the bisection.
-    #
-    # Returns new start revision, new end revision, a set of pending revisions,
-    # and a set of skip revisions.
-    mock_get_start_and_end_revision.return_value = (start, end,
-                                                    pending_revisions,
-                                                    skip_revisions)
-
-    # Simulate behavior of `GetRevisionsListAndHashList()` when successfully
-    # retrieved valid revisions (along with their git hashes) between start and
-    # end (in this case, none).
-    mock_get_revision_and_hash_list.return_value = [], []
+    pending_revisions = {rev}
+
+    mock_load_status_file.return_value = bisect_state
 
-    # Use the test function to simulate `DieWithNoRevisionsWithError()`
-    # behavior.
-    mock_die_with_no_revisions_error.side_effect = MockNoRevisionsErrorException
+    mock_get_range.return_value = (start, end, pending_revisions,
+                                   skip_revisions)
+
+    mock_get_revision_and_hash_list.return_value = [rev], [git_hash]
 
-    # Simulate behavior of arguments passed into the command line.
     args_output = test_helpers.ArgsOutputTest()
     args_output.start_rev = start
     args_output.end_rev = end
     args_output.parallel = 3
     args_output.src_path = None
 
-    # Verify the exception is raised when there are no more tryjobs to launch
-    # between start and end when there are tryjobs that are 'pending', so
-    # the actual bad revision can change when those tryjobs's 'status' are
-    # updated.
     with self.assertRaises(ValueError) as err:
       llvm_bisection.main(args_output)
 
-    self.assertEqual(str(err.exception), no_revisions_error_message)
+    error_message = ('Revision %d exists already in "jobs"' % rev)
+    self.assertEqual(str(err.exception), error_message)
 
     mock_outside_chroot.assert_called_once()
 
     mock_load_status_file.assert_called_once()
 
-    mock_validate_start_and_end.assert_called_once()
-
-    mock_get_start_and_end_revision.assert_called_once()
+    mock_get_range.assert_called_once()
 
     mock_get_revision_and_hash_list.assert_called_once()
 
-    mock_die_with_no_revisions_error.assert_called_once()
-
 
 if __name__ == '__main__':
   unittest.main()