From baaf6bec019f60ea5f1c3e7d9b9dd12302ce61fc Mon Sep 17 00:00:00 2001 From: Edward Lemur Date: Wed, 9 Oct 2019 18:00:44 +0000 Subject: [PATCH] git-cl: Use buildbucket v2 to fetch tryjob results. Bug: 976104 Change-Id: Icf761f1cd093f7600ad43b71af474e52780f1997 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1842335 Reviewed-by: Anthony Polito Reviewed-by: Andrii Shyshkalov Commit-Queue: Edward Lesmes --- git_cl.py | 205 +++++-------------- tests/git_cl_test.py | 475 +++++++++++++++---------------------------- 2 files changed, 223 insertions(+), 457 deletions(-) diff --git a/git_cl.py b/git_cl.py index 56104a18b..ed3f10ff1 100755 --- a/git_cl.py +++ b/git_cl.py @@ -525,16 +525,19 @@ def fetch_try_jobs(auth_config, changelist, buildbucket_host, patchset=None): """Fetches tryjobs from buildbucket. - Returns a map from build ID to build info as a dictionary. + Returns list of buildbucket.v2.Build with the try jobs for the changelist. """ - assert buildbucket_host - assert changelist.GetIssue(), 'CL must be uploaded first' - assert changelist.GetCodereviewServer(), 'CL must be uploaded first' - patchset = patchset or changelist.GetMostRecentPatchset() - assert patchset, 'CL must be uploaded first' + fields = ['id', 'builder', 'status'] + request = { + 'predicate': { + 'gerritChanges': [changelist.GetGerritChange(patchset)], + }, + 'fields': ','.join('builds.*.' + field for field in fields), + } codereview_url = changelist.GetCodereviewServer() codereview_host = urlparse.urlparse(codereview_url).hostname + authenticator = auth.get_authenticator_for_host(codereview_host, auth_config) if authenticator.has_cached_credentials(): http = authenticator.authorize(httplib2.Http()) @@ -543,29 +546,10 @@ def fetch_try_jobs(auth_config, changelist, buildbucket_host, # Get the message on how to login. (auth.LoginRequiredError().message,)) http = httplib2.Http() - http.force_exception_to_status_code = True - buildset = 'patch/gerrit/{hostname}/{issue}/{patch}'.format( - hostname=codereview_host, - issue=changelist.GetIssue(), - patch=patchset) - params = {'tag': 'buildset:%s' % buildset} - - builds = {} - while True: - url = 'https://{hostname}/_ah/api/buildbucket/v1/search?{params}'.format( - hostname=buildbucket_host, - params=urllib.urlencode(params)) - content = _buildbucket_retry('fetching tryjobs', http, url, 'GET') - for build in content.get('builds', []): - builds[build['id']] = build - if 'next_cursor' in content: - params['start_cursor'] = content['next_cursor'] - else: - break - return builds - + response = _call_buildbucket(http, buildbucket_host, 'SearchBuilds', request) + return response.get('builds', []) def _fetch_latest_builds( auth_config, changelist, buildbucket_host, latest_patchset=None): @@ -579,9 +563,8 @@ def _fetch_latest_builds( lastest_patchset(int|NoneType): the patchset to start fetching builds from. If None (default), starts with the latest available patchset. Returns: - A tuple (builds, patchset) where builds is a dict mapping from build ID to - build info from Buildbucket, and patchset is the patchset number where - those builds came from. + A tuple (builds, patchset) where builds is a list of buildbucket.v2.Build, + and patchset is the patchset number where those builds came from. """ assert buildbucket_host assert changelist.GetIssue(), 'CL must be uploaded first' @@ -607,23 +590,20 @@ def _filter_failed(builds): """Returns a list of buckets/builders that had failed builds. Args: - builds (dict): Builds, in the format returned by fetch_try_jobs, - i.e. a dict mapping build ID to build info dict, which includes - the keys status, result, bucket, and builder_name. + builds (list): Builds, in the format returned by fetch_try_jobs, + i.e. a list of buildbucket.v2.Builds which includes status and builder + info. Returns: A dict of bucket to builder to tests (empty list). This is the same format accepted by _trigger_try_jobs and returned by _get_bucket_map. """ buckets = collections.defaultdict(dict) - for build in builds.values(): - if build['status'] == 'COMPLETED' and build['result'] == 'FAILURE': - project = build['project'] - bucket = build['bucket'] - if bucket.startswith('luci.'): - # Assume legacy bucket name luci... - bucket = bucket.split('.')[2] - builder = _get_builder_from_build(build) + for build in builds: + if build['status'] in ('FAILURE', 'INFRA_FAILURE'): + project = build['builder']['project'] + bucket = build['builder']['bucket'] + builder = build['builder']['builder'] buckets[project + '/' + bucket][builder] = [] return buckets @@ -634,99 +614,55 @@ def print_try_jobs(options, builds): print('No tryjobs scheduled.') return - # Make a copy, because we'll be modifying builds dictionary. - builds = builds.copy() - builder_names_cache = {} - - def get_builder(b): - try: - return builder_names_cache[b['id']] - except KeyError: - name = _get_builder_from_build(b) - builder_names_cache[b['id']] = name - return name - + longest_builder = max(len(b['builder']['builder']) for b in builds) + name_fmt = '{builder:<%d}' % longest_builder if options.print_master: - name_fmt = '%%-%ds %%-%ds' % ( - max(len(str(b['bucket'])) for b in builds.itervalues()), - max(len(str(get_builder(b))) for b in builds.itervalues())) - def get_name(b): - return name_fmt % (b['bucket'], get_builder(b)) - else: - name_fmt = '%%-%ds' % ( - max(len(str(get_builder(b))) for b in builds.itervalues())) - def get_name(b): - return name_fmt % get_builder(b) + longest_bucket = max(len(b['builder']['bucket']) for b in builds) + name_fmt = ('{bucket:>%d} ' % longest_bucket) + name_fmt + + builds_by_status = {} + for b in builds: + builds_by_status.setdefault(b['status'], []).append({ + 'id': b['id'], + 'name': name_fmt.format( + builder=b['builder']['builder'], bucket=b['builder']['bucket']), + }) - def sort_key(b): - return b['status'], b.get('result'), get_name(b), b.get('url') + sort_key = lambda b: (b['name'], b['id']) - def pop(title, f, color=None, **kwargs): + def print_builds(title, builds, fmt=None, color=None): """Pop matching builds from `builds` dict and print them.""" + if not builds: + return + fmt = fmt or '{name} https://ci.chromium.org/b/{id}' if not options.color or color is None: - colorize = str + colorize = lambda x: x else: colorize = lambda x: '%s%s%s' % (color, x, Fore.RESET) - result = [] - for b in builds.values(): - if all(b.get(k) == v for k, v in kwargs.iteritems()): - builds.pop(b['id']) - result.append(b) - if result: - print(colorize(title)) - for b in sorted(result, key=sort_key): - print(' ', colorize('\t'.join(map(str, f(b))))) + print(colorize(title)) + for b in sorted(builds, key=sort_key): + print(' ', colorize(fmt.format(**b))) total = len(builds) - pop(status='COMPLETED', result='SUCCESS', - title='Successes:', color=Fore.GREEN, - f=lambda b: (get_name(b), b.get('url'))) - pop(status='COMPLETED', result='FAILURE', failure_reason='INFRA_FAILURE', - title='Infra Failures:', color=Fore.MAGENTA, - f=lambda b: (get_name(b), b.get('url'))) - pop(status='COMPLETED', result='FAILURE', failure_reason='BUILD_FAILURE', - title='Failures:', color=Fore.RED, - f=lambda b: (get_name(b), b.get('url'))) - pop(status='COMPLETED', result='CANCELED', - title='Canceled:', color=Fore.MAGENTA, - f=lambda b: (get_name(b),)) - pop(status='COMPLETED', result='FAILURE', - failure_reason='INVALID_BUILD_DEFINITION', - title='Wrong master/builder name:', color=Fore.MAGENTA, - f=lambda b: (get_name(b),)) - pop(status='COMPLETED', result='FAILURE', - title='Other failures:', - f=lambda b: (get_name(b), b.get('failure_reason'), b.get('url'))) - pop(status='COMPLETED', - title='Other finished:', - f=lambda b: (get_name(b), b.get('result'), b.get('url'))) - pop(status='STARTED', - title='Started:', color=Fore.YELLOW, - f=lambda b: (get_name(b), b.get('url'))) - pop(status='SCHEDULED', - title='Scheduled:', - f=lambda b: (get_name(b), 'id=%s' % b['id'])) + print_builds( + 'Successes:', builds_by_status.pop('SUCCESS', []), color=Fore.GREEN) + print_builds( + 'Infra Failures:', builds_by_status.pop('INFRA_FAILURE', []), + color=Fore.MAGENTA) + print_builds('Failures:', builds_by_status.pop('FAILURE', []), color=Fore.RED) + print_builds('Canceled:', builds_by_status.pop('CANCELED', []), fmt='{name}', + color=Fore.MAGENTA) + print_builds('Started:', builds_by_status.pop('STARTED', [])) + print_builds( + 'Scheduled:', builds_by_status.pop('SCHEDULED', []), fmt='{name} id={id}') # The last section is just in case buildbucket API changes OR there is a bug. - pop(title='Other:', - f=lambda b: (get_name(b), 'id=%s' % b['id'])) - assert len(builds) == 0 + print_builds( + 'Other:', sum(builds_by_status.values(), []), fmt='{name} id={id}') print('Total: %d tryjobs' % total) -def _get_builder_from_build(build): - """Returns a builder name from a BB v1 build info dict.""" - try: - parameters = json.loads(build['parameters_json']) - name = parameters['builder_name'] - except (ValueError, KeyError) as error: - print('WARNING: Failed to get builder name for build %s: %s' % ( - build['id'], error)) - name = None - return name - - def _ComputeDiffLineRanges(files, upstream_commit): """Gets the changed line ranges for each file since upstream_commit. @@ -809,35 +745,6 @@ def _FindYapfConfigFile(fpath, yapf_config_cache, top_dir=None): return ret -def write_try_results_json(output_file, builds): - """Writes a subset of the data from fetch_try_jobs to a file as JSON. - - The input |builds| dict is assumed to be generated by Buildbucket. - Buildbucket documentation: http://goo.gl/G0s101 - """ - - def convert_build_dict(build): - """Extracts some of the information from one build dict.""" - parameters = json.loads(build.get('parameters_json', '{}')) or {} - return { - 'buildbucket_id': build.get('id'), - 'bucket': build.get('bucket'), - 'builder_name': parameters.get('builder_name'), - 'created_ts': build.get('created_ts'), - 'experimental': build.get('experimental'), - 'failure_reason': build.get('failure_reason'), - 'result': build.get('result'), - 'status': build.get('status'), - 'tags': build.get('tags'), - 'url': build.get('url'), - } - - converted = [] - for _, build in sorted(builds.items()): - converted.append(convert_build_dict(build)) - write_json(output_file, converted) - - def print_stats(args): """Prints statistics about the change to the user.""" # --no-ext-diff is broken in some versions of Git, so try to work around @@ -4873,7 +4780,7 @@ def CMDtry_results(parser, args): print('Buildbucket error: %s' % ex) return 1 if options.json: - write_try_results_json(options.json, jobs) + write_json(options.json, jobs) else: print_try_jobs(options, jobs) return 0 diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index c734e220b..963972619 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -2426,167 +2426,6 @@ class TestGitCl(TestCase): self.assertRegexpMatches(out.getvalue(), 'Issue.*123 has been submitted') self.assertRegexpMatches(out.getvalue(), 'Landed as: .*deadbeef') - BUILDBUCKET_BUILDS_MAP = { - '9000': { - 'id': '9000', - 'bucket': 'master.x.y', - 'created_by': 'user:someone@chromium.org', - 'created_ts': '147200002222000', - 'experimental': False, - 'parameters_json': json.dumps({ - 'builder_name': 'my-bot', - 'properties': {'category': 'cq'}, - }), - 'status': 'STARTED', - 'tags': [ - 'build_address:x.y/my-bot/2', - 'builder:my-bot', - 'experimental:false', - 'user_agent:cq', - ], - 'url': 'http://build.cr.org/p/x.y/builders/my-bot/builds/2', - }, - '8000': { - 'id': '8000', - 'bucket': 'master.x.y', - 'created_by': 'user:someone@chromium.org', - 'created_ts': '147200001111000', - 'experimental': False, - 'failure_reason': 'BUILD_FAILURE', - 'parameters_json': json.dumps({ - 'builder_name': 'my-bot', - 'properties': {'category': 'cq'}, - }), - 'result_details_json': json.dumps({ - 'properties': {'buildnumber': 1}, - }), - 'result': 'FAILURE', - 'status': 'COMPLETED', - 'tags': [ - 'build_address:x.y/my-bot/1', - 'builder:my-bot', - 'experimental:false', - 'user_agent:cq', - ], - 'url': 'http://build.cr.org/p/x.y/builders/my-bot/builds/1', - }, - } - - def test_write_try_results_json(self): - expected_output = [ - { - 'bucket': 'master.x.y', - 'buildbucket_id': '8000', - 'builder_name': 'my-bot', - 'created_ts': '147200001111000', - 'experimental': False, - 'failure_reason': 'BUILD_FAILURE', - 'result': 'FAILURE', - 'status': 'COMPLETED', - 'tags': [ - 'build_address:x.y/my-bot/1', - 'builder:my-bot', - 'experimental:false', - 'user_agent:cq', - ], - 'url': 'http://build.cr.org/p/x.y/builders/my-bot/builds/1', - }, - { - 'bucket': 'master.x.y', - 'buildbucket_id': '9000', - 'builder_name': 'my-bot', - 'created_ts': '147200002222000', - 'experimental': False, - 'failure_reason': None, - 'result': None, - 'status': 'STARTED', - 'tags': [ - 'build_address:x.y/my-bot/2', - 'builder:my-bot', - 'experimental:false', - 'user_agent:cq', - ], - 'url': 'http://build.cr.org/p/x.y/builders/my-bot/builds/2', - }, - ] - self.calls = [(('write_json', 'output.json', expected_output), '')] - git_cl.write_try_results_json('output.json', self.BUILDBUCKET_BUILDS_MAP) - - def _setup_fetch_try_jobs(self, most_recent_patchset=20001): - out = StringIO.StringIO() - self.mock(sys, 'stdout', out) - self.mock(git_cl.Changelist, 'GetMostRecentPatchset', - lambda *args: most_recent_patchset) - self.mock(git_cl.auth, 'get_authenticator_for_host', lambda host, _cfg: - self._mocked_call(['get_authenticator_for_host', host])) - self.mock(git_cl, '_buildbucket_retry', lambda *_, **__: - self._mocked_call(['_buildbucket_retry'])) - - def _setup_fetch_try_jobs_gerrit(self, *request_results): - self._setup_fetch_try_jobs(most_recent_patchset=13) - self.calls += [ - ((['git', 'symbolic-ref', 'HEAD'],), 'feature'), - ((['git', 'config', 'branch.feature.gerritissue'],), '1'), - # TODO(tandrii): Uncomment the below if we decide to support checking - # patchsets for Gerrit. - # Simulate that Gerrit has more patchsets than local. - # ((['git', 'config', 'branch.feature.gerritpatchset'],), '12'), - ((['git', 'config', 'branch.feature.gerritserver'],), - 'https://x-review.googlesource.com'), - ((['get_authenticator_for_host', 'x-review.googlesource.com'],), - AuthenticatorMock()), - ] + [((['_buildbucket_retry'],), r) for r in request_results] - - def test_fetch_try_jobs_none_gerrit(self): - self._setup_fetch_try_jobs_gerrit({}) - self.assertEqual(0, git_cl.main(['try-results'])) - # TODO(tandrii): Uncomment the below if we decide to support checking - # patchsets for Gerrit. - # self.assertRegexpMatches( - # sys.stdout.getvalue(), - # r'Warning: Codereview server has newer patchsets \(13\)') - self.assertRegexpMatches(sys.stdout.getvalue(), 'No tryjobs') - - def test_fetch_try_jobs_some_gerrit(self): - self._setup_fetch_try_jobs_gerrit({ - 'builds': self.BUILDBUCKET_BUILDS_MAP.values(), - }) - # TODO(tandrii): Uncomment the below if we decide to support checking - # patchsets for Gerrit. - # self.calls.remove( - # ((['git', 'config', 'branch.feature.gerritpatchset'],), '12')) - self.assertEqual(0, git_cl.main(['try-results', '--patchset', '5'])) - - # ... and doesn't result in warning. - self.assertNotRegexpMatches(sys.stdout.getvalue(), 'Warning') - self.assertRegexpMatches(sys.stdout.getvalue(), '^Failures:') - self.assertRegexpMatches(sys.stdout.getvalue(), 'Started:') - self.assertRegexpMatches(sys.stdout.getvalue(), '2 tryjobs') - - def test_filter_failed_none(self): - self.assertEqual(git_cl._filter_failed({}), {}) - - def test_filter_failed_some(self): - builds = { - '9000': { - 'id': '9000', - 'bucket': 'luci.chromium.try', - 'project': 'chromium', - 'created_by': 'user:someone@chromium.org', - 'created_ts': '147200002222000', - 'experimental': False, - 'parameters_json': json.dumps({ - 'builder_name': 'my-bot', - 'properties': {'category': 'cq'}, - }), - 'status': 'COMPLETED', - 'result': 'FAILURE', - } - } - self.assertEqual( - git_cl._filter_failed(builds), - {'chromium/try': {'my-bot': []}}) - def _mock_gerrit_changes_for_detail_cache(self): self.mock(git_cl.Changelist, '_GetGerritHost', lambda _: 'host') @@ -3137,43 +2976,150 @@ class TestGitCl(TestCase): self.assertEqual(cl._GerritChangeIdentifier(), '123456') -class CMDTryTestCase(unittest.TestCase): +class CMDTestCaseBase(unittest.TestCase): + _STATUSES = [ + 'STATUS_UNSPECIFIED', 'SCHEDULED', 'STARTED', 'SUCCESS', 'FAILURE', + 'INFRA_FAILURE', 'CANCELED', + ] + _CHANGE_DETAIL = { + 'project': 'depot_tools', + 'status': 'OPEN', + 'owner': {'email': 'owner@e.mail'}, + 'current_revision': 'beeeeeef', + 'revisions': { + 'deadbeaf': {'_number': 6}, + 'beeeeeef': { + '_number': 7, + 'fetch': {'http': { + 'url': 'https://chromium.googlesource.com/depot_tools', + 'ref': 'refs/changes/56/123456/7' + }}, + }, + }, + } + _DEFAULT_RESPONSE = { + 'builds': [ + { + 'id': str(100 + idx), + 'builder': { + 'project': 'chromium', + 'bucket': 'try', + 'builder': 'bot_' + status.lower(), + }, + 'status': status, + } + for idx, status in enumerate(_STATUSES) + ] + } + def setUp(self): - super(CMDTryTestCase, self).setUp() + super(CMDTestCaseBase, self).setUp() mock.patch('git_cl.sys.stdout', StringIO.StringIO()).start() - mock.patch('git_cl.uuid.uuid4', _constantFn('uuid4')).start() - mock.patch('git_cl.Changelist.GetIssue', _constantFn(123456)).start() + mock.patch('git_cl.uuid.uuid4', return_value='uuid4').start() + mock.patch('git_cl.Changelist.GetIssue', return_value=123456).start() mock.patch('git_cl.Changelist.GetCodereviewServer', - _constantFn('https://chromium-review.googlesource.com')).start() - mock.patch('git_cl.Changelist.SetPatchset').start() - mock.patch('git_cl.Changelist.GetPatchset', _constantFn(7)).start() + return_value='https://chromium-review.googlesource.com').start() + mock.patch('git_cl.Changelist.GetMostRecentPatchset', + return_value=7).start() mock.patch('git_cl.auth.get_authenticator_for_host', AuthenticatorMock()) + mock.patch('git_cl.Changelist._GetChangeDetail', + return_value=self._CHANGE_DETAIL).start() + mock.patch('git_cl._call_buildbucket', + return_value = self._DEFAULT_RESPONSE).start() + mock.patch('git_common.is_dirty_git_tree', return_value=False).start() self.addCleanup(mock.patch.stopall) - @mock.patch('git_cl.Changelist._GetChangeDetail') + +class CMDTryResultsTestCase(CMDTestCaseBase): + _DEFAULT_REQUEST = { + 'predicate': { + "gerritChanges": [{ + "project": "depot_tools", + "host": "chromium-review.googlesource.com", + "patchset": 7, + "change": 123456, + }], + }, + 'fields': 'builds.*.id,builds.*.builder,builds.*.status', + } + + def testNoJobs(self): + git_cl._call_buildbucket.return_value = {} + + self.assertEqual(0, git_cl.main(['try-results'])) + self.assertEqual('No tryjobs scheduled.\n', sys.stdout.getvalue()) + git_cl._call_buildbucket.assert_called_once_with( + mock.ANY, 'cr-buildbucket.appspot.com', 'SearchBuilds', + self._DEFAULT_REQUEST) + + def testPrintToStdout(self): + self.assertEqual(0, git_cl.main(['try-results'])) + self.assertEqual([ + 'Successes:', + ' bot_success https://ci.chromium.org/b/103', + 'Infra Failures:', + ' bot_infra_failure https://ci.chromium.org/b/105', + 'Failures:', + ' bot_failure https://ci.chromium.org/b/104', + 'Canceled:', + ' bot_canceled ', + 'Started:', + ' bot_started https://ci.chromium.org/b/102', + 'Scheduled:', + ' bot_scheduled id=101', + 'Other:', + ' bot_status_unspecified id=100', + 'Total: 7 tryjobs', + ], sys.stdout.getvalue().splitlines()) + git_cl._call_buildbucket.assert_called_once_with( + mock.ANY, 'cr-buildbucket.appspot.com', 'SearchBuilds', + self._DEFAULT_REQUEST) + + def testPrintToStdoutWithMasters(self): + self.assertEqual(0, git_cl.main(['try-results', '--print-master'])) + self.assertEqual([ + 'Successes:', + ' try bot_success https://ci.chromium.org/b/103', + 'Infra Failures:', + ' try bot_infra_failure https://ci.chromium.org/b/105', + 'Failures:', + ' try bot_failure https://ci.chromium.org/b/104', + 'Canceled:', + ' try bot_canceled ', + 'Started:', + ' try bot_started https://ci.chromium.org/b/102', + 'Scheduled:', + ' try bot_scheduled id=101', + 'Other:', + ' try bot_status_unspecified id=100', + 'Total: 7 tryjobs', + ], sys.stdout.getvalue().splitlines()) + git_cl._call_buildbucket.assert_called_once_with( + mock.ANY, 'cr-buildbucket.appspot.com', 'SearchBuilds', + self._DEFAULT_REQUEST) + + @mock.patch('git_cl.write_json') + def testWriteToJson(self, mockJsonDump): + self.assertEqual(0, git_cl.main(['try-results', '--json', 'file.json'])) + git_cl._call_buildbucket.assert_called_once_with( + mock.ANY, 'cr-buildbucket.appspot.com', 'SearchBuilds', + self._DEFAULT_REQUEST) + mockJsonDump.assert_called_once_with( + 'file.json', self._DEFAULT_RESPONSE['builds']) + + def test_filter_failed(self): + self.assertEqual({}, git_cl._filter_failed([])) + self.assertEqual({ + 'chromium/try': {'bot_failure': [], 'bot_infra_failure': []}, + }, git_cl._filter_failed(self._DEFAULT_RESPONSE['builds'])) + + +class CMDTryTestCase(CMDTestCaseBase): + @mock.patch('git_cl.Changelist.SetCQState') - @mock.patch('git_cl._get_bucket_map', _constantFn({})) - def testSetCQDryRunByDefault(self, mockSetCQState, mockGetChangeDetail): + @mock.patch('git_cl._get_bucket_map', return_value={}) + def testSetCQDryRunByDefault(self, _mockGetBucketMap, mockSetCQState): mockSetCQState.return_value = 0 - mockGetChangeDetail.return_value = { - 'project': 'depot_tools', - 'status': 'OPEN', - 'owner': {'email': 'owner@e.mail'}, - 'current_revision': 'beeeeeef', - 'revisions': { - 'deadbeaf': { - '_number': 6, - }, - 'beeeeeef': { - '_number': 7, - 'fetch': {'http': { - 'url': 'https://chromium.googlesource.com/depot_tools', - 'ref': 'refs/changes/56/123456/7' - }}, - }, - }, - } - self.assertEqual(0, git_cl.main(['try'])) git_cl.Changelist.SetCQState.assert_called_with(git_cl._CQState.DRY_RUN) self.assertEqual( @@ -3181,28 +3127,9 @@ class CMDTryTestCase(unittest.TestCase): 'Scheduling CQ dry run on: ' 'https://chromium-review.googlesource.com/123456\n') - @mock.patch('git_cl.Changelist._GetChangeDetail') @mock.patch('git_cl._call_buildbucket') - def testScheduleOnBuildbucket(self, mockCallBuildbucket, mockGetChangeDetail): + def testScheduleOnBuildbucket(self, mockCallBuildbucket): mockCallBuildbucket.return_value = {} - mockGetChangeDetail.return_value = { - 'project': 'depot_tools', - 'status': 'OPEN', - 'owner': {'email': 'owner@e.mail'}, - 'current_revision': 'beeeeeef', - 'revisions': { - 'deadbeaf': { - '_number': 6, - }, - 'beeeeeef': { - '_number': 7, - 'fetch': {'http': { - 'url': 'https://chromium.googlesource.com/depot_tools', - 'ref': 'refs/changes/56/123456/7' - }}, - }, - }, - } self.assertEqual(0, git_cl.main([ 'try', '-B', 'luci.chromium.try', '-b', 'win', @@ -3241,27 +3168,7 @@ class CMDTryTestCase(unittest.TestCase): mockCallBuildbucket.assert_called_with( mock.ANY, 'cr-buildbucket.appspot.com', 'Batch', expected_request) - @mock.patch('git_cl.Changelist._GetChangeDetail') - def testScheduleOnBuildbucket_WrongBucket(self, mockGetChangeDetail): - mockGetChangeDetail.return_value = { - 'project': 'depot_tools', - 'status': 'OPEN', - 'owner': {'email': 'owner@e.mail'}, - 'current_revision': 'beeeeeef', - 'revisions': { - 'deadbeaf': { - '_number': 6, - }, - 'beeeeeef': { - '_number': 7, - 'fetch': {'http': { - 'url': 'https://chromium.googlesource.com/depot_tools', - 'ref': 'refs/changes/56/123456/7' - }}, - }, - }, - } - + def testScheduleOnBuildbucket_WrongBucket(self): self.assertEqual(0, git_cl.main([ 'try', '-B', 'not-a-bucket', '-b', 'win', '-p', 'key=val', '-p', 'json=[{"a":1}, null]'])) @@ -3301,95 +3208,47 @@ class CMDTryTestCase(unittest.TestCase): self.assertIn(expected_warning, git_cl.sys.stdout.getvalue()) -class CMDUploadTestCase(unittest.TestCase): - +class CMDUploadTestCase(CMDTestCaseBase): def setUp(self): super(CMDUploadTestCase, self).setUp() - mock.patch('git_cl.sys.stdout', StringIO.StringIO()).start() - mock.patch('git_cl.uuid.uuid4', _constantFn('uuid4')).start() - mock.patch('git_cl.Changelist.GetIssue', _constantFn(123456)).start() - mock.patch('git_cl.Changelist.GetCodereviewServer', - _constantFn('https://chromium-review.googlesource.com')).start() - mock.patch('git_cl.Changelist.GetMostRecentPatchset', - _constantFn(7)).start() - mock.patch('git_cl.auth.get_authenticator_for_host', - AuthenticatorMock()).start() - mock.patch('git_common.is_dirty_git_tree', return_value=False).start() + mock.patch('git_cl.fetch_try_jobs').start() + mock.patch('git_cl._trigger_try_jobs', return_value={}).start() + mock.patch('git_cl.Changelist.CMDUpload', return_value=0).start() self.addCleanup(mock.patch.stopall) - @mock.patch('git_cl.fetch_try_jobs') - @mock.patch('git_cl._trigger_try_jobs') - @mock.patch('git_cl.Changelist._GetChangeDetail') - @mock.patch('git_cl.Changelist.CMDUpload', _constantFn(0)) - def testUploadRetryFailed(self, mockGetChangeDetail, mockTriggerTryJobs, - mockFetchTryJobs): + def testUploadRetryFailed(self): # This test mocks out the actual upload part, and just asserts that after # upload, if --retry-failed is added, then the tool will fetch try jobs # from the previous patchset and trigger the right builders on the latest # patchset. - mockGetChangeDetail.return_value = { - 'project': 'depot_tools', - 'status': 'OPEN', - 'owner': {'email': 'owner@e.mail'}, - 'current_revision': 'beeeeeef', - 'revisions': { - 'deadbeaf': { - '_number': 6, - }, - 'beeeeeef': { - '_number': 7, - 'fetch': {'http': { - 'url': 'https://chromium.googlesource.com/depot_tools', - 'ref': 'refs/changes/56/123456/7' - }}, - }, - }, - } - mockFetchTryJobs.side_effect = [ - # Prior patchset - no builds. - {}, - # Prior to prior patchset -- some builds. - { - '9000': { - 'id': '9000', - 'project': 'infra', - 'bucket': 'luci.infra.try', - 'created_by': 'user:someone@chromium.org', - 'created_ts': '147200002222000', - 'experimental': False, - 'parameters_json': json.dumps({ - 'builder_name': 'red-bot', - 'properties': {'category': 'cq'}, - }), - 'status': 'COMPLETED', - 'result': 'FAILURE', - 'tags': ['user_agent:cq'], - }, - 8000: { - 'id': '8000', - 'project': 'infra', - 'bucket': 'luci.infra.try', - 'created_by': 'user:someone@chromium.org', - 'created_ts': '147200002222020', - 'experimental': False, - 'parameters_json': json.dumps({ - 'builder_name': 'green-bot', - 'properties': {'category': 'cq'}, - }), - 'status': 'COMPLETED', - 'result': 'SUCCESS', - 'tags': ['user_agent:cq'], - }, - }, + git_cl.fetch_try_jobs.side_effect = [ + # Latest patchset: No builds. + [], + # Patchset before latest: Some builds. + [ + { + 'id': str(100 + idx), + 'builder': { + 'project': 'chromium', + 'bucket': 'try', + 'builder': 'bot_' + status.lower(), + }, + 'status': status, + } + for idx, status in enumerate(self._STATUSES) + ], ] + self.assertEqual(0, git_cl.main(['upload', '--retry-failed'])) - mockFetchTryJobs.assert_has_calls([ + self.assertEqual([ mock.call(mock.ANY, mock.ANY, 'cr-buildbucket.appspot.com', patchset=7), mock.call(mock.ANY, mock.ANY, 'cr-buildbucket.appspot.com', patchset=6), - ]) - buckets = {'infra/try': {'red-bot': []}} - mockTriggerTryJobs.assert_called_once_with( - mock.ANY, mock.ANY, buckets, mock.ANY, 8) + ], git_cl.fetch_try_jobs.mock_calls) + expected_buckets = { + 'chromium/try': {'bot_failure': [], 'bot_infra_failure': []}, + } + git_cl._trigger_try_jobs.assert_called_once_with( + mock.ANY, mock.ANY, expected_buckets, mock.ANY, 8) if __name__ == '__main__': logging.basicConfig(