From 3bbd43d85f99f73904e5b232a3996dc0d6a79578 Mon Sep 17 00:00:00 2001 From: Edward Lesmes Date: Tue, 2 Mar 2021 19:42:39 +0000 Subject: [PATCH] owners: Set correct branch to compute diff. We have to use refs/remotes/origin/foo instead of refs/heads/foo, as refs/heads/foo might not be defined. https://logs.chromium.org/logs/infra-internal/buildbucket/cr-buildbucket.appspot.com/8853851694141987488/+/u/build/git_cl_upload/stdout Change-Id: Ic729510d7beca63c25ea84394758cb62f76c572f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2729615 Auto-Submit: Edward Lesmes Commit-Queue: Josip Sokcevic Reviewed-by: Josip Sokcevic --- owners_client.py | 7 +++++-- tests/owners_client_test.py | 20 +++++++++++++++----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/owners_client.py b/owners_client.py index 753dada5e..8b2d28530 100644 --- a/owners_client.py +++ b/owners_client.py @@ -176,9 +176,12 @@ class DepotToolsClient(OwnersClient): self._db.override_files = self._GetOriginalOwnersFiles() def _GetOriginalOwnersFiles(self): + remote, _ = scm.GIT.FetchUpstreamTuple(self._root) + branch = ''.join(scm.GIT.RefToRemoteRef(self._branch, remote)) + return { - f: scm.GIT.GetOldContents(self._root, f, self._branch).splitlines() - for _, f in scm.GIT.CaptureStatus(self._root, self._branch) + f: scm.GIT.GetOldContents(self._root, f, branch).splitlines() + for _, f in scm.GIT.CaptureStatus(self._root, branch) if os.path.basename(f) == 'OWNERS' } diff --git a/tests/owners_client_test.py b/tests/owners_client_test.py index 4e74aaf2f..5039ca735 100644 --- a/tests/owners_client_test.py +++ b/tests/owners_client_test.py @@ -42,17 +42,27 @@ class DepotToolsClientTest(unittest.TestCase): }) 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( - '/', 'branch', self.fopen, self.repo) + '/', 'refs/heads/main', self.fopen, self.repo) + + @mock.patch('scm.GIT.CaptureStatus') + @mock.patch('scm.GIT.GetOldContents') + @mock.patch('scm.GIT.FetchUpstreamTuple') + def testListOwners( + self, mockFetchUpstreamTuple, mockGetOldContents, mockCaptureStatus): + mockFetchUpstreamTuple.return_value = ('remote', 'refs/heads/blah') + mockGetOldContents.side_effect = lambda r, f, _b: self.repo.files[r + f] + mockCaptureStatus.return_value = [ + ('M', 'bar/everyone/foo.txt'), + ('M', 'OWNERS'), + ] - def testListOwners(self): self.assertEqual( ['*', 'missing@example.com'], self.client.ListOwners('bar/everyone/foo.txt')) + mockCaptureStatus.assert_called_once_with( + '/', 'refs/remotes/remote/main') class GerritClientTest(unittest.TestCase):