From 829ce021d538c7558787eab30bf1b260c2e34357 Mon Sep 17 00:00:00 2001 From: Edward Lesmes Date: Wed, 18 Nov 2020 18:30:31 +0000 Subject: [PATCH] [owners] Add tests for Depot Tools owners client. Change-Id: I988f5b866560d5839a1121c7129099ffe125fdce Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2545108 Reviewed-by: Josip Sokcevic Commit-Queue: Edward Lesmes --- owners.py | 4 +- owners_client.py | 14 +++-- tests/owners_client_test.py | 102 ++++++++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 7 deletions(-) create mode 100644 tests/owners_client_test.py diff --git a/owners.py b/owners.py index de246359f..ba2297a30 100644 --- a/owners.py +++ b/owners.py @@ -201,7 +201,7 @@ class Database(object): self._check_reviewers(reviewers) self.load_data_needed_for(files) - return set(f for f in files if not self._is_obj_covered_by(f, reviewers)) + return set(f for f in files if not self.is_covered_by(f, reviewers)) def _check_paths(self, files): def _is_under(f, pfx): @@ -214,7 +214,7 @@ class Database(object): _assert_is_collection(reviewers) assert all(self.email_regexp.match(r) for r in reviewers), reviewers - def _is_obj_covered_by(self, objname, reviewers): + def is_covered_by(self, objname, reviewers): reviewers = list(reviewers) + [EVERYONE] while True: for reviewer in reviewers: diff --git a/owners_client.py b/owners_client.py index 6ed0c6b29..da259a3c0 100644 --- a/owners_client.py +++ b/owners_client.py @@ -4,6 +4,7 @@ import os +import gerrit_util import owners import scm @@ -52,16 +53,19 @@ class OwnersClient(object): class DepotToolsClient(OwnersClient): """Implement OwnersClient using owners.py Database.""" - def __init__(self, host, root, branch): + def __init__(self, host, root, fopen, os_path, branch): super(DepotToolsClient, self).__init__(host) self._root = root self._branch = branch - self._db = owners.Database(root, open, os.path) - self._db.override_files({ + self._db = owners.Database(root, fopen, os_path) + self._db.override_files = self._GetOriginalOwnersFiles() + + def _GetOriginalOwnersFiles(self): + return { f: scm.GIT.GetOldContents(self._root, f, self._branch) for _, f in scm.GIT.CaptureStatus(self._root, self._branch) if os.path.basename(f) == 'OWNERS' - }) + } def ListOwnersForFile(self, _project, _branch, path): return sorted(self._db.all_possible_owners([path], None)) @@ -75,7 +79,7 @@ class DepotToolsClient(OwnersClient): reviewers = [r['email'] for r in data['reviewers']['REVIEWER']] # Get reviewers that have approved this change - label = change['labels']['Code-Review'] + label = data['labels']['Code-Review'] max_value = max(int(v) for v in label['values']) approvers = [v['email'] for v in label['all'] if v['value'] == max_value] diff --git a/tests/owners_client_test.py b/tests/owners_client_test.py new file mode 100644 index 000000000..74cccc5b0 --- /dev/null +++ b/tests/owners_client_test.py @@ -0,0 +1,102 @@ +# Copyright (c) 2020 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +import os +import sys +import unittest + +if sys.version_info.major == 2: + import mock +else: + from unittest import mock + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +import gerrit_util +import owners +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" + }, + }, + }, + "reviewers": { + "REVIEWER": [ + {"email": "approver@example.com"}, + {"email": "reviewer@example.com"}, + ], + }, + "current_revision": "cb90826d03533d6c4e1f0e974ebcbfd7a6f42406", + "revisions": { + "cb90826d03533d6c4e1f0e974ebcbfd7a6f42406": { + "files": { + "approved.cc": {}, + "reviewed.h": {}, + "bar/insufficient.py": {}, + }, + }, + }, +} + + + +class DepotToolsClientTest(unittest.TestCase): + def setUp(self): + self.repo = filesystem_mock.MockFileSystem(files={ + '/OWNERS': '\n'.join([ + 'per-file approved.cc=approver@example.com', + 'per-file reviewed.h=reviewer@example.com', + 'missing@example.com', + ]), + '/approved.cc': '', + '/reviewed.h': '', + '/bar/insufficient_reviewers.py': '', + '/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) + + 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')) + + @mock.patch('gerrit_util.GetChange', return_value=_TEST_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')) + + +if __name__ == '__main__': + unittest.main()