From fc5fd927e248e145b7364a95beade1582a6fa911 Mon Sep 17 00:00:00 2001 From: Quinten Yearsley Date: Wed, 31 May 2017 11:50:52 -0700 Subject: [PATCH] Print CQ dry run messages/warnings consistently across commands. For a bit of context, see the TODO in the code -- I think that the original intent of that TODO was that we to make the way that CQ dry runs are triggered consistent, and also make the behavior of dry runs consistent across different commands. Change-Id: I80dfc31ade302a6af7fa84011e2871d416ea9c96 Reviewed-on: https://chromium-review.googlesource.com/518930 Commit-Queue: Quinten Yearsley Reviewed-by: Andrii Shyshkalov --- git_cl.py | 35 +++++++++++++---------------------- tests/git_cl_test.py | 4 ++-- 2 files changed, 15 insertions(+), 24 deletions(-) diff --git a/git_cl.py b/git_cl.py index bd9b4796dc..4b58c7141a 100755 --- a/git_cl.py +++ b/git_cl.py @@ -1681,36 +1681,27 @@ class Changelist(object): return ret def SetCQState(self, new_state): - """Update the CQ state for latest patchset. + """Updates the CQ state for the latest patchset. Issue must have been already uploaded and known. """ assert new_state in _CQState.ALL_STATES assert self.GetIssue() - return self._codereview_impl.SetCQState(new_state) - - def TriggerDryRun(self): - """Triggers a dry run and prints a warning on failure.""" - # TODO(qyearsley): Either re-use this method in CMDset_commit - # and CMDupload, or change CMDtry to trigger dry runs with - # just SetCQState, and catch keyboard interrupt and other - # errors in that method. try: - self.SetCQState(_CQState.DRY_RUN) - print('scheduled CQ Dry Run on %s' % self.GetIssueURL()) + self._codereview_impl.SetCQState(new_state) return 0 except KeyboardInterrupt: raise except: - print('WARNING: failed to trigger CQ Dry Run.\n' + print('WARNING: Failed to %s.\n' 'Either:\n' - ' * your project has no CQ\n' - ' * you don\'t have permission to trigger Dry Run\n' - ' * bug in this code (see stack trace below).\n' - 'Consider specifying which bots to trigger manually ' - 'or asking your project owners for permissions ' - 'or contacting Chrome Infrastructure team at ' - 'https://www.chromium.org/infra\n\n') + ' * Your project has no CQ,\n' + ' * You don\'t have permission to change the CQ state,\n' + ' * There\'s a bug in this code (see stack trace below).\n' + 'Consider specifying which bots to trigger manually or asking your ' + 'project owners for permissions or contacting Chrome Infra at:\n' + 'https://www.chromium.org/infra\n\n' % + ('cancel CQ' if new_state == _CQState.NONE else 'trigger CQ')) # Still raise exception so that stack trace is printed. raise @@ -5400,8 +5391,9 @@ def CMDtry(parser, args): # then we default to triggering a CQ dry run (see http://crbug.com/625697). if not buckets: if options.verbose: - print('git cl try with no bots now defaults to CQ Dry Run.') - return cl.TriggerDryRun() + print('git cl try with no bots now defaults to CQ dry run.') + print('Scheduling CQ dry run on: %s' % cl.GetIssueURL()) + return cl.SetCQState(_CQState.DRY_RUN) for builders in buckets.itervalues(): if any('triggered' in b for b in builders): @@ -5545,7 +5537,6 @@ def CMDset_commit(parser, args): if options.clear: state = _CQState.NONE elif options.dry_run: - # TODO(qyearsley): Use cl.TriggerDryRun. state = _CQState.DRY_RUN else: state = _CQState.COMMIT diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 570c3abadf..dedc69ae7a 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -2671,7 +2671,7 @@ class TestGitCl(TestCase): self.assertEqual(0, git_cl.main(['try'])) self.assertEqual( out.getvalue(), - 'scheduled CQ Dry Run on https://codereview.chromium.org/123\n') + 'Scheduling CQ dry run on: https://codereview.chromium.org/123\n') def test_git_cl_try_default_cq_dry_run_gerrit(self): self.mock(git_cl.Changelist, 'GetChange', @@ -2723,7 +2723,7 @@ class TestGitCl(TestCase): self.assertEqual(0, git_cl.main(['try'])) self.assertEqual( out.getvalue(), - 'scheduled CQ Dry Run on ' + 'Scheduling CQ dry run on: ' 'https://chromium-review.googlesource.com/123456\n') def test_git_cl_try_buildbucket_with_properties_rietveld(self):