aiden
/
depot_tools
mirror of https://chromium.googlesource.com/chromium/tools/depot_tools
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add new canned checks: CheckRietveldTryJobExecution and CheckBuildbotPendingBuilds

Browse Source 
These are the same that the one in chromium's src/PRESUBMIT.py but slightly more
resilitent to exceptions.
Empirical studies have shown that exceptions in presubmit checks result in
disorientation and lack of willingness to commit.

TEST=OMG new unit tests!

Review URL: http://codereview.chromium.org/1119003

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@42102 0039d316-1c4b-4281-b951-d872f2087c98
experimental/szager/collated-output
maruel@chromium.org 16 years ago
parent fb11c7b98d
commit 3fbcb08a1d
2 changed files with 195 additions and 33 deletions
Show Stats Download Patch File Download Diff File

150
presubmit_canned_checks.py
Unescape Escape View File

@ -12,7 +12,7 @@ def CheckChangeHasTestField(input_api, output_api):
return [] return []
else: else:
return [output_api.PresubmitNotifyResult( 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): def CheckChangeHasBugField(input_api, output_api):
@ -21,7 +21,7 @@ def CheckChangeHasBugField(input_api, output_api):
return [] return []
else: else:
return [output_api.PresubmitNotifyResult( 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): def CheckChangeHasTestedField(input_api, output_api):
@ -29,7 +29,7 @@ def CheckChangeHasTestedField(input_api, output_api):
if input_api.change.TESTED: if input_api.change.TESTED:
return [] return []
else: 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): def CheckChangeHasQaField(input_api, output_api):
@ -37,7 +37,7 @@ def CheckChangeHasQaField(input_api, output_api):
if input_api.change.QA: if input_api.change.QA:
return [] return []
else: 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): def CheckDoNotSubmitInDescription(input_api, output_api):
@ -46,7 +46,7 @@ def CheckDoNotSubmitInDescription(input_api, output_api):
keyword = 'DO NOT ' + 'SUBMIT' keyword = 'DO NOT ' + 'SUBMIT'
if keyword in input_api.change.DescriptionText(): if keyword in input_api.change.DescriptionText():
return [output_api.PresubmitError( return [output_api.PresubmitError(
keyword + " is present in the changelist description.")] keyword + ' is present in the changelist description.')]
else: else:
return [] return []
@ -56,9 +56,9 @@ def CheckChangeHasDescription(input_api, output_api):
text = input_api.change.DescriptionText() text = input_api.change.DescriptionText()
if text.strip() == '': if text.strip() == '':
if input_api.is_committing: if input_api.is_committing:
return [output_api.PresubmitError("Add a description.")] return [output_api.PresubmitError('Add a description.')]
else: else:
return [output_api.PresubmitNotifyResult("Add a description.")] return [output_api.PresubmitNotifyResult('Add a description.')]
return [] return []
### Content checks ### Content checks
@ -75,7 +75,7 @@ def CheckDoNotSubmitInFiles(input_api, output_api):
def CheckChangeLintsClean(input_api, output_api, source_file_filter=None): 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)$') _RE_IS_TEST = input_api.re.compile(r'.*tests?.(cc|h)$')
result = [] 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/int : Can be fixed long term; volume of errors too high
# - runtime/virtual : Broken now, but can be fixed in the future? # - runtime/virtual : Broken now, but can be fixed in the future?
# - whitespace/braces : We have a lot of explicit scoping in chrome code. # - whitespace/braces : We have a lot of explicit scoping in chrome code.
cpplint._SetFilters("-build/include,-build/include_order,-build/namespace," cpplint._SetFilters('-build/include,-build/include_order,-build/namespace,'
"-readability/casting,-runtime/int,-runtime/virtual," '-readability/casting,-runtime/int,-runtime/virtual,'
"-whitespace/braces") '-whitespace/braces')
# We currently are more strict with normal code than unit tests; 4 and 5 are # 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 # 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 res_type = output_api.PresubmitError
else: else:
res_type = output_api.PresubmitPromptWarning res_type = output_api.PresubmitPromptWarning
result = [res_type("Changelist failed cpplint.py check.")] result = [res_type('Changelist failed cpplint.py check.')]
return result return result
@ -127,7 +127,7 @@ def CheckChangeHasNoCR(input_api, output_api, source_file_filter=None):
cr_files.append(f.LocalPath()) cr_files.append(f.LocalPath())
if cr_files: if cr_files:
return [output_api.PresubmitPromptWarning( 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 [] return []
@ -162,7 +162,7 @@ def CheckSvnModifiedDirectories(input_api, output_api, source_file_filter=None):
else: else:
error_type = output_api.PresubmitNotifyResult error_type = output_api.PresubmitNotifyResult
errors.append(error_type( errors.append(error_type(
"Potential accidental commits in changelist %s:" % f.LocalPath(), 'Potential accidental commits in changelist %s:' % f.LocalPath(),
items=bad_files)) items=bad_files))
return errors 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): for f in input_api.AffectedSourceFiles(source_file_filter):
contents = input_api.ReadFile(f, 'rb') contents = input_api.ReadFile(f, 'rb')
# Check that the file ends in one and only one newline character. # 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()) eof_files.append(f.LocalPath())
if eof_files: if eof_files:
@ -196,12 +196,12 @@ def CheckChangeHasNoCrAndHasOnlyOneEol(input_api, output_api,
if '\r' in contents: if '\r' in contents:
cr_files.append(f.LocalPath()) cr_files.append(f.LocalPath())
# Check that the file ends in one and only one newline character. # 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()) eof_files.append(f.LocalPath())
outputs = [] outputs = []
if cr_files: if cr_files:
outputs.append(output_api.PresubmitPromptWarning( 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: if eof_files:
outputs.append(output_api.PresubmitPromptWarning( outputs.append(output_api.PresubmitPromptWarning(
'These files should end in one (and only one) newline character:', '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 = [] tabs = []
for f, line_num, line in input_api.RightHandSideLines(source_file_filter): for f, line_num, line in input_api.RightHandSideLines(source_file_filter):
if '\t' in line: if '\t' in line:
tabs.append("%s, line %s" % (f.LocalPath(), line_num)) tabs.append('%s, line %s' % (f.LocalPath(), line_num))
if tabs: if tabs:
return [output_api.PresubmitPromptWarning("Found a tab character in:", return [output_api.PresubmitPromptWarning('Found a tab character in:',
long_text="\n".join(tabs))] long_text='\n'.join(tabs))]
return [] return []
@ -229,11 +229,11 @@ def CheckChangeHasNoStrayWhitespace(input_api, output_api,
errors = [] errors = []
for f, line_num, line in input_api.RightHandSideLines(source_file_filter): for f, line_num, line in input_api.RightHandSideLines(source_file_filter):
if line.rstrip() != line: if line.rstrip() != line:
errors.append("%s, line %s" % (f.LocalPath(), line_num)) errors.append('%s, line %s' % (f.LocalPath(), line_num))
if errors: if errors:
return [output_api.PresubmitPromptWarning( return [output_api.PresubmitPromptWarning(
"Found line ending with white spaces in:", 'Found line ending with white spaces in:',
long_text="\n".join(errors))] long_text='\n'.join(errors))]
return [] return []
@ -260,7 +260,7 @@ def CheckLongLines(input_api, output_api, maxlen=80, source_file_filter=None):
break break
if bad: 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)] return [output_api.PresubmitPromptWarning(msg, items=bad)]
else: else:
return [] return []
@ -281,7 +281,7 @@ def CheckLicense(input_api, output_api, license, source_file_filter=None):
else: else:
res_type = output_api.PresubmitNotifyResult res_type = output_api.PresubmitNotifyResult
return [res_type( 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 [] return []
@ -327,7 +327,7 @@ def CheckSvnProperty(input_api, output_api, prop, expected, affected_files):
res_type = output_api.PresubmitError res_type = output_api.PresubmitError
else: else:
res_type = output_api.PresubmitNotifyResult 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 [res_type(message, items=bad)]
return [] return []
@ -344,14 +344,15 @@ def CheckDoNotSubmit(input_api, output_api):
def CheckTreeIsOpen(input_api, output_api, url, closed): def CheckTreeIsOpen(input_api, output_api, url, closed):
"""Checks that an url's content doesn't match a regexp that would mean that """Checks that an url's content doesn't match a regexp that would mean that
the tree is closed.""" the tree is closed."""
assert(input_api.is_committing) if not input_api.is_committing:
return []
try: try:
connection = input_api.urllib2.urlopen(url) connection = input_api.urllib2.urlopen(url)
status = connection.read() status = connection.read()
connection.close() connection.close()
if input_api.re.match(closed, status): if input_api.re.match(closed, status):
long_text = status + '\n' + url 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)] long_text=long_text)]
except IOError: except IOError:
pass pass
@ -375,7 +376,7 @@ def RunPythonUnitTests(input_api, output_api, unit_tests):
cwd = None cwd = None
env = None env = None
unit_test_name = unit_test 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. # directory instead.
if '.' in unit_test: if '.' in unit_test:
# Tests imported in submodules (subdirectories) assume that the current # 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( subproc = input_api.subprocess.Popen(
[ [
input_api.python_executable, input_api.python_executable,
"-m", '-m',
"%s" % unit_test '%s' % unit_test
], ],
cwd=cwd, cwd=cwd,
env=env, env=env,
@ -405,9 +406,92 @@ def RunPythonUnitTests(input_api, output_api, unit_tests):
stdoutdata, stderrdata = subproc.communicate() stdoutdata, stderrdata = subproc.communicate()
# Discard the output if returncode == 0 # Discard the output if returncode == 0
if subproc.returncode: 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)) unit_test_name, subproc.returncode, stdoutdata, stderrdata))
if outputs: 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))] long_text='\n'.join(outputs))]
return [] 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 []

78
tests/presubmit_unittest.py
Unescape Escape View File

@ -1078,6 +1078,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
'CheckDoNotSubmit', 'CheckDoNotSubmit',
'CheckDoNotSubmitInDescription', 'CheckDoNotSubmitInFiles', 'CheckDoNotSubmitInDescription', 'CheckDoNotSubmitInFiles',
'CheckLongLines', 'CheckTreeIsOpen', 'RunPythonUnitTests', 'CheckLongLines', 'CheckTreeIsOpen', 'RunPythonUnitTests',
'CheckBuildbotPendingBuilds', 'CheckRietveldTryJobExecution',
] ]
# If this test fails, you should add the relevant test. # If this test fails, you should add the relevant test.
self.compareMembers(presubmit_canned_checks, members) self.compareMembers(presubmit_canned_checks, members)
@ -1528,6 +1529,83 @@ class CannedChecksUnittest(PresubmitTestsBase):
input_api, presubmit.OutputApi, ['test_module']) input_api, presubmit.OutputApi, ['test_module'])
self.assertEquals(len(results), 0) self.assertEquals(len(results), 0)
def testCheckRietveldTryJobExecutionBad(self):
change = self.mox.CreateMock(presubmit.SvnChange)
change.scm = 'svn'
change.issue = 2
change.patchset = 5
input_api = self.MockInputApi(change, True)
connection = self.mox.CreateMockAnything()
input_api.urllib2.urlopen('uurl/2/get_build_results/5').AndReturn(
connection)
connection.read().AndReturn('foo')
connection.close()
self.mox.ReplayAll()
results = presubmit_canned_checks.CheckRietveldTryJobExecution(
input_api, presubmit.OutputApi, 'uurl', ('mac', 'linux'), 'georges')
self.assertEquals(len(results), 1)
self.assertEquals(results[0].__class__,
presubmit.OutputApi.PresubmitNotifyResult)
def testCheckRietveldTryJobExecutionGood(self):
change = self.mox.CreateMock(presubmit.SvnChange)
change.scm = 'svn'
change.issue = 2
change.patchset = 5
input_api = self.MockInputApi(change, True)
connection = self.mox.CreateMockAnything()
input_api.urllib2.urlopen('uurl/2/get_build_results/5').AndReturn(
connection)
connection.read().AndReturn("""amiga|Foo|blah
linux|failure|bleh
mac|success|blew
""")
connection.close()
self.mox.ReplayAll()
results = presubmit_canned_checks.CheckRietveldTryJobExecution(
input_api, presubmit.OutputApi, 'uurl', ('mac', 'linux', 'amiga'),
'georges')
self.assertEquals(len(results), 1)
self.assertEquals(results[0].__class__,
presubmit.OutputApi.PresubmitPromptWarning)
def testCheckBuildbotPendingBuildsBad(self):
input_api = self.MockInputApi(None, True)
input_api.json = presubmit.json
connection = self.mox.CreateMockAnything()
input_api.urllib2.urlopen('uurl').AndReturn(connection)
connection.read().AndReturn('foo')
connection.close()
self.mox.ReplayAll()
results = presubmit_canned_checks.CheckBuildbotPendingBuilds(
input_api, presubmit.OutputApi, 'uurl', 2, ('foo'))
self.assertEquals(len(results), 1)
self.assertEquals(results[0].__class__,
presubmit.OutputApi.PresubmitNotifyResult)
def testCheckBuildbotPendingBuildsGood(self):
input_api = self.MockInputApi(None, True)
input_api.json = presubmit.json
connection = self.mox.CreateMockAnything()
input_api.urllib2.urlopen('uurl').AndReturn(connection)
connection.read().AndReturn("""
{
'b1': { 'pending_builds': [0, 1, 2, 3, 4, 5, 6, 7] },
'foo': { 'pending_builds': [0, 1, 2, 3, 4, 5, 6, 7] },
'b2': { 'pending_builds': [0] }
}""")
connection.close()
self.mox.ReplayAll()
results = presubmit_canned_checks.CheckBuildbotPendingBuilds(
input_api, presubmit.OutputApi, 'uurl', 2, ('foo'))
self.assertEquals(len(results), 1)
self.assertEquals(results[0].__class__,
presubmit.OutputApi.PresubmitNotifyResult)
if __name__ == '__main__': if __name__ == '__main__':
import unittest import unittest

Write Preview
Loading…
Cancel
Save