commit 68e6cf3c9f910ba237cc7d61e00353a2dc7cf9a2
author Gavin Mak <gavinmak@google.com> Mon Jan 25 18:24:08 2021 +0000
committer LUCI CQ <infra-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Jan 25 18:24:08 2021 +0000
tree 0ddced63e6ce33408a7d748baef1e64473e70339
parent f08e425f2d22be925f3aa2952c5a6a958a26f97a

Don't open text editor when using --message-file

When using git cl upload --message-file, the text editor prompts the
user to edit the description. This change suppresses the editor when
a message file is passed in.

Change-Id: Ifa568e155e72eeb49f55ded0ddac1b5a940687af
Bug:916230
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2643781
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>

git_cl.py

diff --git a/git_cl.py b/git_cl.py
index 8b6a373..efcf429 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -2251,7 +2251,7 @@
         change_id = self._GetChangeDetail()['change_id']
         change_desc.ensure_change_id(change_id)
       else:  # if not self.GetIssue()
-        if not options.force:
+        if not options.force and not options.message_file:
           change_desc.prompt()
         change_ids = git_footers.get_footer_change_id(change_desc.description)
         if len(change_ids) == 1:
@@ -4186,7 +4186,6 @@
     if options.message:
       parser.error('Only one of --message and --message-file allowed.')
     options.message = gclient_utils.FileRead(options.message_file)
-    options.message_file = None
 
   if ([options.cq_dry_run,
        options.use_commit_queue,

tests/git_cl_test.py

diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py
index 62069e6..388bfa0 100755
--- a/tests/git_cl_test.py
+++ b/tests/git_cl_test.py
@@ -294,6 +294,39 @@
     self.assertEqual(options.force, True)
     self.assertEqual(options.edit_description, False)
 
+  def test_upload_with_message_file_no_editor(self):
+    m = mock.patch('git_cl.ChangeDescription.prompt',
+               return_value=None).start()
+    mock.patch('git_cl.Changelist.GetRemoteBranch',
+               return_value=('foo', git_cl.DEFAULT_NEW_BRANCH)).start()
+    mock.patch('git_cl.GetTargetRef',
+               return_value='refs/heads/main').start()
+    mock.patch('git_cl.Changelist._GerritCommitMsgHookCheck',
+               lambda _, offer_removal: None).start()
+    mock.patch('git_cl.Changelist.GetIssue', return_value=None).start()
+    mock.patch('git_cl.Changelist.GetBranch',
+               side_effect=SystemExitMock).start()
+    mock.patch('git_cl.GenerateGerritChangeId', return_value=None).start()
+    mock.patch('git_cl.RunGit').start()
+
+    cl = git_cl.Changelist()
+    options = optparse.Values()
+    options.target_branch = 'refs/heads/master'
+    options.squash = True
+    options.edit_description = False
+    options.force = False
+    options.preserve_tryjobs = False
+    options.message_file = "message.txt"
+
+    with self.assertRaises(SystemExitMock):
+      cl.CMDUploadChange(options, [], 'foo', git_cl.ChangeDescription('bar'))
+    self.assertEqual(len(m.mock_calls), 0)
+
+    options.message_file = None
+    with self.assertRaises(SystemExitMock):
+      cl.CMDUploadChange(options, [], 'foo', git_cl.ChangeDescription('bar'))
+    self.assertEqual(len(m.mock_calls), 1)
+
   def test_upload_to_old_default_retry_on_rollback(self):
     """Test when default branch migration had to be rolled back to old name"""
     m = mock.patch('git_cl.Changelist._CMDUploadChange',