diff --git a/git_cl.py b/git_cl.py index 430454ec2..756b683e3 100755 --- a/git_cl.py +++ b/git_cl.py @@ -2493,7 +2493,6 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): (self.GetIssue(), self.GetIssueOwner(), details['email'])) confirm_or_exit(action='upload') - def _PostUnsetIssueProperties(self): """Which branch-specific properties to erase when unsetting issue.""" return ['gerritsquashhash'] @@ -2661,15 +2660,14 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): gerrit_util.SubmitChange(self._GetGerritHost(), self.GetIssue(), wait_for_merge=wait_for_merge) - def _GetChangeDetail(self, options=None, issue=None, - no_cache=False): - """Returns details of the issue by querying Gerrit and caching results. + def _GetChangeDetail(self, options=None, no_cache=False): + """Returns details of associated Gerrit change and caching results. If fresh data is needed, set no_cache=True which will clear cache and thus new data will be fetched from Gerrit. """ options = options or [] - issue = issue or self.GetIssue() + issue = self.GetIssue() assert issue, 'issue is required to query Gerrit' # Optimization to avoid multiple RPCs: @@ -2678,7 +2676,7 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): options.append('CURRENT_COMMIT') # Normalize issue and options for consistent keys in cache. - issue = str(issue) + issue = str(self.GetIssue()) options = [o.upper() for o in options] # Check in cache first unless no_cache is True. @@ -2697,8 +2695,7 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): return data try: - data = gerrit_util.GetChangeDetail( - self._GetGerritHost(), str(issue), options) + data = gerrit_util.GetChangeDetail(self._GetGerritHost(), issue, options) except gerrit_util.GerritError as e: if e.http_status == 404: raise GerritChangeNotExists(issue, self.GetCodereviewServer()) diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 2cf432978..3bc4495ce 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -3114,17 +3114,6 @@ class TestGitCl(TestCase): def _mock_gerrit_changes_for_detail_cache(self): self.mock(git_cl._GerritChangelistImpl, '_GetGerritHost', lambda _: 'host') - def test_gerrit_change_detail_cache_normalize(self): - self._mock_gerrit_changes_for_detail_cache() - self.calls = [ - (('GetChangeDetail', 'host', '2', ['CASE']), 'b'), - ] - cl = git_cl.Changelist(codereview='gerrit') - self.assertEqual(cl._GetChangeDetail(issue=2, options=['CaSe']), 'b') - self.assertEqual(cl._GetChangeDetail(issue=2, options=['CASE']), 'b') - self.assertEqual(cl._GetChangeDetail(issue='2'), 'b') - self.assertEqual(cl._GetChangeDetail(issue=2), 'b') - def test_gerrit_change_detail_cache_simple(self): self._mock_gerrit_changes_for_detail_cache() self.calls = [ @@ -3132,13 +3121,14 @@ class TestGitCl(TestCase): (('GetChangeDetail', 'host', '2', []), 'b'), (('GetChangeDetail', 'host', '2', []), 'b2'), ] - cl = git_cl.Changelist(issue=1, codereview='gerrit') - self.assertEqual(cl._GetChangeDetail(), 'a') # Miss. - self.assertEqual(cl._GetChangeDetail(), 'a') - self.assertEqual(cl._GetChangeDetail(issue=2), 'b') # Miss. - self.assertEqual(cl._GetChangeDetail(issue=2, no_cache=True), 'b2') # Miss. - self.assertEqual(cl._GetChangeDetail(), 'a') - self.assertEqual(cl._GetChangeDetail(issue=2), 'b2') + cl1 = git_cl.Changelist(issue=1, codereview='gerrit') + cl2 = git_cl.Changelist(issue=2, codereview='gerrit') + self.assertEqual(cl1._GetChangeDetail(), 'a') # Miss. + self.assertEqual(cl1._GetChangeDetail(), 'a') + self.assertEqual(cl2._GetChangeDetail(), 'b') # Miss. + self.assertEqual(cl2._GetChangeDetail(no_cache=True), 'b2') # Miss. + self.assertEqual(cl1._GetChangeDetail(), 'a') + self.assertEqual(cl2._GetChangeDetail(), 'b2') def test_gerrit_change_detail_cache_options(self): self._mock_gerrit_changes_for_detail_cache()