From ee8be8a368620af69739f41046464630ed4f2309 Mon Sep 17 00:00:00 2001 From: Quinten Yearsley Date: Thu, 5 Mar 2020 21:48:32 +0000 Subject: [PATCH] git-cl: Don't throw AttributeError in git cl upload --retry-failed Bug: 1057745 Change-Id: Ic1b977634e15b163e627c0f389047a26f7b0c7d9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2083677 Commit-Queue: Quinten Yearsley Reviewed-by: Edward Lesmes Reviewed-by: Andrii Shyshkalov --- git_cl.py | 11 ++-- tests/git_cl_test.py | 117 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 122 insertions(+), 6 deletions(-) diff --git a/git_cl.py b/git_cl.py index 2348620ec7..6a691e9bb8 100755 --- a/git_cl.py +++ b/git_cl.py @@ -411,14 +411,17 @@ def _trigger_tryjobs(changelist, jobs, options, patchset): def _make_tryjob_schedule_requests(changelist, jobs, options, patchset): + """Constructs requests for Buildbucket to trigger tryjobs.""" gerrit_changes = [changelist.GetGerritChange(patchset)] - shared_properties = {'category': getattr(options, 'category', 'git_cl_try')} - if getattr(options, 'clobber', False): + shared_properties = { + 'category': options.ensure_value('category', 'git_cl_try') + } + if options.ensure_value('clobber', False): shared_properties['clobber'] = True shared_properties.update(_get_properties_from_options(options) or {}) shared_tags = [{'key': 'user_agent', 'value': 'git_cl_try'}] - if options.retry_failed: + if options.ensure_value('retry_failed', False): shared_tags.append({'key': 'retry_failed', 'value': '1'}) @@ -444,7 +447,7 @@ def _make_tryjob_schedule_requests(changelist, jobs, options, patchset): } }) - if options.revision: + if options.ensure_value('revision', None): requests[-1]['scheduleBuild']['gitilesCommit'] = { 'host': gerrit_changes[0]['host'], 'project': gerrit_changes[0]['project'], diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index b7243e2ac2..ed805b0a7f 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -13,6 +13,7 @@ import datetime import json import logging import multiprocessing +import optparse import os import pprint import shutil @@ -65,15 +66,23 @@ class ChangelistMock(object): # A class variable so we can access it when we don't have access to the # instance that's being set. desc = '' - def __init__(self, **kwargs): - pass + + def __init__(self, gerrit_change=None, **kwargs): + self._gerrit_change = gerrit_change + def GetIssue(self): return 1 + def FetchDescription(self): return ChangelistMock.desc + def UpdateDescription(self, desc, force=False): ChangelistMock.desc = desc + def GetGerritChange(self, patchset=None, **kwargs): + del patchset + return self._gerrit_change + class GitMocks(object): def __init__(self, config=None, branchref=None): @@ -3433,6 +3442,7 @@ class CMDTryTestCase(CMDTestCaseBase): class CMDUploadTestCase(CMDTestCaseBase): + def setUp(self): super(CMDUploadTestCase, self).setUp() mock.patch('git_cl._fetch_tryjobs').start() @@ -3484,6 +3494,109 @@ class CMDUploadTestCase(CMDTestCaseBase): mock.ANY, 8) +class MakeRequestsHelperTestCase(unittest.TestCase): + + def exampleGerritChange(self): + return { + 'host': 'chromium-review.googlesource.com', + 'project': 'depot_tools', + 'change': 1, + 'patchset': 2, + } + + def testMakeRequestsHelperNoOptions(self): + # Basic test for the helper function _make_tryjob_schedule_requests; + # it shouldn't throw AttributeError even when options doesn't have any + # of the expected values; it will use default option values. + changelist = ChangelistMock(gerrit_change=self.exampleGerritChange()) + jobs = [('chromium', 'try', 'my-builder')] + options = optparse.Values() + requests = git_cl._make_tryjob_schedule_requests( + changelist, jobs, options, patchset=None) + + # requestId is non-deterministic. Just assert that it's there and has + # a particular length. + self.assertEqual(len(requests[0]['scheduleBuild'].pop('requestId')), 36) + self.assertEqual(requests, [{ + 'scheduleBuild': { + 'builder': { + 'bucket': 'try', + 'builder': 'my-builder', + 'project': 'chromium' + }, + 'gerritChanges': [self.exampleGerritChange()], + 'properties': { + 'category': 'git_cl_try' + }, + 'tags': [{ + 'key': 'builder', + 'value': 'my-builder' + }, { + 'key': 'user_agent', + 'value': 'git_cl_try' + }] + } + }]) + + def testMakeRequestsHelperPresubmitSetsDryRunProperty(self): + changelist = ChangelistMock(gerrit_change=self.exampleGerritChange()) + jobs = [('chromium', 'try', 'presubmit')] + options = optparse.Values() + requests = git_cl._make_tryjob_schedule_requests( + changelist, jobs, options, patchset=None) + self.assertEqual(requests[0]['scheduleBuild']['properties'], { + 'category': 'git_cl_try', + 'dry_run': 'true' + }) + + def testMakeRequestsHelperRevisionSet(self): + # Gitiles commit is specified when revision is in options. + changelist = ChangelistMock(gerrit_change=self.exampleGerritChange()) + jobs = [('chromium', 'try', 'my-builder')] + options = optparse.Values({'revision': 'ba5eba11'}) + requests = git_cl._make_tryjob_schedule_requests( + changelist, jobs, options, patchset=None) + self.assertEqual( + requests[0]['scheduleBuild']['gitilesCommit'], { + 'host': 'chromium-review.googlesource.com', + 'id': 'ba5eba11', + 'project': 'depot_tools' + }) + + def testMakeRequestsHelperRetryFailedSet(self): + # An extra tag is added when retry_failed is in options. + changelist = ChangelistMock(gerrit_change=self.exampleGerritChange()) + jobs = [('chromium', 'try', 'my-builder')] + options = optparse.Values({'retry_failed': 'true'}) + requests = git_cl._make_tryjob_schedule_requests( + changelist, jobs, options, patchset=None) + self.assertEqual( + requests[0]['scheduleBuild']['tags'], [ + { + 'key': 'builder', + 'value': 'my-builder' + }, + { + 'key': 'user_agent', + 'value': 'git_cl_try' + }, + { + 'key': 'retry_failed', + 'value': '1' + } + ]) + + def testMakeRequestsHelperCategorySet(self): + # The category property can be overriden with options. + changelist = ChangelistMock(gerrit_change=self.exampleGerritChange()) + jobs = [('chromium', 'try', 'my-builder')] + options = optparse.Values({'category': 'my-special-category'}) + requests = git_cl._make_tryjob_schedule_requests( + changelist, jobs, options, patchset=None) + self.assertEqual(requests[0]['scheduleBuild']['properties'], + {'category': 'my-special-category'}) + + class CMDFormatTestCase(unittest.TestCase): def setUp(self):