From 067ef5d8941a59ca176f5ee63bb02d0e3df947c4 Mon Sep 17 00:00:00 2001
From: Edward Lesmes <ehmaldonado@chromium.org>
Date: Thu, 19 Apr 2018 17:54:45 -0400
Subject: [PATCH] Add support for checking that OWNERS files are correctly
 formatted.

Bug: 789773
Change-Id: I4c6c676a821fad33a34ef6c46468af95cdf6c0ec
Reviewed-on: https://chromium-review.googlesource.com/1020073
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
---
 presubmit_canned_checks.py  | 16 ++++++++++++++
 tests/presubmit_unittest.py | 43 +++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py
index 5af5056ce..710a8d966 100644
--- a/presubmit_canned_checks.py
+++ b/presubmit_canned_checks.py
@@ -826,6 +826,22 @@ def CheckBuildbotPendingBuilds(input_api, output_api, url, max_pendings,
   return []
 
 
+def CheckOwnersFormat(input_api, output_api):
+  affected_files = set([
+      f.LocalPath()
+      for f in input_api.change.AffectedFiles()
+      if 'OWNERS' in f.LocalPath() and f.Action() != 'D'
+  ])
+  if not affected_files:
+    return []
+  try:
+    input_api.owners_db.load_data_needed_for(affected_files)
+    return []
+  except Exception as e:
+    return [output_api.PresubmitError(
+        'Error parsing OWNERS files:\n%s' % e)]
+
+
 def CheckOwners(input_api, output_api, source_file_filter=None):
   affected_files = set([f.LocalPath() for f in
       input_api.change.AffectedFiles(file_filter=source_file_filter)])
diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py
index a37b36650..19e92315f 100755
--- a/tests/presubmit_unittest.py
+++ b/tests/presubmit_unittest.py
@@ -1726,6 +1726,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
       'CheckLongLines', 'CheckTreeIsOpen', 'PanProjectChecks',
       'CheckLicense',
       'CheckOwners',
+      'CheckOwnersFormat',
       'CheckPatchFormatted',
       'CheckGNFormatted',
       'CheckSingletonInHeaders',
@@ -2342,6 +2343,48 @@ class CannedChecksUnittest(PresubmitTestsBase):
     self.assertEquals(results[0].__class__,
         presubmit.OutputApi.PresubmitNotifyResult)
 
+  def GetInputApiWithOWNERS(self, owners_content):
+    affected_file = self.mox.CreateMock(presubmit.GitAffectedFile)
+    affected_file.LocalPath = lambda: 'OWNERS'
+    affected_file.Action = lambda: 'M'
+
+    change = self.mox.CreateMock(presubmit.Change)
+    change.AffectedFiles = lambda: [affected_file]
+
+    input_api = self.MockInputApi(None, False)
+    input_api.change = change
+
+    os.path.exists = lambda _: True
+
+    owners_file = presubmit.cStringIO.StringIO(owners_content)
+    fopen = lambda *args: owners_file
+
+    input_api.owners_db = owners.Database('', fopen, os.path)
+
+    return input_api
+
+  def testCheckOwnersFormatWorks(self):
+    input_api = self.GetInputApiWithOWNERS('\n'.join([
+        'set noparent',
+        'per-file lalala = lemur@chromium.org',
+    ]))
+    self.assertEqual(
+        [],
+        presubmit_canned_checks.CheckOwnersFormat(
+            input_api, presubmit.OutputApi)
+    )
+
+  def testCheckOwnersFormatFails(self):
+    input_api = self.GetInputApiWithOWNERS('\n'.join([
+        'set noparent',
+        'invalid format',
+    ]))
+    results = presubmit_canned_checks.CheckOwnersFormat(
+        input_api, presubmit.OutputApi)
+
+    self.assertEqual(1, len(results))
+    self.assertIsInstance(results[0], presubmit.OutputApi.PresubmitError)
+
   def AssertOwnersWorks(self, tbr=False, issue='1', approvers=None,
       reviewers=None, is_committing=True,
       response=None, uncovered_files=None, expected_output='',