Bench Rebaseline from Codereview: separate out all_trybots_finished()

tools/gen_bench_expectations_from_codereview.py

 #!/usr/bin/python
 
 # Copyright (c) 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.
 
 
 """Generate new bench expectations from results of trybots on a code review."""
 
 
 import collections
 import compare_codereview
 import os
 import re
 import shutil
 import subprocess
 import sys
 
 
 BENCH_DATA_URL = 'gs://chromium-skia-gm/perfdata/%s/%s/*'
 CHECKOUT_PATH = os.path.realpath(os.path.join(
     os.path.dirname(os.path.abspath(__file__)), os.pardir))
 TMP_BENCH_DATA_DIR = os.path.join(CHECKOUT_PATH, '.bench_data')
 
 
+TryBuild = collections.namedtuple(
+    'TryBuild', ['builder_name', 'build_number', 'is_finished'])
+
+
 def find_all_builds(codereview_url):
   """Finds and returns information about trybot runs for a code review.
 
   Args:
     codereview_url: URL of the codereview in question.
 
   Returns:
       List of NamedTuples: (builder_name, build_number, is_finished)
   """
   results = compare_codereview.CodeReviewHTMLParser().parse(codereview_url)
-  TryBuild = collections.namedtuple(
-      'TryBuild', ['builder_name', 'build_number', 'is_finished'])
   try_builds = []
-
   for builder, data in results.iteritems():
     if builder.startswith('Perf'):
       try_builds.append(TryBuild(builder, data.url.split('/')[-1],
                                  data.status != 'pending'))
   return try_builds
 
 
+def _all_trybots_finished(try_builds):
+  """Return True iff all of the given try jobs have finished.
+
+  Args:
+      try_builds: list of TryBuild instances.
+
+  Returns:
+      True if all of the given try jobs have finished, otherwise False.
+  """
+  for try_build in try_builds:
+    if not try_build.is_finished:
+      return False
+  return True
+
+
+def all_trybots_finished(codereview_url):
+  """Return True iff all of the try jobs on the given codereview have finished.
+
+  Args:
+      codereview_url: string; URL of the codereview.
+
+  Returns:
+      True if all of the try jobs have finished, otherwise False.
+  """
+  return _all_trybots_finished(find_all_builds(codereview_url))
+
+
 def get_bench_data(builder, build_num, dest_dir):
   """Download the bench data for the given builder at the given build_num.
 
   Args:
       builder: string; name of the builder.
       build_num: string; build number.
       dest_dir: string; destination directory for the bench data.
   """
   url = BENCH_DATA_URL % (builder, build_num)
   subprocess.check_call(['gsutil', 'cp', '-R', url, dest_dir],
@@ skipping 30 matching lines @@
   finished trybots and uses them to generate new expectations for their
   waterfall counterparts.
 
   Args:
       url: string; URL of the code review.
       error_on_unfinished: bool; throw an error if any trybot has not finished.
   """
   try_builds = find_all_builds(codereview_url)
 
   # Verify that all trybots have finished running.
-  if error_on_unfinished:
-    for try_build in try_builds:
-      if not try_build.is_finished:
-        raise TrybotNotFinishedError('%s: #%s is not finished.' % (
-                                         try_build.builder_name,
-                                         try_build.build_number))
+  if error_on_unfinished and not _all_trybots_finished(try_builds):

[benchen, 2014/05/29 13:20:26]

all_trybots_finished(codereview_url) defined above are not used. try_builds are
used below. So shall we still keep the unused
all_trybots_finished(codereview_url)?

[borenet, 2014/05/29 13:21:59]

It's going to be used by the RecreateSKPs bot.

+    raise TrybotNotFinishedError('Not all trybots have finished.')
+
   failed_data_pull = []
   failed_gen_expectations = []
 
   if os.path.isdir(TMP_BENCH_DATA_DIR):
     shutil.rmtree(TMP_BENCH_DATA_DIR)
 
   for try_build in try_builds:
     try_builder = try_build.builder_name
     builder = try_builder.replace('-Trybot', '')
 
@@ skipping 32 matching lines @@
   if failed_gen_expectations:
     failure += 'Failed to generate expectations for: %s\n\n' % ','.join(
         failed_gen_expectations)
   if failure:
     raise Exception(failure)
 
 
 if __name__ == '__main__':
   gen_bench_expectations_from_codereview(sys.argv[1])