From fd238086f1c3caa5bc4c11d3457f81d9a30f3ed7 Mon Sep 17 00:00:00 2001 From: Aaron Gable Date: Wed, 7 Jun 2017 13:42:34 -0700 Subject: [PATCH] Use API to set TBR code-review This prevents TBR (self code-review) permissions from blocking the CL from being uploaded at all. Instead, it will fully upload, and then show a better error message after upload is complete. Bug: 729967 Change-Id: I55e3e98e200143076afcaab858064d9f5c62f8ef Reviewed-on: https://chromium-review.googlesource.com/527325 Reviewed-by: Andrii Shyshkalov Commit-Queue: Aaron Gable --- git_cl.py | 10 ++++++---- tests/git_cl_test.py | 31 ++++++++++++++++++------------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/git_cl.py b/git_cl.py index fdc671a80..455ac04bd 100755 --- a/git_cl.py +++ b/git_cl.py @@ -3001,10 +3001,6 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): # Extra options that can be specified at push time. Doc: # https://gerrit-review.googlesource.com/Documentation/user-upload.html refspec_opts = [] - if change_desc.get_reviewers(tbr_only=True): - print('Adding self-LGTM (Code-Review +1) because of TBRs.') - refspec_opts.append('l=Code-Review+1') - # TODO(tandrii): options.message should be posted as a comment # if --send-email is set on non-initial upload as Rietveld used to do it. @@ -3084,6 +3080,12 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): self._GetGerritHost(), self.GetIssue(), reviewers, cc, notify=bool(options.send_mail)) + if change_desc.get_reviewers(tbr_only=True): + print('Adding self-LGTM (Code-Review +1) because of TBRs.') + gerrit_util.SetReview( + self._GetGerritHost(), self.GetIssue(), + labels={'Code-Review': 1}, notify=bool(options.send_mail)) + return 0 def _ComputeParent(self, remote, upstream_branch, custom_cl_base, force, diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index b966c0671..e1845c864 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -599,6 +599,9 @@ class TestGitCl(TestCase): self.mock(git_cl.gerrit_util, 'AddReviewers', lambda h, i, reviewers, ccs, notify: self._mocked_call( 'AddReviewers', h, i, reviewers, ccs, notify)) + self.mock(git_cl.gerrit_util, 'SetReview', + lambda h, i, labels, notify: self._mocked_call( + 'SetReview', h, i, labels, notify)) self.mock(git_cl.gerrit_util.GceAuthenticator, 'is_gce', classmethod(lambda _: False)) self.mock(git_cl, 'DieWithError', @@ -1414,10 +1417,9 @@ class TestGitCl(TestCase): def _gerrit_upload_calls(cls, description, reviewers, squash, squash_mode='default', expected_upstream_ref='origin/refs/heads/master', - ref_suffix='', title=None, notify=False, + title=None, notify=False, post_amend_description=None, issue=None, cc=None, - git_mirror=None, - custom_cl_base=None): + git_mirror=None, custom_cl_base=None, tbr=None): if post_amend_description is None: post_amend_description = description cc = cc or [] @@ -1517,11 +1519,9 @@ class TestGitCl(TestCase): '1hashPerLine\n'), ] + ref_suffix = '' if title: - if ref_suffix: - ref_suffix += ',m=' + title - else: - ref_suffix = '%m=' + title + ref_suffix += '%m=' + title notify_suffix = 'notify=NONE' if ref_suffix: @@ -1578,6 +1578,11 @@ class TestGitCl(TestCase): 123456 if squash else None, sorted(reviewers), ['joe@example.com'] + cc, notify), ''), ] + if tbr: + calls += [ + (('SetReview', 'chromium-review.googlesource.com', + 123456 if squash else None, {'Code-Review': 1}, notify), ''), + ] calls += cls._git_post_upload_calls() return calls @@ -1589,7 +1594,6 @@ class TestGitCl(TestCase): squash=True, squash_mode=None, expected_upstream_ref='origin/refs/heads/master', - ref_suffix='', title=None, notify=False, post_amend_description=None, @@ -1598,7 +1602,8 @@ class TestGitCl(TestCase): git_mirror=None, fetched_status=None, other_cl_owner=None, - custom_cl_base=None): + custom_cl_base=None, + tbr=None): """Generic gerrit upload test framework.""" if squash_mode is None: if '--no-squash' in upload_args: @@ -1639,10 +1644,10 @@ class TestGitCl(TestCase): description, reviewers, squash, squash_mode=squash_mode, expected_upstream_ref=expected_upstream_ref, - ref_suffix=ref_suffix, title=title, notify=notify, + title=title, notify=notify, post_amend_description=post_amend_description, issue=issue, cc=cc, git_mirror=git_mirror, - custom_cl_base=custom_cl_base) + custom_cl_base=custom_cl_base, tbr=tbr) # Uncomment when debugging. # print '\n'.join(map(lambda x: '%2i: %s' % x, enumerate(self.calls))) git_cl.main(['upload'] + upload_args) @@ -1704,8 +1709,8 @@ class TestGitCl(TestCase): ['reviewer@example.com', 'another@example.com'], squash=False, squash_mode='override_nosquash', - ref_suffix='%l=Code-Review+1', - cc=['more@example.com', 'people@example.com']) + cc=['more@example.com', 'people@example.com'], + tbr='reviewer@example.com') def test_gerrit_upload_squash_first_is_default(self): self._run_gerrit_upload_test(