Add new canned checks: CheckRietveldTryJobExecution and CheckBuildbotPendingBuilds

presubmit_canned_checks.py

Index: presubmit_canned_checks.py
diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py
index 839fe1d7f3b56e6ec18b20492cd5a409d2c32b3d..52081912b6665a23edaafe5d7c32991b6d998192 100644
--- a/presubmit_canned_checks.py
+++ b/presubmit_canned_checks.py
@@ -12,7 +12,7 @@ def CheckChangeHasTestField(input_api, output_api):
     return []
   else:
     return [output_api.PresubmitNotifyResult(
-        "Changelist should have a TEST= field. TEST=none is allowed.")]
+        'Changelist should have a TEST= field. TEST=none is allowed.')]
 
 
 def CheckChangeHasBugField(input_api, output_api):
@@ -21,7 +21,7 @@ def CheckChangeHasBugField(input_api, output_api):
     return []
   else:
     return [output_api.PresubmitNotifyResult(
-        "Changelist should have a BUG= field. BUG=none is allowed.")]
+        'Changelist should have a BUG= field. BUG=none is allowed.')]
 
 
 def CheckChangeHasTestedField(input_api, output_api):
@@ -29,7 +29,7 @@ def CheckChangeHasTestedField(input_api, output_api):
   if input_api.change.TESTED:
     return []
   else:
-    return [output_api.PresubmitError("Changelist must have a TESTED= field.")]
+    return [output_api.PresubmitError('Changelist must have a TESTED= field.')]
 
 
 def CheckChangeHasQaField(input_api, output_api):
@@ -37,7 +37,7 @@ def CheckChangeHasQaField(input_api, output_api):
   if input_api.change.QA:
     return []
   else:
-    return [output_api.PresubmitError("Changelist must have a QA= field.")]
+    return [output_api.PresubmitError('Changelist must have a QA= field.')]
 
 
 def CheckDoNotSubmitInDescription(input_api, output_api):
@@ -46,7 +46,7 @@ def CheckDoNotSubmitInDescription(input_api, output_api):
   keyword = 'DO NOT ' + 'SUBMIT'
   if keyword in input_api.change.DescriptionText():
     return [output_api.PresubmitError(
-        keyword + " is present in the changelist description.")]
+        keyword + ' is present in the changelist description.')]
   else:
     return []
 
@@ -56,9 +56,9 @@ def CheckChangeHasDescription(input_api, output_api):
   text = input_api.change.DescriptionText()
   if text.strip() == '':
     if input_api.is_committing:
-      return [output_api.PresubmitError("Add a description.")]
+      return [output_api.PresubmitError('Add a description.')]
     else:
-      return [output_api.PresubmitNotifyResult("Add a description.")]
+      return [output_api.PresubmitNotifyResult('Add a description.')]
   return []
 
 ### Content checks
@@ -75,7 +75,7 @@ def CheckDoNotSubmitInFiles(input_api, output_api):
 
 
 def CheckChangeLintsClean(input_api, output_api, source_file_filter=None):
-  """Checks that all ".cc" and ".h" files pass cpplint.py."""
+  """Checks that all '.cc' and '.h' files pass cpplint.py."""
   _RE_IS_TEST = input_api.re.compile(r'.*tests?.(cc|h)$')
   result = []
 
@@ -92,9 +92,9 @@ def CheckChangeLintsClean(input_api, output_api, source_file_filter=None):
   # - runtime/int         : Can be fixed long term; volume of errors too high
   # - runtime/virtual     : Broken now, but can be fixed in the future?
   # - whitespace/braces   : We have a lot of explicit scoping in chrome code.
-  cpplint._SetFilters("-build/include,-build/include_order,-build/namespace,"
-                      "-readability/casting,-runtime/int,-runtime/virtual,"
-                      "-whitespace/braces")
+  cpplint._SetFilters('-build/include,-build/include_order,-build/namespace,'
+                      '-readability/casting,-runtime/int,-runtime/virtual,'
+                      '-whitespace/braces')
 
   # We currently are more strict with normal code than unit tests; 4 and 5 are
   # the verbosity level that would normally be passed to cpplint.py through
@@ -114,7 +114,7 @@ def CheckChangeLintsClean(input_api, output_api, source_file_filter=None):
       res_type = output_api.PresubmitError
     else:
       res_type = output_api.PresubmitPromptWarning
-    result = [res_type("Changelist failed cpplint.py check.")]
+    result = [res_type('Changelist failed cpplint.py check.')]
 
   return result
 
@@ -127,7 +127,7 @@ def CheckChangeHasNoCR(input_api, output_api, source_file_filter=None):
       cr_files.append(f.LocalPath())
   if cr_files:
     return [output_api.PresubmitPromptWarning(
-        "Found a CR character in these files:", items=cr_files)]
+        'Found a CR character in these files:', items=cr_files)]
   return []
 
 
@@ -162,7 +162,7 @@ def CheckSvnModifiedDirectories(input_api, output_api, source_file_filter=None):
         else:
           error_type = output_api.PresubmitNotifyResult
         errors.append(error_type(
-            "Potential accidental commits in changelist %s:" % f.LocalPath(),
+            'Potential accidental commits in changelist %s:' % f.LocalPath(),
             items=bad_files))
   return errors
 
@@ -173,7 +173,7 @@ def CheckChangeHasOnlyOneEol(input_api, output_api, source_file_filter=None):
   for f in input_api.AffectedSourceFiles(source_file_filter):
     contents = input_api.ReadFile(f, 'rb')
     # Check that the file ends in one and only one newline character.
-    if len(contents) > 1 and (contents[-1:] != "\n" or contents[-2:-1] == "\n"):
+    if len(contents) > 1 and (contents[-1:] != '\n' or contents[-2:-1] == '\n'):
       eof_files.append(f.LocalPath())
 
   if eof_files:
@@ -196,12 +196,12 @@ def CheckChangeHasNoCrAndHasOnlyOneEol(input_api, output_api,
     if '\r' in contents:
       cr_files.append(f.LocalPath())
     # Check that the file ends in one and only one newline character.
-    if len(contents) > 1 and (contents[-1:] != "\n" or contents[-2:-1] == "\n"):
+    if len(contents) > 1 and (contents[-1:] != '\n' or contents[-2:-1] == '\n'):
       eof_files.append(f.LocalPath())
   outputs = []
   if cr_files:
     outputs.append(output_api.PresubmitPromptWarning(
-        "Found a CR character in these files:", items=cr_files))
+        'Found a CR character in these files:', items=cr_files))
   if eof_files:
     outputs.append(output_api.PresubmitPromptWarning(
       'These files should end in one (and only one) newline character:',
@@ -216,10 +216,10 @@ def CheckChangeHasNoTabs(input_api, output_api, source_file_filter=None):
   tabs = []
   for f, line_num, line in input_api.RightHandSideLines(source_file_filter):
     if '\t' in line:
-      tabs.append("%s, line %s" % (f.LocalPath(), line_num))
+      tabs.append('%s, line %s' % (f.LocalPath(), line_num))
   if tabs:
-    return [output_api.PresubmitPromptWarning("Found a tab character in:",
-                                              long_text="\n".join(tabs))]
+    return [output_api.PresubmitPromptWarning('Found a tab character in:',
+                                              long_text='\n'.join(tabs))]
   return []
 
 
@@ -229,11 +229,11 @@ def CheckChangeHasNoStrayWhitespace(input_api, output_api,
   errors = []
   for f, line_num, line in input_api.RightHandSideLines(source_file_filter):
     if line.rstrip() != line:
-      errors.append("%s, line %s" % (f.LocalPath(), line_num))
+      errors.append('%s, line %s' % (f.LocalPath(), line_num))
   if errors:
     return [output_api.PresubmitPromptWarning(
-        "Found line ending with white spaces in:",
-        long_text="\n".join(errors))]
+        'Found line ending with white spaces in:',
+        long_text='\n'.join(errors))]
   return []
 
 
@@ -260,7 +260,7 @@ def CheckLongLines(input_api, output_api, maxlen=80, source_file_filter=None):
         break
 
   if bad:
-    msg = "Found lines longer than %s characters (first 5 shown)." % maxlen
+    msg = 'Found lines longer than %s characters (first 5 shown).' % maxlen
     return [output_api.PresubmitPromptWarning(msg, items=bad)]
   else:
     return []
@@ -281,7 +281,7 @@ def CheckLicense(input_api, output_api, license, source_file_filter=None):
     else:
       res_type = output_api.PresubmitNotifyResult
     return [res_type(
-        "Found a bad license header in these files:", items=bad_files)]
+        'Found a bad license header in these files:', items=bad_files)]
   return []
 
 
@@ -327,7 +327,7 @@ def CheckSvnProperty(input_api, output_api, prop, expected, affected_files):
       res_type = output_api.PresubmitError
     else:
       res_type = output_api.PresubmitNotifyResult
-    message = "Run the command: svn pset %s %s \\" % (prop, expected)
+    message = 'Run the command: svn pset %s %s \\' % (prop, expected)
     return [res_type(message, items=bad)]
   return []
 
@@ -344,14 +344,15 @@ def CheckDoNotSubmit(input_api, output_api):
 def CheckTreeIsOpen(input_api, output_api, url, closed):
   """Checks that an url's content doesn't match a regexp that would mean that
   the tree is closed."""
-  assert(input_api.is_committing)
+  if not input_api.is_committing:
+    return []
   try:
     connection = input_api.urllib2.urlopen(url)
     status = connection.read()
     connection.close()
     if input_api.re.match(closed, status):
       long_text = status + '\n' + url
-      return [output_api.PresubmitPromptWarning("The tree is closed.",
+      return [output_api.PresubmitPromptWarning('The tree is closed.',
                                                 long_text=long_text)]
   except IOError:
     pass
@@ -375,7 +376,7 @@ def RunPythonUnitTests(input_api, output_api, unit_tests):
     cwd = None
     env = None
     unit_test_name = unit_test
-    # "python -m test.unit_test" doesn't work. We need to change to the right
+    # 'python -m test.unit_test' doesn't work. We need to change to the right
     # directory instead.
     if '.' in unit_test:
       # Tests imported in submodules (subdirectories) assume that the current
@@ -394,8 +395,8 @@ def RunPythonUnitTests(input_api, output_api, unit_tests):
     subproc = input_api.subprocess.Popen(
         [
           input_api.python_executable,
-          "-m",
-          "%s" % unit_test
+          '-m',
+          '%s' % unit_test
         ],
         cwd=cwd,
         env=env,
@@ -405,9 +406,92 @@ def RunPythonUnitTests(input_api, output_api, unit_tests):
     stdoutdata, stderrdata = subproc.communicate()
     # Discard the output if returncode == 0
     if subproc.returncode:
-      outputs.append("Test '%s' failed with code %d\n%s\n%s\n" % (
+      outputs.append('Test \'%s\' failed with code %d\n%s\n%s\n' % (
           unit_test_name, subproc.returncode, stdoutdata, stderrdata))
   if outputs:
-    return [message_type("%d unit tests failed." % len(outputs),
+    return [message_type('%d unit tests failed.' % len(outputs),
                                 long_text='\n'.join(outputs))]
   return []
+
+
+def CheckRietveldTryJobExecution(input_api, output_api, host_url, platforms,
+                                 owner):
+  if not input_api.is_committing:
+    return []
+  if not input_api.change.issue or not input_api.change.patchset:
+    return []
+  url = '%s/%d/get_build_results/%d' % (
+      host_url, input_api.change.issue, input_api.change.patchset)
+  try:
+    connection = input_api.urllib2.urlopen(url)
+    # platform|status|url
+    values = [item.split('|', 2) for item in connection.read().splitlines()]
+    connection.close()
+  except input_api.urllib2.HTTPError, e:
+    if e.code == 404:
+      # Fallback to no try job.
+      return [output_api.PresubmitPromptWarning(
+          'You should try the patch first.')]
+    else:
+      # Another HTTP error happened, warn the user.
+      return [output_api.PresubmitPromptWarning(
+          'Got %s while looking for try job status.' % str(e))]
+
+  if not values:
+    # It returned an empty list. Probably a private review.
+    return []
+  # Reformat as an dict of platform: [status, url]
+  values = dict([[v[0], [v[1], v[2]]] for v in values if len(v) == 3])
+  if not values:
+    # It returned useless data.
+    return [output_api.PresubmitNotifyResult('Failed to parse try job results')]
+
+  for platform in platforms:
+    values.setdefault(platform, ['not started', ''])
+  message = None
+  non_success = [k.upper() for k,v in values.iteritems() if v[0] != 'success']
+  if 'failure' in [v[0] for v in values.itervalues()]:
+    message = 'Try job failures on %s!\n' % ', '.join(non_success)
+  elif non_success:
+    message = ('Unfinished (or not even started) try jobs on '
+                '%s.\n') % ', '.join(non_success)
+  if message:
+    message += (
+        'Is try server wrong or broken? Please notify %s. '
+        'Thanks.\n' % owner)
+    return [output_api.PresubmitPromptWarning(message=message)]
+  return []
+
+
+def CheckBuildbotPendingBuilds(input_api, output_api, url, max_pendings,
+    ignored):
+  if not input_api.json:
+    return [output_api.PresubmitPromptWarning(
+      'Please install simplejson or upgrade to python 2.6+')]
+  try:
+    connection = input_api.urllib2.urlopen(url)
+    raw_data = connection.read()
+    connection.close()
+  except IOError:
+    return [output_api.PresubmitNotifyResult('%s is not accessible' % url)]
+
+  try:
+    data = input_api.json.loads(raw_data)
+  except ValueError:
+    return [output_api.PresubmitNotifyResult('Received malformed json while '
+                                             'looking up buildbot status')]
+
+  out = []
+  for (builder_name, builder) in data.iteritems():
+    if builder_name in ignored:
+      continue
+    pending_builds_len = len(builder.get('pending_builds', []))
+    if pending_builds_len > max_pendings:
+      out.append('%s has %d build(s) pending' %
+                  (builder_name, pending_builds_len))
+  if out:
+    return [output_api.PresubmitPromptWarning(
+        'Build(s) pending. It is suggested to wait that no more than %d '
+            'builds are pending.' % max_pendings,
+        long_text='\n'.join(out))]
+  return []