diff --git a/PRESUBMIT.py b/PRESUBMIT.py index fcac22fe8b..37047d47ce 100755 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -620,9 +620,7 @@ def CommonChecks(input_api, output_api): results.extend(RunPythonTests(input_api, output_api)) results.extend(CheckUsageOfGoogleProtobufNamespace(input_api, output_api)) results.extend(CheckOrphanHeaders(input_api, output_api)) - # TODO(mbonadei): check before re-enable because it seems it is reporting - # some false positives. - # results.extend(CheckNewLineAtTheEndOfProtoFiles(input_api, output_api)) + results.extend(CheckNewlineAtTheEndOfProtoFiles(input_api, output_api)) return results @@ -681,7 +679,7 @@ def CheckOrphanHeaders(input_api, output_api): return results -def CheckNewLineAtTheEndOfProtoFiles(input_api, output_api): +def CheckNewlineAtTheEndOfProtoFiles(input_api, output_api): """Checks that all .proto files are terminated with a newline.""" error_msg = 'File {} must end with exactly one newline.' results = [] @@ -691,6 +689,6 @@ def CheckNewLineAtTheEndOfProtoFiles(input_api, output_api): file_path = f.LocalPath() with open(file_path) as f: lines = f.readlines() - if lines[-1] != '\n' or lines[-2] == '\n': + if len(lines) > 0 and not lines[-1].endswith('\n'): results.append(output_api.PresubmitError(error_msg.format(file_path))) return results diff --git a/presubmit_test.py b/presubmit_test.py index c88beaea19..86465555a5 100755 --- a/presubmit_test.py +++ b/presubmit_test.py @@ -8,10 +8,14 @@ # in the file PATENTS. All contributing project authors may # be found in the AUTHORS file in the root of the source tree. +import os +import shutil +import tempfile +import textwrap import unittest import PRESUBMIT -from presubmit_test_mocks import MockInputApi, MockOutputApi +from presubmit_test_mocks import MockInputApi, MockOutputApi, MockFile class CheckBugEntryField(unittest.TestCase): @@ -44,5 +48,49 @@ class CheckBugEntryField(unittest.TestCase): self.assertEqual(0, len(errors)) +class CheckNewlineAtTheEndOfProtoFiles(unittest.TestCase): + + def setUp(self): + self.tmp_dir = tempfile.mkdtemp() + self.proto_file_path = os.path.join(self.tmp_dir, 'foo.proto') + self.input_api = MockInputApi() + self.output_api = MockOutputApi() + + def tearDown(self): + shutil.rmtree(self.tmp_dir, ignore_errors=True) + + def testErrorIfProtoFileDoesNotEndWithNewline(self): + self.__GenerateProtoWithoutNewlineAtTheEnd() + self.input_api.files = [MockFile(self.proto_file_path)] + errors = PRESUBMIT.CheckNewlineAtTheEndOfProtoFiles(self.input_api, + self.output_api) + self.assertEqual(1, len(errors)) + self.assertEqual( + 'File %s must end with exactly one newline.' % self.proto_file_path, + str(errors[0])) + + def testNoErrorIfProtoFileEndsWithNewline(self): + self.__GenerateProtoWithNewlineAtTheEnd() + self.input_api.files = [MockFile(self.proto_file_path)] + errors = PRESUBMIT.CheckNewlineAtTheEndOfProtoFiles(self.input_api, + self.output_api) + self.assertEqual(0, len(errors)) + + def __GenerateProtoWithNewlineAtTheEnd(self): + with open(self.proto_file_path, 'w') as f: + f.write(textwrap.dedent(""" + syntax = "proto2"; + option optimize_for = LITE_RUNTIME; + package webrtc.audioproc; + """)) + + def __GenerateProtoWithoutNewlineAtTheEnd(self): + with open(self.proto_file_path, 'w') as f: + f.write(textwrap.dedent(""" + syntax = "proto2"; + option optimize_for = LITE_RUNTIME; + package webrtc.audioproc;""")) + + if __name__ == '__main__': unittest.main() diff --git a/presubmit_test_mocks.py b/presubmit_test_mocks.py index 2f86449530..f7ead25fab 100644 --- a/presubmit_test_mocks.py +++ b/presubmit_test_mocks.py @@ -6,6 +6,9 @@ # in the file PATENTS. All contributing project authors may # be found in the AUTHORS file in the root of the source tree. +# This file is inspired to [1]. +# [1] - https://cs.chromium.org/chromium/src/PRESUBMIT_test_mocks.py + class MockInputApi(object): """Mock class for the InputApi class. @@ -16,6 +19,11 @@ class MockInputApi(object): def __init__(self): self.change = MockChange([]) + self.files = [] + + def AffectedSourceFiles(self, file_filter=None): + # pylint: disable=unused-argument + return self.files class MockOutputApi(object): @@ -39,6 +47,7 @@ class MockOutputApi(object): MockOutputApi.PresubmitResult.__init__(self, message, items, long_text) self.type = 'error' + class MockChange(object): """Mock class for Change class. @@ -48,3 +57,17 @@ class MockChange(object): def __init__(self, changed_files): self._changed_files = changed_files + + +class MockFile(object): + """Mock class for the File class. + + This class can be used to form the mock list of changed files in + MockInputApi for presubmit unittests. + """ + + def __init__(self, local_path): + self._local_path = local_path + + def LocalPath(self): + return self._local_path