From e9b71c9b3a1d5b1d9deb70d889e13bc55ca6a70e Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Wed, 10 Jun 2009 18:10:01 +0000 Subject: [PATCH] Add CheckChangeHasOnlyOneEol and CheckChangeHasNoCrAndHasOnlyOneEol. TEST=unit tests BUG=none Review URL: http://codereview.chromium.org/118505 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@18075 0039d316-1c4b-4281-b951-d872f2087c98 --- presubmit_canned_checks.py | 62 ++++++++++++++++++++++++++++----- tests/presubmit_unittest.py | 68 ++++++++++++++++++++++++------------- 2 files changed, 97 insertions(+), 33 deletions(-) diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 6b3a4eed2..d6f8a34fc 100755 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -67,14 +67,56 @@ def CheckDoNotSubmitInFiles(input_api, output_api): def CheckChangeHasNoCR(input_api, output_api, source_file_filter=None): - """Checks that there are no \r, \r\n (CR or CRLF) characters in any of the - source files to be submitted. - """ - outputs = [] + """Checks no '\r' (CR) character is in any source files.""" + cr_files = [] for f in input_api.AffectedSourceFiles(source_file_filter): if '\r' in input_api.ReadFile(f, 'rb'): - outputs.append(output_api.PresubmitPromptWarning( - "Found a CR character in %s" % f.LocalPath())) + cr_files.append(f.LocalPath()) + if cr_files: + return [output_api.PresubmitPromptWarning( + "Found a CR character in these files:", items=cr_files)] + return [] + + +def CheckChangeHasOnlyOneEol(input_api, output_api, source_file_filter=None): + """Checks the files ends with one and only one \n (LF).""" + eof_files = [] + 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"): + eof_files.append(f.LocalPath()) + + if eof_files: + return [output_api.PresubmitPromptWarning( + 'These files should end in one (and only one) newline character:', + items=eof_files)] + return [] + + +def CheckChangeHasNoCrAndHasOnlyOneEol(input_api, output_api, + source_file_filter=None): + """Runs both CheckChangeHasNoCR and CheckChangeHasOnlyOneEOL in one pass. + + It is faster because it is reading the file only once. + """ + cr_files = [] + eof_files = [] + for f in input_api.AffectedSourceFiles(source_file_filter): + contents = input_api.ReadFile(f, 'rb') + 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"): + eof_files.append(f.LocalPath()) + outputs = [] + if cr_files: + outputs.append(output_api.PresubmitPromptWarning( + "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:', + items=eof_files)) return outputs @@ -82,11 +124,13 @@ def CheckChangeHasNoTabs(input_api, output_api, source_file_filter=None): """Checks that there are no tab characters in any of the text files to be submitted. """ + tabs = [] for f, line_num, line in input_api.RightHandSideLines(source_file_filter): if '\t' in line: - return [output_api.PresubmitPromptWarning( - "Found a tab character in %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 [] diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index aeeeaaaf8..e8598bf54 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -937,7 +937,9 @@ class CannedChecksUnittest(PresubmitTestsBase): def testMembersChanged(self): self.mox.ReplayAll() members = [ - 'CheckChangeHasBugField', 'CheckChangeHasNoCR', 'CheckChangeHasNoTabs', + 'CheckChangeHasBugField', 'CheckChangeHasOnlyOneEol', + 'CheckChangeHasNoCR', 'CheckChangeHasNoCrAndHasOnlyOneEol', + 'CheckChangeHasNoTabs', 'CheckChangeHasQaField', 'CheckChangeHasTestedField', 'CheckChangeHasTestField', 'CheckChangeSvnEolStyle', 'CheckDoNotSubmit', @@ -987,6 +989,28 @@ class CannedChecksUnittest(PresubmitTestsBase): self.assertEquals(len(results2), 1) self.assertEquals(results2[0].__class__, error_type) + def ReadFileTest(self, check, content1, content2, error_type): + input_api1 = self.MockInputApi() + self.mox.StubOutWithMock(input_api1, 'ReadFile') + input_api1.change = self.MakeBasicChange('foo', 'Foo\n') + affected_file1 = self.mox.CreateMock(presubmit.SvnAffectedFile) + input_api1.AffectedSourceFiles(None).AndReturn([affected_file1]) + input_api1.ReadFile(affected_file1, 'rb').AndReturn(content1) + input_api2 = self.MockInputApi() + self.mox.StubOutWithMock(input_api2, 'ReadFile') + input_api2.change = self.MakeBasicChange('foo', 'Foo\n') + affected_file2 = self.mox.CreateMock(presubmit.SvnAffectedFile) + input_api2.AffectedSourceFiles(None).AndReturn([affected_file2]) + input_api2.ReadFile(affected_file2, 'rb').AndReturn(content2) + affected_file2.LocalPath().AndReturn('bar.cc') + self.mox.ReplayAll() + + results = check(input_api1, presubmit.OutputApi) + self.assertEquals(results, []) + results2 = check(input_api2, presubmit.OutputApi) + self.assertEquals(len(results2), 1) + self.assertEquals(results2[0].__class__, error_type) + def testCannedCheckChangeHasBugField(self): self.DescriptionTest(presubmit_canned_checks.CheckChangeHasBugField, 'BUG=1234', '', @@ -1018,31 +1042,27 @@ class CannedChecksUnittest(PresubmitTestsBase): 'DO NOTSUBMIT', 'DO NOT ' + 'SUBMIT', presubmit.OutputApi.PresubmitError) - def testCheckChangeHasNoCR(self): - input_api1 = self.MockInputApi() - self.mox.StubOutWithMock(input_api1, 'ReadFile') - input_api1.change = self.MakeBasicChange('foo', 'Foo\n') - affected_file1 = self.mox.CreateMock(presubmit.SvnAffectedFile) - input_api1.AffectedSourceFiles(None).AndReturn([affected_file1]) - input_api1.ReadFile(affected_file1, 'rb').AndReturn("Hey!\nHo!\n") - input_api2 = self.MockInputApi() - self.mox.StubOutWithMock(input_api2, 'ReadFile') - input_api2.change = self.MakeBasicChange('foo', 'Foo\n') - affected_file2 = self.mox.CreateMock(presubmit.SvnAffectedFile) - input_api2.AffectedSourceFiles(None).AndReturn([affected_file2]) - input_api2.ReadFile(affected_file2, 'rb').AndReturn("Hey!\r\nHo!\r\n") - affected_file2.LocalPath().AndReturn('bar.cc') - self.mox.ReplayAll() + def testCheckChangeHasOnlyOneEol(self): + self.ReadFileTest(presubmit_canned_checks.CheckChangeHasOnlyOneEol, + "Hey!\nHo!\n", "Hey!\nHo!\n\n", + presubmit.OutputApi.PresubmitPromptWarning) - results = presubmit_canned_checks.CheckChangeHasNoCR( - input_api1, presubmit.OutputApi, None) - self.assertEquals(results, []) - results2 = presubmit_canned_checks.CheckChangeHasNoCR( - input_api2, presubmit.OutputApi, None) - self.assertEquals(len(results2), 1) - self.assertEquals(results2[0].__class__, + def testCheckChangeHasNoCR(self): + self.ReadFileTest(presubmit_canned_checks.CheckChangeHasNoCR, + "Hey!\nHo!\n", "Hey!\r\nHo!\r\n", presubmit.OutputApi.PresubmitPromptWarning) - + + def testCheckChangeHasNoCrAndHasOnlyOneEol(self): + self.ReadFileTest( + presubmit_canned_checks.CheckChangeHasNoCrAndHasOnlyOneEol, + "Hey!\nHo!\n", "Hey!\nHo!\n\n", + presubmit.OutputApi.PresubmitPromptWarning) + self.mox.VerifyAll() + self.ReadFileTest( + presubmit_canned_checks.CheckChangeHasNoCrAndHasOnlyOneEol, + "Hey!\nHo!\n", "Hey!\r\nHo!\r\n", + presubmit.OutputApi.PresubmitPromptWarning) + def testCannedCheckChangeHasNoTabs(self): self.ContentTest(presubmit_canned_checks.CheckChangeHasNoTabs, 'blah blah', 'blah\tblah',