Enabling and fixing CheckNewlineAtTheEndOfProtoFiles

This check has been skipped during the migration from src/webrtc to
src. It was also reporting false positives. Now it should be fixed.

NOTRY=True

Bug: chromium:611808
Change-Id: Id8567dd92099e75ac35351f053829deebf28a9d1
Reviewed-on: https://webrtc-review.googlesource.com/1580
Reviewed-by: Henrik Kjellander <kjellander@webrtc.org>
Reviewed-by: Henrik Kjellander <kjellander@google.com>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#19887}

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

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()

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