From b46b033bd71f548d9e28bc55f878cd4e29f76ad1 Mon Sep 17 00:00:00 2001
From: Robert Iannucci <iannucci@chromium.org>
Date: Fri, 30 Aug 2024 01:20:40 +0000
Subject: [PATCH] [git-cl] Make quick Gerrit RPC at the top of CMDupload.

This prevents the following scenario:

$ git cl upload ...
<runs `git status`>  # slow operation
<runs `git cl presubmit`>  # slow operation
<runs $EDITOR /commit/message> # requires thinking
CRASH!! You need to authenticate!!
$ <authenticate>
$ git cl upload ...
<runs `git status`>  # slow operation
<runs `git cl presubmit`>  # slow operation
<runs $EDITOR /commit/message> # opens new, fresh, editor :(

Now it will do:

$ git cl upload ...
CRASH!! You need to authenticate!!
$ <authenticate>
$ git cl upload ...
<runs `git status`>  # slow operation
<runs `git cl presubmit`>  # slow operation
<runs $EDITOR /commit/message> # requires thinking

R=ayatane, sokcevic@google.com

Change-Id: Icada2b69b3f4ddaff810dc82d54d65f3918d1434
Bug: 351071334
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5826970
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
Auto-Submit: Robbie Iannucci <iannucci@chromium.org>
---
 git_cl.py            | 14 ++++++++++++--
 tests/git_cl_test.py | 14 ++++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/git_cl.py b/git_cl.py
index 936a182a8..918007732 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -5252,6 +5252,18 @@ def CMDupload(parser, args):
                        if opt.help != optparse.SUPPRESS_HELP))
         return
 
+    cl = Changelist(branchref=options.target_branch)
+
+    # Do a quick RPC to Gerrit to ensure that our authentication is all working
+    # properly. Otherwise `git cl upload` will:
+    #   * run `git status` (slow for large repos)
+    #   * run presubmit tests (likely slow)
+    #   * ask the user to edit the CL description (requires thinking)
+    #
+    # And then attempt to push the change up to Gerrit, which can fail if
+    # authentication is not working properly.
+    gerrit_util.GetAccountDetails(cl.GetGerritHost())
+
     # TODO(crbug.com/1475405): Warn users if the project uses submodules and
     # they have fsmonitor enabled.
     if os.path.isfile('.gitmodules'):
@@ -5299,8 +5311,6 @@ def CMDupload(parser, args):
         # Load default for user, repo, squash=true, in this order.
         options.squash = settings.GetSquashGerritUploads()
 
-    cl = Changelist(branchref=options.target_branch)
-
     # Warm change details cache now to avoid RPCs later, reducing latency for
     # developers.
     if cl.GetIssue():
diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py
index a67c1da51..b647543d0 100755
--- a/tests/git_cl_test.py
+++ b/tests/git_cl_test.py
@@ -51,6 +51,18 @@ def callError(code=1, cmd='', cwd='', stdout=b'', stderr=b''):
 CERR1 = callError(1)
 
 
+def getAccountDetailsMock(host, account_id='self'):
+    if account_id == 'self':
+        return {
+            '_account_id': 123456,
+            'avatars': [],
+            'email': 'getAccountDetailsMock@example.com',
+            'name': 'GetAccountDetails(self)',
+            'status': 'OOO',
+        }
+    return None
+
+
 class TemporaryFileMock(object):
     def __init__(self):
         self.suffix = 0
@@ -1199,6 +1211,8 @@ class TestGitCl(unittest.TestCase):
                    return_value=change_id).start()
         mock.patch('git_common.get_or_create_merge_base',
                    return_value='origin/' + default_branch).start()
+        mock.patch('gerrit_util.GetAccountDetails',
+                    getAccountDetailsMock).start()
         mock.patch(
             'gclient_utils.AskForData',
             lambda prompt: self._mocked_call('ask_for_data', prompt)).start()