Commit 583ca660 authored by Joanna Wang's avatar Joanna Wang Committed by LUCI CQ

[depot_tools] Give user option to clear cl issue for merged changes.

Test:
Merged: https://chromium-review.googlesource.com/c/infra/gerrit-plugins/buildbucket/+/3600858

Created new CL in same branch:
https://chromium-review.googlesource.com/c/infra/gerrit-plugins/buildbucket/+/3609588

Bug: 1312511

Change-Id: I20ccfc978fd84c4c68f71007cf636fc1ee8e3d4d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3609115Reviewed-by: 's avatarGavin Mak <gavinmak@google.com>
Commit-Queue: Joanna Wang <jojwang@chromium.org>
parent 41c57603
...@@ -1754,10 +1754,19 @@ class Changelist(object): ...@@ -1754,10 +1754,19 @@ class Changelist(object):
return return
status = self._GetChangeDetail()['status'] status = self._GetChangeDetail()['status']
if status in ('MERGED', 'ABANDONED'): if status == 'ABANDONED':
DieWithError('Change %s has been %s, new uploads are not allowed' % DieWithError(
(self.GetIssueURL(), 'Change %s has been abandoned, new uploads are not allowed' %
'submitted' if status == 'MERGED' else 'abandoned')) (self.GetIssueURL()))
if status == 'MERGED':
answer = gclient_utils.AskForData(
'Change %s has been submitted, new uploads are not allowed. '
'Would you like to start a new change (Y/n)?' % self.GetIssueURL()
).lower()
if answer not in ('y', ''):
DieWithError('New uploads are not allowed.')
self.SetIssue()
return
# TODO(vadimsh): For some reason the chunk of code below was skipped if # TODO(vadimsh): For some reason the chunk of code below was skipped if
# 'is_gce' is True. I'm just refactoring it to be 'skip if not cookies'. # 'is_gce' is True. I'm just refactoring it to be 'skip if not cookies'.
......
...@@ -734,7 +734,8 @@ class TestGitCl(unittest.TestCase): ...@@ -734,7 +734,8 @@ class TestGitCl(unittest.TestCase):
def _gerrit_base_calls(cls, issue=None, fetched_description=None, def _gerrit_base_calls(cls, issue=None, fetched_description=None,
fetched_status=None, other_cl_owner=None, fetched_status=None, other_cl_owner=None,
custom_cl_base=None, short_hostname='chromium', custom_cl_base=None, short_hostname='chromium',
change_id=None, default_branch='main'): change_id=None, default_branch='main',
reset_issue=False):
calls = [] calls = []
if custom_cl_base: if custom_cl_base:
ancestor_revision = custom_cl_base ancestor_revision = custom_cl_base
...@@ -743,6 +744,9 @@ class TestGitCl(unittest.TestCase): ...@@ -743,6 +744,9 @@ class TestGitCl(unittest.TestCase):
ancestor_revision = 'origin/' + default_branch ancestor_revision = 'origin/' + default_branch
if issue: if issue:
# TODO: if tests don't provide a `change_id` the default used here
# will cause the TRACES_README_FORMAT mock (which uses the test provided
# `change_id` to fail.
gerrit_util.GetChangeDetail.return_value = { gerrit_util.GetChangeDetail.return_value = {
'owner': {'email': (other_cl_owner or 'owner@example.com')}, 'owner': {'email': (other_cl_owner or 'owner@example.com')},
'change_id': (change_id or '123456789'), 'change_id': (change_id or '123456789'),
...@@ -752,8 +756,21 @@ class TestGitCl(unittest.TestCase): ...@@ -752,8 +756,21 @@ class TestGitCl(unittest.TestCase):
}}, }},
'status': fetched_status or 'NEW', 'status': fetched_status or 'NEW',
} }
if fetched_status == 'ABANDONED': if fetched_status == 'ABANDONED':
return calls return calls
if fetched_status == 'MERGED':
calls.append(
(('ask_for_data',
'Change https://chromium-review.googlesource.com/%s has been '
'submitted, new uploads are not allowed. Would you like to start '
'a new change (Y/n)?' % issue), 'y' if reset_issue else 'n')
)
if not reset_issue:
return calls
# Part of SetIssue call.
calls.append(
((['git', 'log', '-1', '--format=%B'],), ''))
if other_cl_owner: if other_cl_owner:
calls += [ calls += [
(('ask_for_data', 'Press Enter to upload, or Ctrl+C to abort'), ''), (('ask_for_data', 'Press Enter to upload, or Ctrl+C to abort'), ''),
...@@ -1095,7 +1112,8 @@ class TestGitCl(unittest.TestCase): ...@@ -1095,7 +1112,8 @@ class TestGitCl(unittest.TestCase):
edit_description=None, edit_description=None,
fetched_description=None, fetched_description=None,
default_branch='main', default_branch='main',
push_opts=None): push_opts=None,
reset_issue=False):
"""Generic gerrit upload test framework.""" """Generic gerrit upload test framework."""
if squash_mode is None: if squash_mode is None:
if '--no-squash' in upload_args: if '--no-squash' in upload_args:
...@@ -1169,8 +1187,16 @@ class TestGitCl(unittest.TestCase): ...@@ -1169,8 +1187,16 @@ class TestGitCl(unittest.TestCase):
custom_cl_base=custom_cl_base, custom_cl_base=custom_cl_base,
short_hostname=short_hostname, short_hostname=short_hostname,
change_id=change_id, change_id=change_id,
default_branch=default_branch) default_branch=default_branch,
if fetched_status != 'ABANDONED': reset_issue=reset_issue)
if fetched_status == 'ABANDONED' or (
fetched_status == 'MERGED' and not reset_issue):
pass # readability
else:
if fetched_status == 'MERGED' and reset_issue:
fetched_status = 'NEW'
issue = None
mock.patch( mock.patch(
'gclient_utils.temporary_file', TemporaryFileMock()).start() 'gclient_utils.temporary_file', TemporaryFileMock()).start()
mock.patch('os.remove', return_value=True).start() mock.patch('os.remove', return_value=True).start()
...@@ -1424,6 +1450,30 @@ class TestGitCl(unittest.TestCase): ...@@ -1424,6 +1450,30 @@ class TestGitCl(unittest.TestCase):
'authenticate to Gerrit as yet-another@example.com.\n' 'authenticate to Gerrit as yet-another@example.com.\n'
'Uploading may fail due to lack of permissions', 'Uploading may fail due to lack of permissions',
sys.stdout.getvalue()) sys.stdout.getvalue())
@mock.patch('sys.stderr', StringIO())
def test_gerrit_upload_for_merged(self):
with self.assertRaises(SystemExitMock):
self._run_gerrit_upload_test(
[],
'desc ✔\n\nBUG=\n\nChange-Id: I123456789',
[],
issue=123456,
fetched_status='MERGED',
change_id='I123456789',
reset_issue=False)
self.assertEqual(
'New uploads are not allowed.\n',
sys.stderr.getvalue())
def test_gerrit_upload_new_issue_for_merged(self):
self._run_gerrit_upload_test(
[],
'desc ✔\n\nBUG=\n\nChange-Id: I123456789',
[],
issue=123456,
fetched_status='MERGED',
change_id='I123456789',
reset_issue=True)
def test_upload_change_description_editor(self): def test_upload_change_description_editor(self):
fetched_description = 'foo\n\nChange-Id: 123456789' fetched_description = 'foo\n\nChange-Id: 123456789'
......
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