From 4986d4a74b1975bd08e10fdf6ae8ef4468b4d287 Mon Sep 17 00:00:00 2001 From: Allen Li Date: Sat, 20 Jul 2024 00:29:35 +0000 Subject: [PATCH] [git_cl] Parametrize cwd in ShouldUseSSO Remove implicit/global assumptions which could affect tests. Bug: b/351071334 Change-Id: Ie266228f404b768cb539fdc17dddbbb13693e939 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5723208 Reviewed-by: Robbie Iannucci Commit-Queue: Allen Li Reviewed-by: Yiwei Zhang --- gerrit_util.py | 5 ++--- git_cl.py | 6 +++--- tests/gerrit_util_test.py | 12 ++++++------ 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/gerrit_util.py b/gerrit_util.py index e27c3f9b0..5f59e2ce4 100644 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -171,7 +171,7 @@ class SSOHelper(object): ssoHelper = SSOHelper() -def ShouldUseSSO(host: str) -> bool: +def ShouldUseSSO(cwd: str, host: str) -> bool: """Return True if we should use SSO for the current user.""" LOGGER.debug("Determining whether we should use SSO...") if not newauth.Enabled(): @@ -183,7 +183,6 @@ def ShouldUseSSO(host: str) -> bool: if not ssoHelper.find_cmd(): LOGGER.debug("SSO=False: no SSO command") return False - cwd = os.getcwd() email = scm.GIT.GetConfig(cwd, 'user.email', default='') if email.endswith('@google.com'): LOGGER.debug("SSO=True: email is google.com") @@ -365,7 +364,7 @@ class SSOAuthenticator(_Authenticator): if not cls._resolve_sso_cmd(): return False if conn is not None: - return ShouldUseSSO(conn.host) + return ShouldUseSSO(os.getcwd(), conn.host) email = scm.GIT.GetConfig(os.getcwd(), 'user.email', default='') return email.endswith('@google.com') diff --git a/git_cl.py b/git_cl.py index bf8772a24..c2426725e 100755 --- a/git_cl.py +++ b/git_cl.py @@ -3748,16 +3748,16 @@ class GitAuthConfigChanger(object): remote_url: str = cl.GetRemoteUrl() return cls( - mode=cls._infer_mode(gerrit_host), + mode=cls._infer_mode(os.getcwd(), gerrit_host), remote_url=remote_url, ) @staticmethod - def _infer_mode(gerrit_host: str) -> GitConfigMode: + def _infer_mode(cwd: str, gerrit_host: str) -> GitConfigMode: """Infer default mode to use.""" if not newauth.Enabled(): return GitConfigMode.OLD_AUTH - if gerrit_util.ShouldUseSSO(gerrit_host): + if gerrit_util.ShouldUseSSO(cwd, gerrit_host): return GitConfigMode.NEW_AUTH_SSO return GitConfigMode.NEW_AUTH diff --git a/tests/gerrit_util_test.py b/tests/gerrit_util_test.py index ddd0fee54..4385d51e8 100755 --- a/tests/gerrit_util_test.py +++ b/tests/gerrit_util_test.py @@ -736,19 +736,19 @@ class ShouldUseSSOTest(unittest.TestCase): def testDisabled(self): self.newauth.return_value = False - self.assertFalse(gerrit_util.ShouldUseSSO('fake-host')) + self.assertFalse(gerrit_util.ShouldUseSSO('/', 'fake-host')) def testMissingCommand(self): self.sso.return_value = 'fake-host' - self.assertFalse(gerrit_util.ShouldUseSSO('fake-host')) + self.assertFalse(gerrit_util.ShouldUseSSO('/', 'fake-host')) @mock.patch('scm.GIT.GetConfig', return_value='firefly@google.com') def testGoogle(self, _): - self.assertTrue(gerrit_util.ShouldUseSSO('fake-host')) + self.assertTrue(gerrit_util.ShouldUseSSO('/', 'fake-host')) @mock.patch('scm.GIT.GetConfig', return_value='firefly@gmail.com') def testGmail(self, _): - self.assertFalse(gerrit_util.ShouldUseSSO('fake-host')) + self.assertFalse(gerrit_util.ShouldUseSSO('/', 'fake-host')) @mock.patch('gerrit_util.GetAccountEmails', return_value=[{ @@ -756,7 +756,7 @@ class ShouldUseSSOTest(unittest.TestCase): }]) @mock.patch('scm.GIT.GetConfig', return_value='firefly@chromium.org') def testLinkedChromium(self, _cfg, email): - self.assertTrue(gerrit_util.ShouldUseSSO('fake-host')) + self.assertTrue(gerrit_util.ShouldUseSSO('/', 'fake-host')) email.assert_called_with('fake-host', 'self', authenticator=mock.ANY) @mock.patch('gerrit_util.GetAccountEmails', @@ -765,7 +765,7 @@ class ShouldUseSSOTest(unittest.TestCase): }]) @mock.patch('scm.GIT.GetConfig', return_value='firefly@chromium.org') def testUnlinkedChromium(self, _cfg, email): - self.assertFalse(gerrit_util.ShouldUseSSO('fake-host')) + self.assertFalse(gerrit_util.ShouldUseSSO('/', 'fake-host')) email.assert_called_with('fake-host', 'self', authenticator=mock.ANY)