Fix style in tools/auto_bisect.

tools/auto_bisect/bisect_perf_regression.py

 #!/usr/bin/env python
 # Copyright 2013 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.
 
 """Chromium auto-bisect tool
 
 This script bisects a range of commits using binary search. It starts by getting
 reference values for the specified "good" and "bad" commits. Then, for revisions
 in between, it will get builds, run tests and classify intermediate revisions as
@@ skipping 110 matching lines @@
 # Git master branch name.
 BISECT_MASTER_BRANCH = 'master'
 # File to store 'git diff' content.
 BISECT_PATCH_FILE = 'deps_patch.txt'
 # SVN repo where the bisect try jobs are submitted.
 PERF_SVN_REPO_URL = 'svn://svn.chromium.org/chrome-try/try-perf'
 FULL_SVN_REPO_URL = 'svn://svn.chromium.org/chrome-try/try'
 ANDROID_CHROME_SVN_REPO_URL = ('svn://svn.chromium.org/chrome-try-internal/'
                                'try-perf')
 
+
 class RunGitError(Exception):
 
   def __str__(self):
     return '%s\nError executing git command.' % self.args[0]
 
 
 def GetSHA1HexDigest(contents):
   """Returns SHA1 hex digest of the given string."""
   return hashlib.sha1(contents).hexdigest()
 
@@ skipping 313 matching lines @@
           current_arg_split = current_arg.split('=')
 
           # Check 2 cases, --arg=<val> and --arg <val>
           if len(current_arg_split) == 2:
             arg_dict[arg_to_parse] = current_arg_split[1]
           elif i + 1 < len(command_args):
             arg_dict[arg_to_parse] = command_args[i+1]
 
     path_to_generate = os.path.join('tools', 'perf', 'generate_profile')
 
-    if arg_dict.has_key('--profile-dir') and arg_dict.has_key('--browser'):
+    if '--profile-dir' in arg_dict and '--browser' in arg_dict:
       profile_path, profile_type = os.path.split(arg_dict['--profile-dir'])
-      return not bisect_utils.RunProcess(['python', path_to_generate,
-          '--profile-type-to-generate', profile_type,
-          '--browser', arg_dict['--browser'], '--output-dir', profile_path])
+      return not bisect_utils.RunProcess(
+          [
+              'python', path_to_generate,
+              '--profile-type-to-generate', profile_type,
+              '--browser', arg_dict['--browser'],
+              '--output-dir', profile_path
+          ])
     return False
   return True
 
 
 def _CheckRegressionConfidenceError(
     good_revision,
     bad_revision,
     known_good_value,
     known_bad_value):
   """Checks whether we can be confident beyond a certain degree that the given
   metrics represent a regression.
 
   Args:
     good_revision: string representing the commit considered 'good'
     bad_revision: Same as above for 'bad'.
     known_good_value: A dict with at least: 'values', 'mean' and 'std_err'
     known_bad_value: Same as above.
 
   Returns:
-    False if there is no error (i.e. we can be confident there's a regressioni),
+    False if there is no error (i.e. we can be confident there's a regression),
     a string containing the details of the lack of confidence otherwise.
   """
   error = False
   # Adding good and bad values to a parameter list.
   confidence_params = []
   for l in [known_bad_value['values'], known_good_value['values']]:
     # Flatten if needed, by averaging the values in each nested list
     if isinstance(l, list) and all([isinstance(x, list) for x in l]):
       averages = map(math_utils.Mean, l)
       confidence_params.append(averages)
@@ skipping 70 matching lines @@
     if returncode:
       raise RunGitError('Deleting branch failed, %s', output)
 
   # Check if the tree is dirty: make sure the index is up to date and then
   # run diff-index.
   bisect_utils.RunGit(['update-index', '--refresh', '-q'])
   output, returncode = bisect_utils.RunGit(['diff-index', 'HEAD'])
   if output:
     raise RunGitError('Cannot send a try job with a dirty tree.')
 
-  # Create/check out the telemetry-tryjob branch, and edit the configs
-  # for the tryjob there.
+  # Create and check out the telemetry-tryjob branch, and edit the configs
+  # for the try job there.
   output, returncode = bisect_utils.RunGit(['checkout', '-b', new_branch])
   if returncode:
     raise RunGitError('Failed to checkout branch: %s.' % output)
 
   output, returncode = bisect_utils.RunGit(
       ['branch', '--set-upstream-to', parent_branch])
   if returncode:
     raise RunGitError('Error in git branch --set-upstream-to')
 
 
 def _StartBuilderTryJob(
     builder_type, git_revision, builder_name, job_name, patch=None):
   """Attempts to run a try job from the current directory.
 
   Args:
     builder_type: One of the builder types in fetch_build, e.g. "perf".
     git_revision: A git commit hash.
     builder_name: Name of the bisect bot to be used for try job.
     bisect_job_name: Try job name, used to identify which bisect
         job was responsible for requesting a build.
     patch: A DEPS patch (used while bisecting dependency repositories),
         or None if we're bisecting the top-level repository.
   """
   # TODO(prasadv, qyearsley): Make this a method of BuildArchive
   # (which may be renamed to BuilderTryBot or Builder).
   try:
-    # Temporary branch for running tryjob.
+    # Temporary branch for running a try job.
     _PrepareBisectBranch(BISECT_MASTER_BRANCH, BISECT_TRYJOB_BRANCH)
     patch_content = '/dev/null'
     # Create a temporary patch file.
     if patch:
       WriteStringToFile(patch, BISECT_PATCH_FILE)
       patch_content = BISECT_PATCH_FILE
 
     try_command = [
         'try',
         '--bot=%s' % builder_name,
         '--revision=%s' % git_revision,
         '--name=%s' % job_name,
         '--svn_repo=%s' % _TryJobSvnRepo(builder_type),
         '--diff=%s' % patch_content,
     ]
     # Execute try job to build revision.
     print try_command
     output, return_code = bisect_utils.RunGit(try_command)
 
     command_string = ' '.join(['git'] + try_command)
     if return_code:
-      raise RunGitError('Could not execute tryjob: %s.\n'
+      raise RunGitError('Could not execute try job: %s.\n'
                         'Error: %s' % (command_string, output))
     logging.info('Try job successfully submitted.\n TryJob Details: %s\n%s',
                  command_string, output)
   finally:
     # Delete patch file if exists.
     try:
       os.remove(BISECT_PATCH_FILE)
     except OSError as e:
       if e.errno != errno.ENOENT:
         raise
@@ skipping 100 matching lines @@
             results[depot_name] = None
       return results
     except ImportError:
       deps_file_contents = ReadStringFromFile(deps_file)
       parse_results = _ParseRevisionsFromDEPSFileManually(deps_file_contents)
       results = {}
       for depot_name, depot_revision in parse_results.iteritems():
         depot_revision = depot_revision.strip('@')
         logging.warn(depot_name, depot_revision)
         for cur_name, cur_data in bisect_utils.DEPOT_DEPS_NAME.iteritems():
-          if (cur_data.has_key('deps_var') and
-              cur_data['deps_var'] == depot_name):
+          if cur_data.get('deps_var') == depot_name:
             src_name = cur_name
             results[src_name] = depot_revision
             break
       return results
 
   def _Get3rdPartyRevisions(self, depot):
     """Parses the DEPS file to determine WebKit/v8/etc... versions.
 
     Args:
       depot: A depot name. Should be in the DEPOT_NAMES list.
@@ skipping 154 matching lines @@
         extra_src=self.opts.extra_src)
 
     try:
       _StartBuilderTryJob(self.opts.builder_type, git_revision, builder_name,
                           job_name=build_request_id, patch=deps_patch)
     except RunGitError as e:
       logging.warn('Failed to post builder try job for revision: [%s].\n'
                    'Error: %s', git_revision, e)
       return None
 
-    # Get the buildbot master url to monitor build status.
+    # Get the buildbot master URL to monitor build status.
     buildbot_server_url = fetch_build.GetBuildBotUrl(
         builder_type=self.opts.builder_type,
         target_arch=self.opts.target_arch,
         target_platform=self.opts.target_platform,
         extra_src=self.opts.extra_src)
 
     archive_filename, error_msg = _WaitUntilBuildIsReady(
         fetch_build_func, builder_name, build_request_id, build_timeout,
         buildbot_server_url)
     if not archive_filename:
@@ skipping 83 matching lines @@
     """Checks if build can be downloaded based on target platform and depot."""
     if (self.opts.target_platform in ['chromium', 'android', 'android-chrome']
         and self.opts.builder_type):
       # In case of android-chrome platform, download archives only for
       # android-chrome depot; for other depots such as chromium, v8, skia
       # etc., build the binary locally.
       if self.opts.target_platform == 'android-chrome':
         return depot == 'android-chrome'
       else:
         return (depot == 'chromium' or
-              'chromium' in bisect_utils.DEPOT_DEPS_NAME[depot]['from'] or
-              'v8' in bisect_utils.DEPOT_DEPS_NAME[depot]['from'])
+                'chromium' in bisect_utils.DEPOT_DEPS_NAME[depot]['from'] or
+                'v8' in bisect_utils.DEPOT_DEPS_NAME[depot]['from'])
     return False
 
   def UpdateDepsContents(self, deps_contents, depot, git_revision, deps_key):
     """Returns modified version of DEPS file contents.
 
     Args:
       deps_contents: DEPS file content.
       depot: Current depot being bisected.
       git_revision: A git hash to be updated in DEPS.
       deps_key: Key in vars section of DEPS file to be searched.
@@ skipping 309 matching lines @@
         if parsed_metric:
           metric_values.append(math_utils.Mean(parsed_metric))
         # If we're bisecting on a metric (ie, changes in the mean or
         # standard deviation) and no metric values are produced, bail out.
         if not metric_values:
           break
       elif self._IsBisectModeReturnCode():
         metric_values.append(return_code)
 
       elapsed_minutes = (time.time() - start_time) / 60.0
-      time_limit = self.opts.max_time_minutes *  test_run_multiplier
+      time_limit = self.opts.max_time_minutes * test_run_multiplier
       if elapsed_minutes >= time_limit:
         break
 
     if metric and len(metric_values) == 0:
       err_text = 'Metric %s was not found in the test output.' % metric
       # TODO(qyearsley): Consider also getting and displaying a list of metrics
       # that were found in the output here.
       return (err_text, failure_code, output_of_all_runs)
 
     # If we're bisecting on return codes, we're really just looking for zero vs
@@ skipping 51 matching lines @@
   def _RunPostSync(self, _depot):
     """Performs any work after syncing.
 
     Args:
       depot: Depot name.
 
     Returns:
       True if successful.
     """
     if 'android' in self.opts.target_platform:
-      if not builder.SetupAndroidBuildEnvironment(self.opts,
-          path_to_src=self.src_cwd):
+      if not builder.SetupAndroidBuildEnvironment(
+          self.opts, path_to_src=self.src_cwd):
         return False
 
     return self.RunGClientHooks()
 
   @staticmethod
   def ShouldSkipRevision(depot, revision):
     """Checks whether a particular revision can be safely skipped.
 
     Some commits can be safely skipped (such as a DEPS roll for the repos
     still using .DEPS.git), since the tool is git based those changes
@@ skipping 84 matching lines @@
 
     # A value other than 0 indicates that the test couldn't be run, and results
     # should also include an error message.
     if results[1] != 0:
       return results
 
     external_revisions = self._Get3rdPartyRevisions(depot)
 
     if not external_revisions is None:
       return (results[0], results[1], external_revisions,
-          time.time() - after_build_time, after_build_time -
-          start_build_time)
+              time.time() - after_build_time, after_build_time -
+              start_build_time)
     else:
       return ('Failed to parse DEPS file for external revisions.',
-               BUILD_RESULT_FAIL)
+              BUILD_RESULT_FAIL)
 
   def _SyncRevision(self, depot, revision, sync_client):
     """Syncs depot to particular revision.
 
     Args:
       depot: The depot that's being used at the moment (src, webkit, etc.)
       revision: The revision to sync to.
       sync_client: Program used to sync, e.g. "gclient". Can be None.
 
     Returns:
       True if successful, False otherwise.
     """
     self.depot_registry.ChangeToDepotDir(depot)
 
     if sync_client:
       self.PerformPreBuildCleanup()
 
     # When using gclient to sync, you need to specify the depot you
     # want so that all the dependencies sync properly as well.
     # i.e. gclient sync src@<SHA1>
     if sync_client == 'gclient' and revision:
       revision = '%s@%s' % (bisect_utils.DEPOT_DEPS_NAME[depot]['src'],
-          revision)
+                            revision)
       if depot == 'chromium' and self.opts.target_platform == 'android-chrome':
         return self._SyncRevisionsForAndroidChrome(revision)
 
     return source_control.SyncToRevision(revision, sync_client)
 
   def _SyncRevisionsForAndroidChrome(self, revision):
     """Syncs android-chrome and chromium repos to particular revision.
 
     This is a special case for android-chrome as the gclient sync for chromium
     overwrites the android-chrome revision to TOT. Therefore both the repos
@@ skipping 21 matching lines @@
       current_value: The value of the metric being checked.
       known_bad_value: The reference value for a "failed" run.
       known_good_value: The reference value for a "passed" run.
 
     Returns:
       True if the current_value is closer to the known_good_value than the
       known_bad_value.
     """
     if self.opts.bisect_mode == bisect_utils.BISECT_MODE_STD_DEV:
       dist_to_good_value = abs(current_value['std_dev'] -
-          known_good_value['std_dev'])
+                               known_good_value['std_dev'])
       dist_to_bad_value = abs(current_value['std_dev'] -
-          known_bad_value['std_dev'])
+                              known_bad_value['std_dev'])
     else:
       dist_to_good_value = abs(current_value['mean'] - known_good_value['mean'])
       dist_to_bad_value = abs(current_value['mean'] - known_bad_value['mean'])
 
     return dist_to_good_value < dist_to_bad_value
 
   def _GetV8BleedingEdgeFromV8TrunkIfMappable(
       self, revision, bleeding_edge_branch):
     """Gets v8 bleeding edge revision mapped to v8 revision in trunk.
 
@@ skipping 54 matching lines @@
               return re_results.group('git_revision')
           except (IndexError, ValueError):
             pass
         if not git_revision:
           # Wasn't successful, try the old way of looking for "Prepare push to"
           git_revision = source_control.ResolveToRevision(
               int(commit_position) - 1, 'v8_bleeding_edge',
               bisect_utils.DEPOT_DEPS_NAME, -1, cwd=v8_bleeding_edge_dir)
 
           if git_revision:
-            revision_info = source_control.QueryRevisionInfo(git_revision,
-                cwd=v8_bleeding_edge_dir)
+            revision_info = source_control.QueryRevisionInfo(
+                git_revision, cwd=v8_bleeding_edge_dir)
 
             if 'Prepare push to trunk' in revision_info['subject']:
               return git_revision
     return None
 
   def _GetNearestV8BleedingEdgeFromTrunk(
       self, revision, v8_branch, bleeding_edge_branch, search_forward=True):
     """Gets the nearest V8 roll and maps to bleeding edge revision.
 
     V8 is a bit tricky to bisect since it isn't just rolled out like blink.
@@ skipping 56 matching lines @@
 
     # Support for the chromium revisions with external V8 repo.
     # ie https://chromium.googlesource.com/external/v8.git
     cmd = ['config', '--get', 'remote.origin.url']
     v8_repo_url = bisect_utils.CheckRunGit(cmd, cwd=cwd)
 
     if 'external/v8.git' in v8_repo_url:
       v8_branch = 'origin/master'
       bleeding_edge_branch = 'origin/bleeding_edge'
 
-    r1 = self._GetNearestV8BleedingEdgeFromTrunk(min_revision_state.revision,
-        v8_branch, bleeding_edge_branch, search_forward=True)
-    r2 = self._GetNearestV8BleedingEdgeFromTrunk(max_revision_state.revision,
-        v8_branch, bleeding_edge_branch, search_forward=False)
+    r1 = self._GetNearestV8BleedingEdgeFromTrunk(
+        min_revision_state.revision,
+        v8_branch,
+        bleeding_edge_branch,
+        search_forward=True)
+    r2 = self._GetNearestV8BleedingEdgeFromTrunk(
+        max_revision_state.revision,
+        v8_branch,
+        bleeding_edge_branch,
+        search_forward=False)
     min_revision_state.external['v8_bleeding_edge'] = r1
     max_revision_state.external['v8_bleeding_edge'] = r2
 
     if (not self._GetV8BleedingEdgeFromV8TrunkIfMappable(
             min_revision_state.revision, bleeding_edge_branch)
         or not self._GetV8BleedingEdgeFromV8TrunkIfMappable(
             max_revision_state.revision, bleeding_edge_branch)):
       self.warnings.append(
           'Trunk revisions in V8 did not map directly to bleeding_edge. '
           'Attempted to expand the range to find V8 rolls which did map '
           'directly to bleeding_edge revisions, but results might not be '
           'valid.')
 
   def _FindNextDepotToBisect(
       self, current_depot, min_revision_state, max_revision_state):
     """Decides which depot the script should dive into next (if any).
 
     Args:
       current_depot: Current depot being bisected.
       min_revision_state: State of the earliest revision in the bisect range.
       max_revision_state: State of the latest revision in the bisect range.
 
     Returns:
       Name of the depot to bisect next, or None.
     """
     external_depot = None
     for next_depot in bisect_utils.DEPOT_NAMES:
-      if bisect_utils.DEPOT_DEPS_NAME[next_depot].has_key('platform'):
-        if bisect_utils.DEPOT_DEPS_NAME[next_depot]['platform'] != os.name:
-          continue
+      if ('platform' in bisect_utils.DEPOT_DEPS_NAME[next_depot] and
+          bisect_utils.DEPOT_DEPS_NAME[next_depot]['platform'] != os.name):
+        continue
 
       if not (bisect_utils.DEPOT_DEPS_NAME[next_depot]['recurse']
               and min_revision_state.depot
               in bisect_utils.DEPOT_DEPS_NAME[next_depot]['from']):
         continue
 
       if current_depot == 'v8':
         # We grab the bleeding_edge info here rather than earlier because we
         # finally have the revision range. From that we can search forwards and
         # backwards to try to match trunk revisions to bleeding_edge.
@@ skipping 24 matching lines @@
     Returns:
       A list containing the revisions between |start_revision| and
       |end_revision| inclusive.
     """
     # Change into working directory of external library to run
     # subsequent commands.
     self.depot_registry.ChangeToDepotDir(current_depot)
 
     # V8 (and possibly others) is merged in periodically. Bisecting
     # this directory directly won't give much good info.
-    if bisect_utils.DEPOT_DEPS_NAME[current_depot].has_key('custom_deps'):
+    if 'custom_deps' in bisect_utils.DEPOT_DEPS_NAME[current_depot]:
       config_path = os.path.join(self.src_cwd, '..')
       if bisect_utils.RunGClientAndCreateConfig(
           self.opts, bisect_utils.DEPOT_DEPS_NAME[current_depot]['custom_deps'],
           cwd=config_path):
         return []
       if bisect_utils.RunGClient(
           ['sync', '--revision', previous_revision], cwd=self.src_cwd):
         return []
 
     if current_depot == 'v8_bleeding_edge':
       self.depot_registry.ChangeToDepotDir('chromium')
 
       shutil.move('v8', 'v8.bak')
       shutil.move('v8_bleeding_edge', 'v8')
 
       self.cleanup_commands.append(['mv', 'v8', 'v8_bleeding_edge'])
       self.cleanup_commands.append(['mv', 'v8.bak', 'v8'])
 
-      self.depot_registry.SetDepotDir('v8_bleeding_edge',
-                                  os.path.join(self.src_cwd, 'v8'))
-      self.depot_registry.SetDepotDir('v8', os.path.join(self.src_cwd,
-                                                         'v8.bak'))
+      self.depot_registry.SetDepotDir(
+          'v8_bleeding_edge', os.path.join(self.src_cwd, 'v8'))
+      self.depot_registry.SetDepotDir(
+          'v8', os.path.join(self.src_cwd, 'v8.bak'))
 
       self.depot_registry.ChangeToDepotDir(current_depot)
 
     depot_revision_list = self.GetRevisionList(current_depot,
                                                end_revision,
                                                start_revision)
 
     self.depot_registry.ChangeToDepotDir('chromium')
 
     return depot_revision_list
@@ skipping 67 matching lines @@
             bisect_utils.FILE_DEPS_GIT, oldest_deps_change, bad_revision)
 
         if len(changes_to_deps) != len(changes_to_gitdeps):
           # Grab the timestamp of the last DEPS change
           cmd = ['log', '--format=%ct', '-1', changes_to_deps[0]]
           output = bisect_utils.CheckRunGit(cmd)
           commit_time = int(output)
 
           # Try looking for a commit that touches the .DEPS.git file in the
           # next 15 minutes after the DEPS file change.
-          cmd = ['log', '--format=%H', '-1',
-              '--before=%d' % (commit_time + 900), '--after=%d' % commit_time,
-              'origin/master', '--', bisect_utils.FILE_DEPS_GIT]
+          cmd = [
+              'log', '--format=%H', '-1',
+              '--before=%d' % (commit_time + 900),
+              '--after=%d' % commit_time,
+              'origin/master', '--', bisect_utils.FILE_DEPS_GIT
+          ]
           output = bisect_utils.CheckRunGit(cmd)
           output = output.strip()
           if output:
-            self.warnings.append('Detected change to DEPS and modified '
+            self.warnings.append(
+                'Detected change to DEPS and modified '
                 'revision range to include change to .DEPS.git')
             return (output, good_revision)
           else:
-            self.warnings.append('Detected change to DEPS but couldn\'t find '
+            self.warnings.append(
+                'Detected change to DEPS but couldn\'t find '
                 'matching change to .DEPS.git')
     return (bad_revision, good_revision)
 
   def CheckIfRevisionsInProperOrder(
       self, target_depot, good_revision, bad_revision):
     """Checks that |good_revision| is an earlier revision than |bad_revision|.
 
     Args:
       good_revision: Number/tag of the known good revision.
       bad_revision: Number/tag of the known bad revision.
 
     Returns:
       True if the revisions are in the proper order (good earlier than bad).
     """
     cwd = self.depot_registry.GetDepotDir(target_depot)
     good_position = source_control.GetCommitPosition(good_revision, cwd)
     bad_position = source_control.GetCommitPosition(bad_revision, cwd)
     # Compare commit timestamp for repos that don't support commit position.
     if not (bad_position and good_position):
       good_position = source_control.GetCommitTime(good_revision, cwd=cwd)
       bad_position = source_control.GetCommitTime(bad_revision, cwd=cwd)
 
     return good_position <= bad_position
 
   def CanPerformBisect(self, good_revision, bad_revision):
     """Checks whether a given revision is bisectable.
 
     Checks for following:
-    1. Non-bisectable revsions for android bots (refer to crbug.com/385324).
-    2. Non-bisectable revsions for Windows bots (refer to crbug.com/405274).
+    1. Non-bisectable revisions for android bots (refer to crbug.com/385324).
+    2. Non-bisectable revisions for Windows bots (refer to crbug.com/405274).
 
     Args:
       good_revision: Known good revision.
       bad_revision: Known bad revision.
 
     Returns:
       A dictionary indicating the result. If revision is not bisectable,
       this will contain the field "error", otherwise None.
     """
     if self.opts.target_platform == 'android':
@@ skipping 29 matching lines @@
     performance tests again with and without the CL, adding the results to
     the over bisect results.
 
     Args:
       results: BisectResults from the bisect.
       target_depot: The target depot we're bisecting.
       command_to_run: Specify the command to execute the performance test.
       metric: The performance metric to monitor.
     """
     run_results_tot, run_results_reverted = self._RevertCulpritCLAndRetest(
-      results, target_depot, command_to_run, metric)
+        results, target_depot, command_to_run, metric)
 
     results.AddRetestResults(run_results_tot, run_results_reverted)
 
     if len(results.culprit_revisions) != 1:
       return
 
     # Cleanup reverted files if anything is left.
     _, _, culprit_depot = results.culprit_revisions[0]
-    bisect_utils.CheckRunGit(['reset', '--hard', 'HEAD'],
+    bisect_utils.CheckRunGit(
+        ['reset', '--hard', 'HEAD'],
         cwd=self.depot_registry.GetDepotDir(culprit_depot))
 
   def _RevertCL(self, culprit_revision, culprit_depot):
     """Reverts the specified revision in the specified depot."""
     if self.opts.output_buildbot_annotations:
       bisect_utils.OutputAnnotationStepStart(
           'Reverting culprit CL: %s' % culprit_revision)
     _, return_code = bisect_utils.RunGit(
         ['revert', '--no-commit', culprit_revision],
         cwd=self.depot_registry.GetDepotDir(culprit_depot))
@@ skipping 50 matching lines @@
           'Culprit CL is in another depot, attempting to revert and build'
           ' locally to retest. This may not match the performance of official'
           ' builds.')
 
     run_results_reverted = self._RunTestWithAnnotations(
         'Re-Testing ToT with reverted culprit',
         'Failed to run reverted CL.',
         head_revision, target_depot, command_to_run, metric, force_build)
 
     # Clear the reverted file(s).
-    bisect_utils.RunGit(['reset', '--hard', 'HEAD'],
+    bisect_utils.RunGit(
+        ['reset', '--hard', 'HEAD'],
         cwd=self.depot_registry.GetDepotDir(culprit_depot))
 
     # Retesting with the reverted CL failed, so bail out of retesting against
     # ToT.
     if run_results_reverted[1]:
       return (None, None)
 
     run_results_tot = self._RunTestWithAnnotations(
         'Re-Testing ToT',
         'Failed to run ToT.',
         head_revision, target_depot, command_to_run, metric, force_build)
 
     return (run_results_tot, run_results_reverted)
 
-  def _RunTestWithAnnotations(self, step_text, error_text, head_revision,
+  def _RunTestWithAnnotations(
+      self, step_text, error_text, head_revision,
       target_depot, command_to_run, metric, force_build):
     """Runs the performance test and outputs start/stop annotations.
 
     Args:
       results: BisectResults from the bisect.
       target_depot: The target depot we're bisecting.
       command_to_run: Specify the command to execute the performance test.
       metric: The performance metric to monitor.
       force_build: Whether to force a build locally.
 
@@ skipping 163 matching lines @@
       # Check how likely it is that the good and bad results are different
       # beyond chance-induced variation.
       confidence_error = False
       if not self.opts.debug_ignore_regression_confidence:
         confidence_error = _CheckRegressionConfidenceError(good_revision,
                                                            bad_revision,
                                                            known_good_value,
                                                            known_bad_value)
         if confidence_error:
           self.warnings.append(confidence_error)
-          bad_revision_state.passed = True # Marking the 'bad' revision as good.
+          # Marking the 'bad' revision as good BECAUSE XXX ...

[RobertoCN, 2015/03/12 20:57:03]

because if there is no confidence that there's a regression, the revision given
as 'bad' is likely within noise range of what is considered good.

[qyearsley, 2015/03/12 21:09:03]

Ah, thanks for catching this, I meant to expand on the comment or remove the
comment.

I was wondering whether the passed attribute of the bad revision RevisionState
is specially checked later on, like in bisect_results or bisect_printer? Or
whether it would cause the output to be confusing in some way if this wasn't set
to True?

(If this statement isn't necessary then I'd like to remove it...)

[RobertoCN, 2015/03/13 22:46:17]

I don't remember there being any real need for this. I probably marked it as
good to prevent the culprit finding function from accidentally locating one
(IIRC my thoughts were: if the bisect is given the range of 2 adjacent revisions
and there's no confidence, none of them should be considered bad, lest the
results printer or some other component find a false culprit)

+          bad_revision_state.passed = True
           return BisectResults(bisect_state, self.depot_registry, self.opts,
                                self.warnings)
 
       while True:
         if not revision_states:
           break
 
         if max_revision - min_revision <= 1:
           min_revision_state = revision_states[min_revision]
           max_revision_state = revision_states[max_revision]
           current_depot = min_revision_state.depot
           # TODO(sergiyb): Under which conditions can first two branches be hit?
           if min_revision_state.passed == '?':
             next_revision_index = min_revision
           elif max_revision_state.passed == '?':
             next_revision_index = max_revision
           elif current_depot in ['android-chrome', 'chromium', 'v8']:
             previous_revision = revision_states[min_revision].revision
             # If there were changes to any of the external libraries we track,
             # should bisect the changes there as well.
             external_depot = self._FindNextDepotToBisect(
                 current_depot, min_revision_state, max_revision_state)
             # If there was no change in any of the external depots, the search
             # is over.
             if not external_depot:
               if current_depot == 'v8':
-                self.warnings.append('Unfortunately, V8 bisection couldn\'t '
+                self.warnings.append(
+                    'Unfortunately, V8 bisection couldn\'t '
                     'continue any further. The script can only bisect into '
                     'V8\'s bleeding_edge repository if both the current and '
                     'previous revisions in trunk map directly to revisions in '
                     'bleeding_edge.')
               break
 
             earliest_revision = max_revision_state.external[external_depot]
             latest_revision = min_revision_state.external[external_depot]
 
             new_revision_list = self.PrepareToBisectOnDepot(
@@ skipping 69 matching lines @@
 
           # If the build is broken, remove it and redo search.
           revision_states.pop(next_revision_index)
 
           max_revision -= 1
 
         if self.opts.output_buildbot_annotations:
           self.printer.PrintPartialResults(bisect_state)
           bisect_utils.OutputAnnotationStepClosed()
 
-
       self._ConfidenceExtraTestRuns(min_revision_state, max_revision_state,
                                     command_to_run, metric)
       results = BisectResults(bisect_state, self.depot_registry, self.opts,
                               self.warnings)
 
       self._GatherResultsFromRevertedCulpritCL(
           results, target_depot, command_to_run, metric)
 
       return results
     else:
@@ skipping 343 matching lines @@
       else:
         print 'Could not confirm bug is closed, proceeding.'
       if opts.output_buildbot_annotations:
         bisect_utils.OutputAnnotationStepClosed()
       if issue_closed:
         results = BisectResults(abort_reason='the bug is closed.')
         bisect_test = BisectPerformanceMetrics(opts, os.getcwd())
         bisect_test.printer.FormatAndPrintResults(results)
         return 0
 
-
     if opts.extra_src:
       extra_src = bisect_utils.LoadExtraSrc(opts.extra_src)
       if not extra_src:
         raise RuntimeError('Invalid or missing --extra_src.')
       bisect_utils.AddAdditionalDepotInfo(extra_src.GetAdditionalDepotInfo())
 
     if opts.working_directory:
       custom_deps = bisect_utils.DEFAULT_GCLIENT_CUSTOM_DEPS
       if opts.no_custom_deps:
         custom_deps = None
@@ skipping 30 matching lines @@
       # bugs. If you change this, please update the perf dashboard as well.
       bisect_utils.OutputAnnotationStepStart('Results')
     print 'Runtime Error: %s' % e
     if opts.output_buildbot_annotations:
       bisect_utils.OutputAnnotationStepClosed()
   return 1
 
 
 if __name__ == '__main__':
   sys.exit(main())