From 9aa1a9673a3c60858982bed95d6ad7bef469144c Mon Sep 17 00:00:00 2001
From: Edward Lemur <ehmaldonado@google.com>
Date: Tue, 25 Feb 2020 00:58:38 +0000
Subject: [PATCH] git-cl: Add GetAuthor method and cache GetLocalDescription
 results

Bug: 1051631
Change-Id: I5da32978bbed16bcc9854f15db91a901f5892cda
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2067402
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
---
 git_cl.py            | 17 ++++++++++-------
 tests/git_cl_test.py | 23 ++++++++++++++---------
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/git_cl.py b/git_cl.py
index 395b460f78..78174df6c0 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -1031,8 +1031,8 @@ class Changelist(object):
     self.upstream_branch = None
     self.lookedup_issue = False
     self.issue = issue or None
-    self.has_description = False
     self.description = None
+    self.local_description = None
     self.lookedup_patchset = False
     self.patchset = None
     self.cc = None
@@ -1284,17 +1284,18 @@ class Changelist(object):
 
   def GetLocalDescription(self, upstream_branch):
     """Return the log messages of all commits up to the branch point."""
-    args = ['log', '--pretty=format:%s%n%n%b', '%s...' % (upstream_branch)]
-    return RunGitWithCode(args)[1].strip()
+    if self.local_description is None:
+      args = ['log', '--pretty=format:%s%n%n%b', '%s...' % (upstream_branch)]
+      self.local_description = RunGitWithCode(args)[1].strip()
+    return self.local_description
 
   def FetchDescription(self, pretty=False):
     assert self.GetIssue(), 'issue is required to query Gerrit'
 
-    if not self.has_description:
+    if self.description is None:
       data = self._GetChangeDetail(['CURRENT_REVISION', 'CURRENT_COMMIT'])
       current_rev = data['current_revision']
       self.description = data['revisions'][current_rev]['commit']['message']
-      self.has_description = True
 
     if not pretty:
       return self.description
@@ -1314,6 +1315,9 @@ class Changelist(object):
       self.lookedup_patchset = True
     return self.patchset
 
+  def GetAuthor(self):
+    return scm.GIT.GetConfig(settings.GetRoot(), 'user.email')
+
   def SetPatchset(self, patchset):
     """Set this branch's patchset. If patchset=0, clears the patchset."""
     assert self.GetBranch()
@@ -1388,7 +1392,7 @@ class Changelist(object):
       # with these log messages.
       description = self.GetLocalDescription(upstream_branch)
 
-    author = RunGit(['config', 'user.email']).strip() or None
+    author = self.GetAuthor()
     return presubmit_support.GitChange(
         name,
         description,
@@ -1417,7 +1421,6 @@ class Changelist(object):
         description, notify='NONE')
 
     self.description = description
-    self.has_description = True
 
   def RunHook(self, committing, may_prompt, verbose, change, parallel):
     """Calls sys.exit() if the hook fails; returns a HookResults otherwise."""
diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py
index 4a780b45be..ad6adcf9e8 100755
--- a/tests/git_cl_test.py
+++ b/tests/git_cl_test.py
@@ -192,7 +192,6 @@ class TestGitClBasic(unittest.TestCase):
   def test_fetch_description(self):
     cl = git_cl.Changelist(issue=1, codereview_host='host')
     cl.description = 'x'
-    cl.has_description = True
     self.assertEqual(cl.FetchDescription(), 'x')
 
   @mock.patch('git_cl.Changelist.EnsureAuthenticated')
@@ -819,15 +818,7 @@ class TestGitCl(unittest.TestCase):
       ((['git', 'rev-parse', 'HEAD'],), '12345'),
     ]
 
-    if not issue:
-      calls += [
-        ((['git', 'log', '--pretty=format:%s%n%n%b',
-           ancestor_revision + '...'],),
-         'foo'),
-      ]
-
     calls += [
-      ((['git', 'config', 'user.email'],), 'me@example.com'),
       (('time.time',), 1000,),
       (('time.time',), 3000,),
       (('add_repeated', 'sub_commands', {
@@ -1194,12 +1185,15 @@ class TestGitCl(unittest.TestCase):
     mock.patch('os.path.isfile',
               lambda path: self._mocked_call(['os.path.isfile', path])).start()
     mock.patch('git_cl.Changelist.GitSanityChecks', return_value=True).start()
+    mock.patch(
+        'git_cl.Changelist.GetLocalDescription', return_value='foo').start()
 
     self.mockGit.config['gerrit.host'] = 'true'
     self.mockGit.config['branch.master.gerritissue'] = (
         str(issue) if issue else None)
     self.mockGit.config['remote.origin.url'] = (
         'https://%s.googlesource.com/my/repo' % short_hostname)
+    self.mockGit.config['user.email'] = 'me@example.com'
 
     self.calls = self._gerrit_base_calls(
         issue=issue,
@@ -2784,6 +2778,17 @@ class TestGitCl(unittest.TestCase):
     self.assertEqual(cl._GerritChangeIdentifier(), '123456')
 
 
+class ChangelistTest(unittest.TestCase):
+  @mock.patch('git_cl.RunGitWithCode')
+  def testGetLocalDescription(self, _mock):
+    git_cl.RunGitWithCode.return_value = (0, 'description')
+    cl = git_cl.Changelist()
+    self.assertEqual('description', cl.GetLocalDescription('branch'))
+    self.assertEqual('description', cl.GetLocalDescription('branch'))
+    git_cl.RunGitWithCode.assert_called_once_with(
+        ['log', '--pretty=format:%s%n%n%b', 'branch...'])
+
+
 class CMDTestCaseBase(unittest.TestCase):
   _STATUSES = [
       'STATUS_UNSPECIFIED', 'SCHEDULED', 'STARTED', 'SUCCESS', 'FAILURE',