Skip to content

C
Chromium_Depot_tools
Commit bfffd45e authored by bulach@chromium.org's avatar bulach@chromium.org
Browse files
Options

Presubmit tests: allow 100columns limit for .java files. 

Chrome for Android introduced the requirement for java files.
Android's style guide:
http://source.android.com/source/code-style.html#limit-line-length
defines 100 columns for .java files.
This patch changes the presubmit tests which are also used by chromium's CQ.

TEST=testCannedCheckJavaLongLines

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@122938 0039d316-1c4b-4281-b951-d872f2087c98
parent 8fab6b66
Hide whitespace changes
Inline Side-by-side
Showing with 60 additions and 33 deletions
presubmit_canned_checks.py
View file @ bfffd45e
......@@ -79,7 +79,7 @@ def CheckDoNotSubmitInFiles(input_api, output_api):
# We want to check every text file, not just source files.
file_filter = lambda x : x
keyword = 'DO NOT ' + 'SUBMIT'
errors = _FindNewViolationsOfRule(lambda line : keyword not in line,
errors = _FindNewViolationsOfRule(lambda _, line : keyword not in line,
input_api, file_filter)
text = '\n'.join('Found %s in %s' % (keyword, loc) for loc in errors)
if text:
......@@ -234,8 +234,8 @@ def _FindNewViolationsOfRule(callable_rule, input_api, source_file_filter=None,
"""Find all newly introduced violations of a per-line rule (a callable).
Arguments:
callable_rule: a callable taking a line of input and returning True
if the rule is satisfied and False if there was a problem.
callable_rule: a callable taking a file extension and line of input and
returning True if the rule is satisfied and False if there was a problem.
input_api: object to enumerate the affected files.
source_file_filter: a filter to be passed to the input api.
error_formatter: a callable taking (filename, line_number, line) and
......@@ -251,11 +251,12 @@ def _FindNewViolationsOfRule(callable_rule, input_api, source_file_filter=None,
# to the SCM to determine the changed region can be quite expensive on
# Win32. Assuming that most files will be kept problem-free, we can
# skip the SCM operations most of the time.
if all(callable_rule(line) for line in f.NewContents()):
extension = str(f.LocalPath()).rsplit('.', 1)[-1]
if all(callable_rule(extension, line) for line in f.NewContents()):
continue # No violation found in full text: can skip considering diff.
for line_num, line in f.ChangedContents():
if not callable_rule(line):
if not callable_rule(extension, line):
errors.append(error_formatter(f.LocalPath(), line_num, line))
return errors
......@@ -274,7 +275,7 @@ def CheckChangeHasNoTabs(input_api, output_api, source_file_filter=None):
('Makefile', 'makefile') and
source_file_filter(affected_file))
tabs = _FindNewViolationsOfRule(lambda line : '\t' not in line,
tabs = _FindNewViolationsOfRule(lambda _, line : '\t' not in line,
input_api, filter_more)
if tabs:
......@@ -287,7 +288,7 @@ def CheckChangeTodoHasOwner(input_api, output_api, source_file_filter=None):
"""Checks that the user didn't add TODO(name) without an owner."""
unowned_todo = input_api.re.compile('TO' + 'DO[^(]')
errors = _FindNewViolationsOfRule(lambda x : not unowned_todo.search(x),
errors = _FindNewViolationsOfRule(lambda _, x : not unowned_todo.search(x),
input_api, source_file_filter)
errors = ['Found TO' + 'DO with no owner in ' + x for x in errors]
if errors:
......@@ -298,7 +299,7 @@ def CheckChangeTodoHasOwner(input_api, output_api, source_file_filter=None):
def CheckChangeHasNoStrayWhitespace(input_api, output_api,
source_file_filter=None):
"""Checks that there is no stray whitespace at source lines end."""
errors = _FindNewViolationsOfRule(lambda line : line.rstrip() == line,
errors = _FindNewViolationsOfRule(lambda _, line : line.rstrip() == line,
input_api, source_file_filter)
if errors:
return [output_api.PresubmitPromptWarning(
......@@ -311,18 +312,25 @@ def CheckLongLines(input_api, output_api, maxlen=80, source_file_filter=None):
"""Checks that there aren't any lines longer than maxlen characters in any of
the text files to be submitted.
"""
# Stupidly long symbols that needs to be worked around if takes 66% of line.
long_symbol = maxlen * 2 / 3
# Hard line length limit at 50% more.
extra_maxlen = maxlen * 3 / 2
maxlens = {
'java': 100,
'': maxlen,
}
# Note: these are C++ specific but processed on all languages. :(
MACROS = ('#define', '#include', '#import', '#pragma', '#if', '#endif')
def no_long_lines(line):
if len(line) <= maxlen:
def no_long_lines(file_extension, line):
file_maxlen = maxlens.get(file_extension, maxlens[''])
# Stupidly long symbols that needs to be worked around if takes 66% of line.
long_symbol = file_maxlen * 2 / 3
# Hard line length limit at 50% more.
extra_maxlen = file_maxlen * 3 / 2
line_len = len(line)
if line_len <= file_maxlen:
return True
if len(line) > extra_maxlen:
if line_len > extra_maxlen:
return False
return (
......
tests/presubmit_unittest.py
View file @ bfffd45e
......@@ -1498,13 +1498,16 @@ class CannedChecksUnittest(PresubmitTestsBase):
self.assertEquals(len(results2), 1)
self.assertEquals(results2[0].__class__, error_type)
def ContentTest(self, check, content1, content2, error_type):
def ContentTest(self, check, content1, content1_path, content2,
content2_path, error_type):
"""Runs a test of a content-checking rule.
Args:
check: the check to run.
content1: content which is expected to pass the check.
content1_path: file path for content1.
content2: content which is expected to fail the check.
content2_path: file path for content2.
error_type: the type of the error expected for content2.
"""
change1 = presubmit.Change(
......@@ -1514,12 +1517,13 @@ class CannedChecksUnittest(PresubmitTestsBase):
input_api1.AffectedFiles(
include_deletes=False,
file_filter=mox.IgnoreArg()).AndReturn([affected_file])
affected_file.LocalPath().AndReturn(content1_path)
affected_file.NewContents().AndReturn([
'ahoy',
'afoo',
content1,
'hay',
'yer',
'ya'])
'bfoo',
'cfoo',
'dfoo'])
change2 = presubmit.Change(
'foo2', 'foo2\n', self.fake_root_dir, None, 0, 0, None)
......@@ -1528,19 +1532,20 @@ class CannedChecksUnittest(PresubmitTestsBase):
input_api2.AffectedFiles(
include_deletes=False,
file_filter=mox.IgnoreArg()).AndReturn([affected_file])
affected_file.LocalPath().AndReturn(content2_path)
affected_file.NewContents().AndReturn([
'ahoy',
'dfoo',
content2,
'hay',
'yer',
'ya'])
'efoo',
'ffoo',
'gfoo'])
# It falls back to ChangedContents when there is a failure. This is an
# optimization since NewContents() is much faster to execute than
# ChangedContents().
affected_file.ChangedContents().AndReturn([
(42, content2),
(43, 'yer'),
(23, 'ya')])
(43, 'hfoo'),
(23, 'ifoo')])
affected_file.LocalPath().AndReturn('foo.cc')
self.mox.ReplayAll()
......@@ -1663,14 +1668,14 @@ class CannedChecksUnittest(PresubmitTestsBase):
def testCannedCheckDoNotSubmitInFiles(self):
self.ContentTest(
lambda x,y,z: presubmit_canned_checks.CheckDoNotSubmitInFiles(x, y),
'DO NOTSUBMIT', 'DO NOT ' + 'SUBMIT',
'DO NOTSUBMIT', None, 'DO NOT ' + 'SUBMIT', None,
presubmit.OutputApi.PresubmitError)
def testCheckChangeHasNoStrayWhitespace(self):
self.ContentTest(
lambda x,y,z:
presubmit_canned_checks.CheckChangeHasNoStrayWhitespace(x, y),
'Foo', 'Foo ',
'Foo', None, 'Foo ', None,
presubmit.OutputApi.PresubmitPromptWarning)
def testCheckChangeHasOnlyOneEol(self):
......@@ -1696,12 +1701,12 @@ class CannedChecksUnittest(PresubmitTestsBase):
def testCheckChangeTodoHasOwner(self):
self.ContentTest(presubmit_canned_checks.CheckChangeTodoHasOwner,
"TODO(foo): bar", "TODO: bar",
"TODO(foo): bar", None, "TODO: bar", None,
presubmit.OutputApi.PresubmitPromptWarning)
def testCannedCheckChangeHasNoTabs(self):
self.ContentTest(presubmit_canned_checks.CheckChangeHasNoTabs,
'blah blah', 'blah\tblah',
'blah blah', None, 'blah\tblah', None,
presubmit.OutputApi.PresubmitPromptWarning)
# Make sure makefiles are ignored.
......@@ -1716,8 +1721,10 @@ class CannedChecksUnittest(PresubmitTestsBase):
affected_file3.LocalPath().AndReturn('makefile')
# Only this one will trigger.
affected_file4 = self.mox.CreateMock(presubmit.SvnAffectedFile)
affected_file4.LocalPath().AndReturn('makefile.foo')
affected_file1.LocalPath().AndReturn('foo.cc')
affected_file1.NewContents().AndReturn(['yo, '])
affected_file4.LocalPath().AndReturn('makefile.foo')
affected_file4.LocalPath().AndReturn('makefile.foo')
affected_file4.NewContents().AndReturn(['ye\t'])
affected_file4.ChangedContents().AndReturn([(46, 'ye\t')])
affected_file4.LocalPath().AndReturn('makefile.foo')
......@@ -1744,12 +1751,17 @@ class CannedChecksUnittest(PresubmitTestsBase):
def testCannedCheckLongLines(self):
check = lambda x, y, z: presubmit_canned_checks.CheckLongLines(x, y, 10, z)
self.ContentTest(check, '0123456789', '01234567890',
self.ContentTest(check, '0123456789', None, '01234567890', None,
presubmit.OutputApi.PresubmitPromptWarning)
def testCannedCheckJavaLongLines(self):
check = lambda x, y, _: presubmit_canned_checks.CheckLongLines(x, y)
self.ContentTest(check, 'A ' * 50, 'foo.java', 'A ' * 50 + 'B', 'foo.java',
presubmit.OutputApi.PresubmitPromptWarning)
def testCannedCheckLongLinesLF(self):
check = lambda x, y, z: presubmit_canned_checks.CheckLongLines(x, y, 10, z)
self.ContentTest(check, '012345678\n', '0123456789\n',
self.ContentTest(check, '012345678\n', None, '0123456789\n', None,
presubmit.OutputApi.PresubmitPromptWarning)
def testCannedCheckLongLinesMacro(self):
......@@ -1758,7 +1770,9 @@ class CannedChecksUnittest(PresubmitTestsBase):
check,
# Put a space in so it doesn't trigger long symbols. Allow 1/3 more.
'#if 56 89 12 45',
None,
'#if 56 89 12 456',
None,
presubmit.OutputApi.PresubmitPromptWarning)
def testCannedCheckLongLinesHttp(self):
......@@ -1766,7 +1780,9 @@ class CannedChecksUnittest(PresubmitTestsBase):
self.ContentTest(
check,
' http:// 0 23 5',
None,
' http:// 0 23 56',
None,
presubmit.OutputApi.PresubmitPromptWarning)
def testCannedCheckLongLinesLongSymbol(self):
......@@ -1774,7 +1790,9 @@ class CannedChecksUnittest(PresubmitTestsBase):
self.ContentTest(
check,
' TUP5D_LoNG_SY ',
None,
' TUP5D_LoNG_SY5 ',
None,
presubmit.OutputApi.PresubmitPromptWarning)
def testCheckChangeSvnEolStyleCommit(self):
......@@ -2257,6 +2275,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
for _ in range(3):
input_api.AffectedFiles(file_filter=mox.IgnoreArg(), include_deletes=False
).AndReturn([affected_file])
affected_file.LocalPath()
affected_file.NewContents().AndReturn('Hey!\nHo!\nHey!\nHo!\n\n')
affected_file.ChangedContents().AndReturn([
(0, 'Hey!\n'),
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment