git-cl: Mock GetChangeDetail on tests.

Mock GetChangeDetail instead of using self.calls to make the code
easier to change.

Bug: 1051631
Change-Id: I77361caccaf694644839825d890343864267688f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2062716
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Anthony Polito <apolito@google.com>
diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py
index 0cb1831..555cc90 100755
--- a/tests/git_cl_test.py
+++ b/tests/git_cl_test.py
@@ -629,9 +629,7 @@
         'git_cl.presubmit_support.DoPresubmitChecks', PresubmitMock).start()
     mock.patch('git_cl.watchlists.Watchlists', WatchlistsMock).start()
     mock.patch('git_cl.auth.Authenticator', AuthenticatorMock).start()
-    mock.patch(
-        'git_cl.gerrit_util.GetChangeDetail',
-        lambda *a: self._mocked_call('GetChangeDetail', *a)).start()
+    mock.patch('gerrit_util.GetChangeDetail').start()
     mock.patch(
         'git_cl.gerrit_util.GetChangeComments',
         lambda *a: self._mocked_call('GetChangeComments', *a)).start()
@@ -796,21 +794,15 @@
       ]
 
     if issue:
-      calls += [
-        (('GetChangeDetail', '%s-review.googlesource.com' % short_hostname,
-          'my%2Frepo~123456',
-          ['DETAILED_ACCOUNTS', 'CURRENT_REVISION', 'CURRENT_COMMIT', 'LABELS']
-         ),
-         {
-           'owner': {'email': (other_cl_owner or 'owner@example.com')},
-           'change_id': (change_id or '123456789'),
-           'current_revision': 'sha1_of_current_revision',
-           'revisions': {'sha1_of_current_revision': {
-             'commit': {'message': fetched_description},
-           }},
-           'status': fetched_status or 'NEW',
-         }),
-      ]
+      gerrit_util.GetChangeDetail.return_value = {
+        'owner': {'email': (other_cl_owner or 'owner@example.com')},
+        'change_id': (change_id or '123456789'),
+        'current_revision': 'sha1_of_current_revision',
+        'revisions': {'sha1_of_current_revision': {
+          'commit': {'message': fetched_description},
+        }},
+        'status': fetched_status or 'NEW',
+      }
       if fetched_status == 'ABANDONED':
         calls += [
           (('DieWithError', 'Change https://%s-review.googlesource.com/'
@@ -1714,30 +1706,25 @@
     mock.patch('git_cl.IsGitVersionAtLeast', return_value=True).start()
     self.mockGit.config['remote.origin.url'] = (
         'https://%s.googlesource.com/my/repo' % git_short_host)
-
-    self.calls += [
-      (('GetChangeDetail', git_short_host + '-review.googlesource.com',
-        'my%2Frepo~123456', ['ALL_REVISIONS', 'CURRENT_COMMIT']),
-       {
-         'current_revision': '7777777777',
-         'revisions': {
-           '1111111111': {
-             '_number': 1,
-             'fetch': {'http': {
-               'url': 'https://%s.googlesource.com/my/repo' % git_short_host,
-               'ref': 'refs/changes/56/123456/1',
-             }},
-           },
-           '7777777777': {
-             '_number': 7,
-             'fetch': {'http': {
-               'url': 'https://%s.googlesource.com/my/repo' % git_short_host,
-               'ref': 'refs/changes/56/123456/7',
-             }},
-           },
-         },
-       }),
-    ]
+    gerrit_util.GetChangeDetail.return_value = {
+      'current_revision': '7777777777',
+      'revisions': {
+        '1111111111': {
+          '_number': 1,
+          'fetch': {'http': {
+            'url': 'https://%s.googlesource.com/my/repo' % git_short_host,
+            'ref': 'refs/changes/56/123456/1',
+          }},
+        },
+        '7777777777': {
+          '_number': 7,
+          'fetch': {'http': {
+            'url': 'https://%s.googlesource.com/my/repo' % git_short_host,
+            'ref': 'refs/changes/56/123456/7',
+          }},
+        },
+      },
+    }
 
   def test_patch_gerrit_default(self):
     self._patch_common()
@@ -1810,7 +1797,7 @@
       git_cl.main(['patch', '123456'])
 
   @mock.patch(
-      'git_cl.gerrit_util.GetChangeDetail',
+      'gerrit_util.GetChangeDetail',
       side_effect=gerrit_util.GerritError(404, ''))
   def test_patch_gerrit_not_exists(self, *_mocks):
     self.mockGit.config['remote.origin.url'] = (
@@ -2018,16 +2005,12 @@
   def test_description(self):
     self.mockGit.config['remote.origin.url'] = (
         'https://chromium.googlesource.com/my/repo')
-    self.calls = [
-        (('GetChangeDetail', 'chromium-review.googlesource.com',
-          'my%2Frepo~123123', ['CURRENT_REVISION', 'CURRENT_COMMIT']),
-         {
-           'current_revision': 'sha1',
-           'revisions': {'sha1': {
-             'commit': {'message': 'foobar'},
-           }},
-         }),
-    ]
+    gerrit_util.GetChangeDetail.return_value = {
+      'current_revision': 'sha1',
+      'revisions': {'sha1': {
+        'commit': {'message': 'foobar'},
+      }},
+    }
     self.assertEqual(0, git_cl.main([
         'description',
         'https://chromium-review.googlesource.com/c/my/repo/+/123123',
@@ -2311,11 +2294,7 @@
 
   def test_gerrit_change_detail_cache_simple(self):
     self._mock_gerrit_changes_for_detail_cache()
-    self.calls = [
-        (('GetChangeDetail', 'host', 'my%2Frepo~1', []), 'a'),
-        (('GetChangeDetail', 'host', 'ab%2Frepo~2', []), 'b'),
-        (('GetChangeDetail', 'host', 'ab%2Frepo~2', []), 'b2'),
-    ]
+    gerrit_util.GetChangeDetail.side_effect = ['a', 'b']
     cl1 = git_cl.Changelist(issue=1)
     cl1._cached_remote_url = (
         True, 'https://chromium.googlesource.com/a/my/repo.git/')
@@ -2325,19 +2304,10 @@
     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()
-    self.calls = [
-        (('GetChangeDetail', 'host', 'repo~1', ['C', 'A', 'B']), 'cab'),
-        (('GetChangeDetail', 'host', 'repo~1', ['A', 'D']), 'ad'),
-        (('GetChangeDetail', 'host', 'repo~1', ['A']), 'a'),  # no_cache=True
-        # no longer in cache.
-        (('GetChangeDetail', 'host', 'repo~1', ['B']), 'b'),
-    ]
+    gerrit_util.GetChangeDetail.side_effect = ['cab', 'ad']
     cl = git_cl.Changelist(issue=1)
     cl._cached_remote_url = (True, 'https://chromium.googlesource.com/repo/')
     self.assertEqual(cl._GetChangeDetail(options=['C', 'A', 'B']), 'cab')
@@ -2352,21 +2322,13 @@
     self.assertEqual(cl._GetChangeDetail(options=['D']), 'ad')
     self.assertEqual(cl._GetChangeDetail(), 'cab')
 
-    # Finally, no_cache should invalidate all caches for given change.
-    self.assertEqual(cl._GetChangeDetail(options=['A'], no_cache=True), 'a')
-    self.assertEqual(cl._GetChangeDetail(options=['B']), 'b')
-
   def test_gerrit_description_caching(self):
-    def gen_detail(rev, desc):
-      return {
-        'current_revision': rev,
-        'revisions': {rev: {'commit': {'message': desc}}}
-      }
-    self.calls = [
-        (('GetChangeDetail', 'host', 'my%2Frepo~1',
-          ['CURRENT_REVISION', 'CURRENT_COMMIT']),
-         gen_detail('rev1', 'desc1')),
-    ]
+    gerrit_util.GetChangeDetail.return_value = {
+      'current_revision': 'rev1',
+      'revisions': {
+        'rev1': {'commit': {'message': 'desc1'}},
+      },
+    }
 
     self._mock_gerrit_changes_for_detail_cache()
     cl = git_cl.Changelist(issue=1)
@@ -2477,11 +2439,7 @@
   def test_git_cl_comments_fetch_gerrit(self, *_mocks):
     self.mockGit.config['remote.origin.url'] = (
         'https://chromium.googlesource.com/infra/infra')
-    self.calls = [
-      (('GetChangeDetail', 'chromium-review.googlesource.com',
-        'infra%2Finfra~1',
-        ['MESSAGES', 'DETAILED_ACCOUNTS', 'CURRENT_REVISION',
-         'CURRENT_COMMIT']), {
+    gerrit_util.GetChangeDetail.return_value = {
         'owner': {'email': 'owner@example.com'},
         'current_revision': 'ba5eba11',
         'revisions': {
@@ -2528,7 +2486,8 @@
              u'message': u'Patch Set 2: Code-Review+1',
           },
         ]
-      }),
+      }
+    self.calls = [
       (('GetChangeComments', 'chromium-review.googlesource.com',
         'infra%2Finfra~1'), {
         '/COMMIT_MSG': [
@@ -2620,72 +2579,69 @@
     # comments from the latest patchset are shown.
     self.mockGit.config['remote.origin.url'] = (
         'https://chromium.googlesource.com/infra/infra')
-    self.calls = [
-      (('GetChangeDetail', 'chromium-review.googlesource.com',
-        'infra%2Finfra~1',
-        ['MESSAGES', 'DETAILED_ACCOUNTS', 'CURRENT_REVISION',
-         'CURRENT_COMMIT']), {
-        'owner': {'email': 'owner@example.com'},
-        'current_revision': 'ba5eba11',
-        'revisions': {
-          'deadbeaf': {
-            '_number': 1,
-          },
-          'ba5eba11': {
-            '_number': 2,
-          },
+    gerrit_util.GetChangeDetail.return_value = {
+      'owner': {'email': 'owner@example.com'},
+      'current_revision': 'ba5eba11',
+      'revisions': {
+        'deadbeaf': {
+          '_number': 1,
         },
-        'messages': [
-          {
-             u'_revision_number': 1,
-             u'author': {
-               u'_account_id': 1111084,
-               u'email': u'commit-bot@chromium.org',
-               u'name': u'Commit Bot'
-             },
-             u'date': u'2017-03-15 20:08:45.000000000',
-             u'id': u'f5a6c25ecbd3b3b54a43ae418ed97eff046dc50b',
-             u'message': u'Patch Set 1:\n\nDry run: CQ is trying the patch...',
-             u'tag': u'autogenerated:cq:dry-run'
-          },
-          {
-             u'_revision_number': 1,
-             u'author': {
-               u'_account_id': 123,
-               u'email': u'tricium@serviceaccount.com',
-               u'name': u'Tricium'
-             },
-             u'date': u'2017-03-16 20:00:41.000000000',
-             u'id': u'f5a6c25ecbd3b3b54a43ae418ed97eff046d1234',
-             u'message': u'(1 comment)',
-             u'tag': u'autogenerated:tricium',
-          },
-          {
-             u'_revision_number': 1,
-             u'author': {
-               u'_account_id': 123,
-               u'email': u'tricium@serviceaccount.com',
-               u'name': u'Tricium'
-             },
-             u'date': u'2017-03-16 20:00:41.000000000',
-             u'id': u'f5a6c25ecbd3b3b54a43ae418ed97eff046d1234',
-             u'message': u'(1 comment)',
-             u'tag': u'autogenerated:tricium',
-          },
-          {
-             u'_revision_number': 2,
-             u'author': {
-               u'_account_id': 123,
-               u'email': u'tricium@serviceaccount.com',
-               u'name': u'reviewer'
-             },
-             u'date': u'2017-03-17 05:30:37.000000000',
-             u'tag': u'autogenerated:tricium',
-             u'id': u'f5a6c25ecbd3b3b54a43ae418ed97eff046d4568',
-             u'message': u'(1 comment)',
-          },
-        ]
-      }),
+        'ba5eba11': {
+          '_number': 2,
+        },
+      },
+      'messages': [
+      {
+        u'_revision_number': 1,
+        u'author': {
+          u'_account_id': 1111084,
+          u'email': u'commit-bot@chromium.org',
+          u'name': u'Commit Bot'
+        },
+        u'date': u'2017-03-15 20:08:45.000000000',
+        u'id': u'f5a6c25ecbd3b3b54a43ae418ed97eff046dc50b',
+        u'message': u'Patch Set 1:\n\nDry run: CQ is trying the patch...',
+        u'tag': u'autogenerated:cq:dry-run'
+      },
+      {
+        u'_revision_number': 1,
+        u'author': {
+          u'_account_id': 123,
+          u'email': u'tricium@serviceaccount.com',
+          u'name': u'Tricium'
+        },
+        u'date': u'2017-03-16 20:00:41.000000000',
+        u'id': u'f5a6c25ecbd3b3b54a43ae418ed97eff046d1234',
+        u'message': u'(1 comment)',
+        u'tag': u'autogenerated:tricium',
+      },
+      {
+        u'_revision_number': 1,
+        u'author': {
+          u'_account_id': 123,
+          u'email': u'tricium@serviceaccount.com',
+          u'name': u'Tricium'
+        },
+        u'date': u'2017-03-16 20:00:41.000000000',
+        u'id': u'f5a6c25ecbd3b3b54a43ae418ed97eff046d1234',
+        u'message': u'(1 comment)',
+        u'tag': u'autogenerated:tricium',
+      },
+      {
+        u'_revision_number': 2,
+        u'author': {
+          u'_account_id': 123,
+          u'email': u'tricium@serviceaccount.com',
+          u'name': u'reviewer'
+        },
+        u'date': u'2017-03-17 05:30:37.000000000',
+        u'tag': u'autogenerated:tricium',
+        u'id': u'f5a6c25ecbd3b3b54a43ae418ed97eff046d4568',
+        u'message': u'(1 comment)',
+      },
+      ]
+    }
+    self.calls = [
       (('GetChangeComments', 'chromium-review.googlesource.com',
         'infra%2Finfra~1'), {}),
       (('GetChangeRobotComments', 'chromium-review.googlesource.com',
@@ -2862,16 +2818,27 @@
     mock.patch('git_cl.sys.stdout', StringIO()).start()
     mock.patch('git_cl.uuid.uuid4', return_value='uuid4').start()
     mock.patch('git_cl.Changelist.GetIssue', return_value=123456).start()
-    mock.patch('git_cl.Changelist.GetCodereviewServer',
-               return_value='https://chromium-review.googlesource.com').start()
-    mock.patch('git_cl.Changelist.GetMostRecentPatchset',
-               return_value=7).start()
-    mock.patch('git_cl.auth.Authenticator',
-               return_value=AuthenticatorMock()).start()
-    mock.patch('git_cl.Changelist._GetChangeDetail',
-               return_value=self._CHANGE_DETAIL).start()
-    mock.patch('git_cl._call_buildbucket',
-               return_value = self._DEFAULT_RESPONSE).start()
+    mock.patch(
+        'git_cl.Changelist.GetCodereviewServer',
+        return_value='https://chromium-review.googlesource.com').start()
+    mock.patch(
+        'git_cl.Changelist._GetGerritHost',
+        return_value='chromium-review.googlesource.com').start()
+    mock.patch(
+        'git_cl.Changelist.GetMostRecentPatchset',
+        return_value=7).start()
+    mock.patch(
+        'git_cl.Changelist.GetRemoteUrl',
+        return_value='https://chromium.googlesource.com/depot_tools').start()
+    mock.patch(
+        'auth.Authenticator',
+        return_value=AuthenticatorMock()).start()
+    mock.patch(
+        'gerrit_util.GetChangeDetail',
+        return_value=self._CHANGE_DETAIL).start()
+    mock.patch(
+        'git_cl._call_buildbucket',
+        return_value = self._DEFAULT_RESPONSE).start()
     mock.patch('git_common.is_dirty_git_tree', return_value=False).start()
     self.addCleanup(mock.patch.stopall)
 
@@ -3240,6 +3207,14 @@
     mock.patch('git_cl.Settings.GetIsGerrit', return_value=True).start()
     self.addCleanup(mock.patch.stopall)
 
+  def testWarmUpChangeDetailCache(self):
+    self.assertEqual(0, git_cl.main(['upload']))
+    gerrit_util.GetChangeDetail.assert_called_once_with(
+        'chromium-review.googlesource.com', 'depot_tools~123456',
+        frozenset([
+            'LABELS', 'CURRENT_REVISION', 'DETAILED_ACCOUNTS',
+            'CURRENT_COMMIT']))
+
   def testUploadRetryFailed(self):
     # This test mocks out the actual upload part, and just asserts that after
     # upload, if --retry-failed is added, then the tool will fetch try jobs