Commit 0dd5482c authored by Edward Lesmes's avatar Edward Lesmes Committed by LUCI CQ

depot_tools: Restore appending title before change description.

On an initial upload, git-cl upload -t <title> should append <title>
before the change description.

This broke because options.squash is set to a default value very late
into code execution, so its value was not available when computing the
initial description.

options.squash is now set when parsing the options, early into code
execution.

Bug: 1064905
Change-Id: I04a18c572dd5b04a3013d39f71bc43d026ea85d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2122324Reviewed-by: 's avatarJosip Sokcevic <sokcevic@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
parent d1d4ac90
...@@ -1436,7 +1436,7 @@ class Changelist(object): ...@@ -1436,7 +1436,7 @@ class Changelist(object):
else: else:
description = _create_description_from_log(git_diff_args) description = _create_description_from_log(git_diff_args)
if options.title and options.squash: if options.title and options.squash:
description = options.title + '\n\n' + message description = options.title + '\n\n' + description
# Extract bug number from branch name. # Extract bug number from branch name.
bug = options.bug bug = options.bug
...@@ -2260,10 +2260,6 @@ class Changelist(object): ...@@ -2260,10 +2260,6 @@ class Changelist(object):
def CMDUploadChange( def CMDUploadChange(
self, options, git_diff_args, custom_cl_base, change_desc): self, options, git_diff_args, custom_cl_base, change_desc):
"""Upload the current branch to Gerrit.""" """Upload the current branch to Gerrit."""
if options.squash is None:
# Load default for user, repo, squash=true, in this order.
options.squash = settings.GetSquashGerritUploads()
remote, remote_branch = self.GetRemoteBranch() remote, remote_branch = self.GetRemoteBranch()
branch = GetTargetRef(remote, remote_branch, options.target_branch) branch = GetTargetRef(remote, remote_branch, options.target_branch)
...@@ -4287,6 +4283,10 @@ def CMDupload(parser, args): ...@@ -4287,6 +4283,10 @@ def CMDupload(parser, args):
if options.use_commit_queue: if options.use_commit_queue:
options.send_mail = True options.send_mail = True
if options.squash is None:
# Load default for user, repo, squash=true, in this order.
options.squash = settings.GetSquashGerritUploads()
cl = Changelist() cl = Changelist()
# Warm change details cache now to avoid RPCs later, reducing latency for # Warm change details cache now to avoid RPCs later, reducing latency for
# developers. # developers.
......
...@@ -658,6 +658,7 @@ class TestGitCl(unittest.TestCase): ...@@ -658,6 +658,7 @@ class TestGitCl(unittest.TestCase):
super(TestGitCl, self).setUp() super(TestGitCl, self).setUp()
self.calls = [] self.calls = []
self._calls_done = [] self._calls_done = []
self.failed = False
mock.patch('sys.stdout', StringIO()).start() mock.patch('sys.stdout', StringIO()).start()
mock.patch( mock.patch(
'git_cl.time_time', 'git_cl.time_time',
...@@ -734,7 +735,8 @@ class TestGitCl(unittest.TestCase): ...@@ -734,7 +735,8 @@ class TestGitCl(unittest.TestCase):
def tearDown(self): def tearDown(self):
try: try:
self.assertEqual([], self.calls) if not self.failed:
self.assertEqual([], self.calls)
except AssertionError: except AssertionError:
calls = ''.join(' %s\n' % str(call) for call in self.calls[:5]) calls = ''.join(' %s\n' % str(call) for call in self.calls[:5])
if len(self.calls) > 5: if len(self.calls) > 5:
...@@ -771,6 +773,7 @@ class TestGitCl(unittest.TestCase): ...@@ -771,6 +773,7 @@ class TestGitCl(unittest.TestCase):
(prior_calls, len(self._calls_done), expected_args, (prior_calls, len(self._calls_done), expected_args,
len(self._calls_done), args, following_calls)) len(self._calls_done), args, following_calls))
self.failed = True
self.fail('@%d\n' self.fail('@%d\n'
' Expected: %r\n' ' Expected: %r\n'
' Actual: %r\n' ' Actual: %r\n'
...@@ -1164,6 +1167,7 @@ class TestGitCl(unittest.TestCase): ...@@ -1164,6 +1167,7 @@ class TestGitCl(unittest.TestCase):
final_description=None, final_description=None,
gitcookies_exists=True, gitcookies_exists=True,
force=False, force=False,
log_description=None,
edit_description=None, edit_description=None,
fetched_description=None): fetched_description=None):
"""Generic gerrit upload test framework.""" """Generic gerrit upload test framework."""
...@@ -1211,7 +1215,8 @@ class TestGitCl(unittest.TestCase): ...@@ -1211,7 +1215,8 @@ class TestGitCl(unittest.TestCase):
lambda path: self._mocked_call(['os.path.isfile', path])).start() 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.GitSanityChecks', return_value=True).start()
mock.patch( mock.patch(
'git_cl._create_description_from_log', return_value=description).start() 'git_cl._create_description_from_log',
return_value=log_description or description).start()
mock.patch( mock.patch(
'git_cl.Changelist._AddChangeIdToCommitMessage', 'git_cl.Changelist._AddChangeIdToCommitMessage',
return_value=post_amend_description or description).start() return_value=post_amend_description or description).start()
...@@ -1319,7 +1324,7 @@ class TestGitCl(unittest.TestCase): ...@@ -1319,7 +1324,7 @@ class TestGitCl(unittest.TestCase):
short_hostname='other', short_hostname='other',
change_id='I123456789') change_id='I123456789')
def test_gerrit_patchset_title_special_chars(self): def test_gerrit_patchset_title_special_chars_nosquash(self):
self._run_gerrit_upload_test( self._run_gerrit_upload_test(
['-f', '-t', 'We\'ll escape ^_ ^ special chars...@{u}'], ['-f', '-t', 'We\'ll escape ^_ ^ special chars...@{u}'],
'desc ✔\n\nBUG=\n\nChange-Id: I123456789', 'desc ✔\n\nBUG=\n\nChange-Id: I123456789',
...@@ -1398,6 +1403,16 @@ class TestGitCl(unittest.TestCase): ...@@ -1398,6 +1403,16 @@ class TestGitCl(unittest.TestCase):
squash=True, squash=True,
change_id='123456789') change_id='123456789')
def test_gerrit_upload_squash_first_title(self):
self._run_gerrit_upload_test(
['-f', '-t', 'title'],
'title\n\ndesc\n\nChange-Id: 123456789',
[],
force=True,
squash=True,
log_description='desc',
change_id='123456789')
def test_gerrit_upload_squash_first_with_labels(self): def test_gerrit_upload_squash_first_with_labels(self):
self._run_gerrit_upload_test( self._run_gerrit_upload_test(
['--squash', '--cq-dry-run', '--enable-auto-submit'], ['--squash', '--cq-dry-run', '--enable-auto-submit'],
...@@ -3469,6 +3484,10 @@ class CMDUploadTestCase(CMDTestCaseBase): ...@@ -3469,6 +3484,10 @@ class CMDUploadTestCase(CMDTestCaseBase):
mock.patch('git_cl._fetch_tryjobs').start() mock.patch('git_cl._fetch_tryjobs').start()
mock.patch('git_cl._trigger_tryjobs', return_value={}).start() mock.patch('git_cl._trigger_tryjobs', return_value={}).start()
mock.patch('git_cl.Changelist.CMDUpload', return_value=0).start() mock.patch('git_cl.Changelist.CMDUpload', return_value=0).start()
mock.patch('git_cl.Settings.GetRoot', return_value='').start()
mock.patch(
'git_cl.Settings.GetSquashGerritUploads',
return_value=True).start()
self.addCleanup(mock.patch.stopall) self.addCleanup(mock.patch.stopall)
def testWarmUpChangeDetailCache(self): def testWarmUpChangeDetailCache(self):
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment