[owners] Validate owners config in Depot Tools client.

Change-Id: Ia9cb19d2b1f776c41ef7e96e6a0ebfb0fcd41344
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2548029
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Anthony Polito <apolito@google.com>
Reviewed-by: Anthony Polito <apolito@google.com>

owners_client.py

@@ -14,6 +14,10 @@ PENDING = 'PENDING'
 INSUFFICIENT_REVIEWERS = 'INSUFFICIENT_REVIEWERS'
 
 
+class InvalidOwnersConfig(Exception):
+  pass
+
+
 class OwnersClient(object):
   """Interact with OWNERS files in a repository.
 
@@ -36,7 +40,7 @@ class OwnersClient(object):
     raise Exception('Not implemented')
 
   def GetChangeApprovalStatus(self, change_id):
-    """Check the approval status for the latest patch in a change.
+    """Check the approval status for the latest revision_id in a change.
 
     Returns a map of path to approval status, where the status can be one of:
     - APPROVED: An owner of the file has reviewed the change.
@@ -46,7 +50,7 @@ class OwnersClient(object):
     """
     raise Exception('Not implemented')
 
-  def IsOwnerConfigurationValid(self, change_id, patch):
+  def ValidateOwnersConfig(self, change_id):
     """Check if the owners configuration in a change is valid."""
     raise Exception('Not implemented')
 
@@ -56,6 +60,8 @@ class DepotToolsClient(OwnersClient):
   def __init__(self, host, root, fopen, os_path, branch):
     super(DepotToolsClient, self).__init__(host)
     self._root = root
+    self._fopen = fopen
+    self._os_path = os_path
     self._branch = branch
     self._db = owners.Database(root, fopen, os_path)
     self._db.override_files = self._GetOriginalOwnersFiles()
@@ -96,3 +102,18 @@ class DepotToolsClient(OwnersClient):
       else:
         status[f] = INSUFFICIENT_REVIEWERS
     return status
+
+  def ValidateOwnersConfig(self, change_id):
+    data = gerrit_util.GetChange(
+        self._host, change_id,
+        ['DETAILED_ACCOUNTS', 'DETAILED_LABELS', 'CURRENT_FILES',
+         'CURRENT_REVISION'])
+
+    files = data['revisions'][data['current_revision']]['files']
+
+    db = owners.Database(self._root, self._fopen, self._os_path)
+    try:
+      db.load_data_needed_for(
+          [f for f in files if os.path.basename(f) == 'OWNERS'])
+    except Exception as e:
+      raise InvalidOwnersConfig('Error parsing OWNERS files:\n%s' % e)

tests/owners_client_test.py

@@ -20,39 +20,40 @@ import owners_client
 from testing_support import filesystem_mock
 
 
-_TEST_CHANGE = {
-  "labels": {
-    "Code-Review": {
-      "all": [
-        {
-          "value": 1,
-          "email": "approver@example.com",
-        }
-      ],
-      "values": {
-        "-1": "Don't submit as-is",
-        " 0": "No score",
-        "+1": "Looks good to me"
+def _get_change():
+  return {
+    "labels": {
+      "Code-Review": {
+        "all": [
+          {
+            "value": 1,
+            "email": "approver@example.com",
+          }
+        ],
+        "values": {
+          "-1": "Don't submit as-is",
+          " 0": "No score",
+          "+1": "Looks good to me"
+        },
       },
     },
-  },
-  "reviewers": {
-    "REVIEWER": [
-      {"email": "approver@example.com"},
-      {"email": "reviewer@example.com"},
-    ],
-  },
-  "current_revision": "cb90826d03533d6c4e1f0e974ebcbfd7a6f42406",
-  "revisions": {
-    "cb90826d03533d6c4e1f0e974ebcbfd7a6f42406": {
-      "files": {
-        "approved.cc": {},
-        "reviewed.h": {},
-        "bar/insufficient.py": {},
+    "reviewers": {
+      "REVIEWER": [
+        {"email": "approver@example.com"},
+        {"email": "reviewer@example.com"},
+      ],
+    },
+    "current_revision": "cb90826d03533d6c4e1f0e974ebcbfd7a6f42406",
+    "revisions": {
+      "cb90826d03533d6c4e1f0e974ebcbfd7a6f42406": {
+        "files": {
+          "approved.cc": {},
+          "reviewed.h": {},
+          "bar/insufficient.py": {},
+        },
       },
     },
-  },
-}
+  }
 
 
 
@@ -70,32 +71,45 @@ class DepotToolsClientTest(unittest.TestCase):
         '/bar/everyone/OWNERS': '*',
         '/bar/everyone/foo.txt': '',
     })
-    self.files = self.repo.files
     self.root = '/'
     self.fopen = self.repo.open_for_reading
     mock.patch(
         'owners_client.DepotToolsClient._GetOriginalOwnersFiles',
         return_value={}).start()
     self.addCleanup(mock.patch.stopall)
+    self.client = owners_client.DepotToolsClient(
+        'host', '/', self.fopen, self.repo, 'branch')
 
   def testListOwners(self):
-    client = owners_client.DepotToolsClient(
-        'host', '/', self.fopen, self.repo, 'branch')
     self.assertEquals(
         ['*', 'missing@example.com'],
-        client.ListOwnersForFile('project', 'branch', 'bar/everyone/foo.txt'))
+        self.client.ListOwnersForFile(
+            'project', 'branch', 'bar/everyone/foo.txt'))
 
-  @mock.patch('gerrit_util.GetChange', return_value=_TEST_CHANGE)
+  @mock.patch('gerrit_util.GetChange', return_value=_get_change())
   def testGetChangeApprovalStatus(self, _mock):
-    client = owners_client.DepotToolsClient(
-        'host', '/', self.fopen, self.repo, 'branch')
     self.assertEquals(
         {
             'approved.cc': owners_client.APPROVED,
             'reviewed.h': owners_client.PENDING,
             'bar/insufficient.py': owners_client.INSUFFICIENT_REVIEWERS,
         },
-        client.GetChangeApprovalStatus('changeid'))
+        self.client.GetChangeApprovalStatus('changeid'))
+
+  @mock.patch('gerrit_util.GetChange', return_value=_get_change())
+  def testValidateOwnersConfig_OK(self, get_change_mock):
+    self.client.ValidateOwnersConfig('changeid')
+
+  @mock.patch('gerrit_util.GetChange', return_value=_get_change())
+  def testValidateOwnersConfig_Invalid(self, get_change_mock):
+    change = get_change_mock()
+    change['revisions'][change['current_revision']]['files'] = {'/OWNERS': {}}
+    self.repo.files['/OWNERS'] = '\n'.join([
+        'foo@example.com',
+        'invalid directive',
+    ])
+    with self.assertRaises(owners_client.InvalidOwnersConfig):
+      self.client.ValidateOwnersConfig('changeid')
 
 
 if __name__ == '__main__':