From 5e37f6ded4ef5805c9db346a834d82e4e86b3e50 Mon Sep 17 00:00:00 2001 From: Edward Lesmes Date: Wed, 17 Feb 2021 23:32:16 +0000 Subject: [PATCH] owners_client: Always use slashes as separators. Change-Id: I6a74971878e5a1abaf6d8f5db20d0387c1abf308 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2701392 Auto-Submit: Edward Lesmes Reviewed-by: Josip Sokcevic Reviewed-by: Gavin Mak Commit-Queue: Edward Lesmes --- owners_client.py | 2 ++ tests/owners_client_test.py | 52 ++++++++++++++++++------------------- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/owners_client.py b/owners_client.py index 5fcd555f7..14c1bf5ef 100644 --- a/owners_client.py +++ b/owners_client.py @@ -208,6 +208,8 @@ class GerritClient(OwnersClient): self._owners_cache = {} def ListOwners(self, path): + # Always use slashes as separators. + path = path.replace(os.sep, '/') if path not in self._owners_cache: # GetOwnersForFile returns a list of account details sorted by order of # best reviewer for path. If owners have the same score, the order is diff --git a/tests/owners_client_test.py b/tests/owners_client_test.py index 1332f1db7..7695f9ccd 100644 --- a/tests/owners_client_test.py +++ b/tests/owners_client_test.py @@ -26,28 +26,6 @@ dave = 'dave@example.com' emily = 'emily@example.com' -def _get_owners(): - return { - "code_owners": [ - { - "account": { - "email": 'approver@example.com' - } - }, - { - "account": { - "email": 'reviewer@example.com' - }, - }, - { - "account": { - "email": 'missing@example.com' - }, - } - ] - } - - class DepotToolsClientTest(unittest.TestCase): def setUp(self): self.repo = filesystem_mock.MockFileSystem(files={ @@ -80,17 +58,39 @@ class DepotToolsClientTest(unittest.TestCase): class GerritClientTest(unittest.TestCase): def setUp(self): self.client = owners_client.GerritClient('host', 'project', 'branch') + mock.patch( + 'gerrit_util.GetOwnersForFile', + return_value={ + "code_owners": [ + { + "account": { + "email": 'approver@example.com' + } + }, + { + "account": { + "email": 'reviewer@example.com' + }, + }, + { + "account": { + "email": 'missing@example.com' + }, + } + ] + }).start() + self.addCleanup(mock.patch.stopall) - @mock.patch('gerrit_util.GetOwnersForFile', return_value=_get_owners()) - def testListOwners(self, _get_owners_mock): + def testListOwners(self): self.assertEquals( ['approver@example.com', 'reviewer@example.com', 'missing@example.com'], - self.client.ListOwners('bar/everyone/foo.txt')) + self.client.ListOwners(os.path.join('bar', 'everyone', 'foo.txt'))) # Result should be cached. self.assertEquals( ['approver@example.com', 'reviewer@example.com', 'missing@example.com'], - self.client.ListOwners('bar/everyone/foo.txt')) + self.client.ListOwners(os.path.join('bar', 'everyone', 'foo.txt'))) + # Always use slashes as separators. gerrit_util.GetOwnersForFile.assert_called_once_with( 'host', 'project', 'branch', 'bar/everyone/foo.txt')