From 58a69cbe01c318d91752a21effad0c46c38b521a Mon Sep 17 00:00:00 2001 From: "machenbach@chromium.org" Date: Sat, 1 Mar 2014 02:08:29 +0000 Subject: [PATCH] Support multiple try masters when sending tries to rietveld. BUG= Review URL: https://codereview.chromium.org/178223016 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@254321 0039d316-1c4b-4281-b951-d872f2087c98 --- git_cl.py | 119 +++++++++++++++++++++++------------- presubmit_support.py | 97 ++++++++++++++++++++++++++++- rietveld.py | 28 ++++++++- tests/presubmit_unittest.py | 106 +++++++++++++++++++++++++++++++- 4 files changed, 302 insertions(+), 48 deletions(-) diff --git a/git_cl.py b/git_cl.py index ef883e139..e8c769f49 100755 --- a/git_cl.py +++ b/git_cl.py @@ -2084,6 +2084,9 @@ def CMDtry(parser, args): "the try server waterfall for the builders name and the tests " "available. Can also be used to specify gtest_filter, e.g. " "-bwin_rel:base_unittests:ValuesTest.*Value")) + group.add_option( + "-m", "--master", default='', + help=("Specify a try master where to run the tries.")) group.add_option( "-r", "--revision", help="Revision to use for the try job; default: the " @@ -2117,50 +2120,74 @@ def CMDtry(parser, args): if not options.name: options.name = cl.GetBranch() - # Process --bot and --testfilter. - if not options.bot: - # Get try slaves from PRESUBMIT.py files if not specified. - change = cl.GetChange(cl.GetCommonAncestorWithUpstream(), None) - options.bot = presubmit_support.DoGetTrySlaves( - change, - change.LocalPaths(), - settings.GetRoot(), - None, - None, - options.verbose, - sys.stdout) - if not options.bot: - parser.error('No default try builder to try, use --bot') - - builders_and_tests = {} - old_style = filter(lambda x: isinstance(x, basestring), options.bot) - new_style = filter(lambda x: isinstance(x, tuple), options.bot) - - for bot in old_style: - if ':' in bot: - builder, tests = bot.split(':', 1) - builders_and_tests.setdefault(builder, []).extend(tests.split(',')) - elif ',' in bot: - parser.error('Specify one bot per --bot flag') - else: - builders_and_tests.setdefault(bot, []).append('defaulttests') + def GetMasterMap(): + # Process --bot and --testfilter. + if not options.bot: + change = cl.GetChange(cl.GetCommonAncestorWithUpstream(), None) + + # Get try masters from PRESUBMIT.py files. + masters = presubmit_support.DoGetTryMasters( + change, + change.LocalPaths(), + settings.GetRoot(), + None, + None, + options.verbose, + sys.stdout) + if masters: + return masters + + # Fall back to deprecated method: get try slaves from PRESUBMIT.py files. + options.bot = presubmit_support.DoGetTrySlaves( + change, + change.LocalPaths(), + settings.GetRoot(), + None, + None, + options.verbose, + sys.stdout) + if not options.bot: + parser.error('No default try builder to try, use --bot') + + builders_and_tests = {} + # TODO(machenbach): The old style command-line options don't support + # multiple try masters yet. + old_style = filter(lambda x: isinstance(x, basestring), options.bot) + new_style = filter(lambda x: isinstance(x, tuple), options.bot) + + for bot in old_style: + if ':' in bot: + builder, tests = bot.split(':', 1) + builders_and_tests.setdefault(builder, []).extend(tests.split(',')) + elif ',' in bot: + parser.error('Specify one bot per --bot flag') + else: + builders_and_tests.setdefault(bot, []).append('defaulttests') + + for bot, tests in new_style: + builders_and_tests.setdefault(bot, []).extend(tests) + + # Return a master map with one master to be backwards compatible. The + # master name defaults to an empty string, which will cause the master + # not to be set on rietveld (deprecated). + return {options.master: builders_and_tests} - for bot, tests in new_style: - builders_and_tests.setdefault(bot, []).extend(tests) + masters = GetMasterMap() if options.testfilter: forced_tests = sum((t.split(',') for t in options.testfilter), []) - builders_and_tests = dict( - (b, forced_tests) for b, t in builders_and_tests.iteritems() - if t != ['compile']) + masters = dict((master, dict( + (b, forced_tests) for b, t in slaves.iteritems() + if t != ['compile'])) for master, slaves in masters.iteritems()) - if any('triggered' in b for b in builders_and_tests): - print >> sys.stderr, ( - 'ERROR You are trying to send a job to a triggered bot. This type of' - ' bot requires an\ninitial job from a parent (usually a builder). ' - 'Instead send your job to the parent.\n' - 'Bot list: %s' % builders_and_tests) - return 1 + for builders in masters.itervalues(): + if any('triggered' in b for b in builders): + print >> sys.stderr, ( + 'ERROR You are trying to send a job to a triggered bot. This type of' + ' bot requires an\ninitial job from a parent (usually a builder). ' + 'Instead send your job to the parent.\n' + 'Bot list: %s' % builders) + return 1 patchset = cl.GetMostRecentPatchset() if patchset and patchset != cl.GetPatchset(): @@ -2169,18 +2196,22 @@ def CMDtry(parser, args): 'upload fail?\ngit-cl try always uses latest patchset from rietveld. ' 'Continuing using\npatchset %s.\n' % patchset) try: - cl.RpcServer().trigger_try_jobs( + cl.RpcServer().trigger_distributed_try_jobs( cl.GetIssue(), patchset, options.name, options.clobber, - options.revision, builders_and_tests) + options.revision, masters) except urllib2.HTTPError, e: if e.code == 404: print('404 from rietveld; ' 'did you mean to use "git try" instead of "git cl try"?') return 1 print('Tried jobs on:') - length = max(len(builder) for builder in builders_and_tests) - for builder in sorted(builders_and_tests): - print ' %*s: %s' % (length, builder, ','.join(builders_and_tests[builder])) + + for (master, builders) in masters.iteritems(): + if master: + print 'Master: %s' % master + length = max(len(builder) for builder in builders) + for builder in sorted(builders): + print ' %*s: %s' % (length, builder, ','.join(builders[builder])) return 0 diff --git a/presubmit_support.py b/presubmit_support.py index dabfbf0ba..513b29f7c 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -19,6 +19,7 @@ import contextlib import fnmatch import glob import inspect +import itertools import json # Exposed through the API. import logging import marshal # Exposed through the API. @@ -1007,6 +1008,8 @@ class GetTrySlavesExecuter(object): @staticmethod def ExecPresubmitScript(script_text, presubmit_path, project, change): """Executes GetPreferredTrySlaves() from a single presubmit script. + + This will soon be deprecated and replaced by GetPreferredTryMasters(). Args: script_text: The text of the presubmit script. @@ -1074,6 +1077,36 @@ class GetTrySlavesExecuter(object): return result +class GetTryMastersExecuter(object): + @staticmethod + def ExecPresubmitScript(script_text, presubmit_path, project, change): + """Executes GetPreferredTryMasters() from a single presubmit script. + + Args: + script_text: The text of the presubmit script. + presubmit_path: Project script to run. + project: Project name to pass to presubmit script for bot selection. + + Return: + A map of try masters to map of builders to set of tests. + """ + context = {} + try: + exec script_text in context + except Exception, e: + raise PresubmitFailure('"%s" had an exception.\n%s' + % (presubmit_path, e)) + + function_name = 'GetPreferredTryMasters' + if function_name not in context: + return {} + get_preferred_try_masters = context[function_name] + if not len(inspect.getargspec(get_preferred_try_masters)[0]) == 2: + raise PresubmitFailure( + 'Expected function "GetPreferredTryMasters" to take two arguments.') + return get_preferred_try_masters(project, change) + + def DoGetTrySlaves(change, changed_files, repository_root, @@ -1081,7 +1114,7 @@ def DoGetTrySlaves(change, project, verbose, output_stream): - """Get the list of try servers from the presubmit scripts. + """Get the list of try servers from the presubmit scripts (deprecated). Args: changed_files: List of modified files. @@ -1132,6 +1165,68 @@ def DoGetTrySlaves(change, return slaves +def _MergeMasters(masters1, masters2): + """Merges two master maps. Merges also the tests of each builder.""" + result = {} + for (master, builders) in itertools.chain(masters1.iteritems(), + masters2.iteritems()): + new_builders = result.setdefault(master, {}) + for (builder, tests) in builders.iteritems(): + new_builders.setdefault(builder, set([])).update(tests) + return result + + +def DoGetTryMasters(change, + changed_files, + repository_root, + default_presubmit, + project, + verbose, + output_stream): + """Get the list of try masters from the presubmit scripts. + + Args: + changed_files: List of modified files. + repository_root: The repository root. + default_presubmit: A default presubmit script to execute in any case. + project: Optional name of a project used in selecting trybots. + verbose: Prints debug info. + output_stream: A stream to write debug output to. + + Return: + Map of try masters to map of builders to set of tests. + """ + presubmit_files = ListRelevantPresubmitFiles(changed_files, repository_root) + if not presubmit_files and verbose: + output_stream.write("Warning, no PRESUBMIT.py found.\n") + results = {} + executer = GetTryMastersExecuter() + + if default_presubmit: + if verbose: + output_stream.write("Running default presubmit script.\n") + fake_path = os.path.join(repository_root, 'PRESUBMIT.py') + results = _MergeMasters(results, executer.ExecPresubmitScript( + default_presubmit, fake_path, project, change)) + for filename in presubmit_files: + filename = os.path.abspath(filename) + if verbose: + output_stream.write("Running %s\n" % filename) + # Accept CRLF presubmit script. + presubmit_script = gclient_utils.FileRead(filename, 'rU') + results = _MergeMasters(results, executer.ExecPresubmitScript( + presubmit_script, filename, project, change)) + + # Make sets to lists again for later JSON serialization. + for builders in results.itervalues(): + for builder in builders: + builders[builder] = list(builders[builder]) + + if results and verbose: + output_stream.write('%s\n' % str(results)) + return results + + class PresubmitExecuter(object): def __init__(self, change, committing, rietveld_obj, verbose): """ diff --git a/rietveld.py b/rietveld.py index aab8e5014..aad998e9c 100644 --- a/rietveld.py +++ b/rietveld.py @@ -331,10 +331,12 @@ class Rietveld(object): cursor = '&cursor=%s' % data['cursor'] def trigger_try_jobs( - self, issue, patchset, reason, clobber, revision, builders_and_tests): + self, issue, patchset, reason, clobber, revision, builders_and_tests, + master=None): """Requests new try jobs. |builders_and_tests| is a map of builders: [tests] to run. + |master| is the name of the try master the builders belong to. Returns the keys of the new TryJobResult entites. """ @@ -346,8 +348,24 @@ class Rietveld(object): ] if revision: params.append(('revision', revision)) + if master: + # Temporarily allow empty master names for old configurations. The try + # job will not be associated with a master name on rietveld. This is + # going to be deprecated. + params.append(('master', master)) return self.post('/%d/try/%d' % (issue, patchset), params) + def trigger_distributed_try_jobs( + self, issue, patchset, reason, clobber, revision, masters): + """Requests new try jobs. + + |masters| is a map of masters: map of builders: [tests] to run. + """ + for (master, builders_and_tests) in masters.iteritems(): + self.trigger_try_jobs( + issue, patchset, reason, clobber, revision, builders_and_tests, + master) + def get_pending_try_jobs(self, cursor=None, limit=100): """Retrieves the try job requests in pending state. @@ -539,6 +557,12 @@ class ReadOnlyRietveld(object): ReadOnlyRietveld._local_changes.setdefault(issue, {})[flag] = value def trigger_try_jobs( # pylint:disable=R0201 - self, issue, patchset, reason, clobber, revision, builders_and_tests): + self, issue, patchset, reason, clobber, revision, builders_and_tests, + master=None): logging.info('ReadOnlyRietveld: triggering try jobs %r for issue %d' % (builders_and_tests, issue)) + + def trigger_distributed_try_jobs( # pylint:disable=R0201 + self, issue, patchset, reason, clobber, revision, masters): + logging.info('ReadOnlyRietveld: triggering try jobs %r for issue %d' % + (masters, issue)) diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 28bf5bcec..6c2c8b3a6 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -8,6 +8,7 @@ # pylint: disable=E1101,E1103 import functools +import itertools import logging import os import StringIO @@ -60,6 +61,11 @@ def GetPreferredTrySlaves(project): return %s """ + presubmit_trymaster = """ +def GetPreferredTryMasters(project, change): + return %s +""" + presubmit_diffs = """ --- file1 2011-02-09 10:38:16.517224845 -0800 +++ file2 2011-02-09 10:38:53.177226516 -0800 @@ -170,7 +176,8 @@ class PresubmitUnittest(PresubmitTestsBase): 'marshal', 'normpath', 'optparse', 'os', 'owners', 'pickle', 'presubmit_canned_checks', 'random', 're', 'rietveld', 'scm', 'subprocess', 'sys', 'tempfile', 'time', 'traceback', 'types', 'unittest', - 'urllib2', 'warn', 'multiprocessing', + 'urllib2', 'warn', 'multiprocessing', 'DoGetTryMasters', + 'GetTryMastersExecuter', 'itertools', ] # If this test fails, you should add the relevant test. self.compareMembers(presubmit, members) @@ -1027,6 +1034,103 @@ def CheckChangeOnCommit(input_api, output_api): except presubmit.PresubmitFailure: pass + def testGetTryMastersExecuter(self): + self.mox.ReplayAll() + change = presubmit.Change( + 'foo', + 'Blah Blah\n\nSTORY=http://tracker.com/42\nBUG=boo\n', + self.fake_root_dir, + None, + 0, + 0, + None) + executer = presubmit.GetTryMastersExecuter() + self.assertEqual({}, executer.ExecPresubmitScript('', '', '', change)) + self.assertEqual({}, + executer.ExecPresubmitScript('def foo():\n return\n', '', '', change)) + + expected_result = {'m1': {'s1': set(['t1', 't2'])}, + 'm2': {'s1': set(['defaulttests']), + 's2': set(['defaulttests'])}} + empty_result1 = {} + empty_result2 = {'m': {}} + space_in_name_result = {'m r': {'s\tv': set(['t1'])}} + for result in ( + expected_result, empty_result1, empty_result2, space_in_name_result): + self.assertEqual( + result, + executer.ExecPresubmitScript( + self.presubmit_trymaster % result, '', '', change)) + + def testMergeMasters(self): + merge = presubmit._MergeMasters + self.assertEqual({}, merge({}, {})) + self.assertEqual({'m1': {}}, merge({}, {'m1': {}})) + self.assertEqual({'m1': {}}, merge({'m1': {}}, {})) + parts = [ + {'try1.cr': {'win': set(['defaulttests'])}}, + {'try1.cr': {'linux1': set(['test1'])}, + 'try2.cr': {'linux2': set(['defaulttests'])}}, + {'try1.cr': {'mac1': set(['defaulttests']), + 'mac2': set(['test1', 'test2']), + 'linux1': set(['defaulttests'])}}, + ] + expected = { + 'try1.cr': {'win': set(['defaulttests']), + 'linux1': set(['defaulttests', 'test1']), + 'mac1': set(['defaulttests']), + 'mac2': set(['test1', 'test2'])}, + 'try2.cr': {'linux2': set(['defaulttests'])}, + } + for permutation in itertools.permutations(parts): + self.assertEqual(expected, reduce(merge, permutation, {})) + + def testDoGetTryMasters(self): + root_text = (self.presubmit_trymaster + % '{"t1.cr": {"win": set(["defaulttests"])}}') + linux_text = (self.presubmit_trymaster + % ('{"t1.cr": {"linux1": set(["t1"])},' + ' "t2.cr": {"linux2": set(["defaulttests"])}}')) + + join = presubmit.os.path.join + isfile = presubmit.os.path.isfile + FileRead = presubmit.gclient_utils.FileRead + filename = 'foo.cc' + filename_linux = join('linux_only', 'penguin.cc') + root_presubmit = join(self.fake_root_dir, 'PRESUBMIT.py') + linux_presubmit = join(self.fake_root_dir, 'linux_only', 'PRESUBMIT.py') + inherit_path = join(self.fake_root_dir, self._INHERIT_SETTINGS) + + isfile(inherit_path).AndReturn(False) + isfile(root_presubmit).AndReturn(True) + FileRead(root_presubmit, 'rU').AndReturn(root_text) + + isfile(inherit_path).AndReturn(False) + isfile(root_presubmit).AndReturn(True) + isfile(linux_presubmit).AndReturn(True) + FileRead(root_presubmit, 'rU').AndReturn(root_text) + FileRead(linux_presubmit, 'rU').AndReturn(linux_text) + self.mox.ReplayAll() + + change = presubmit.Change( + 'mychange', '', self.fake_root_dir, [], 0, 0, None) + + output = StringIO.StringIO() + self.assertEqual({'t1.cr': {'win': ['defaulttests']}}, + presubmit.DoGetTryMasters(change, [filename], + self.fake_root_dir, + None, None, False, output)) + output = StringIO.StringIO() + expected = { + 't1.cr': {'win': ['defaulttests'], 'linux1': ['t1']}, + 't2.cr': {'linux2': ['defaulttests']}, + } + self.assertEqual(expected, + presubmit.DoGetTryMasters(change, + [filename, filename_linux], + self.fake_root_dir, None, None, + False, output)) + def testMainUnversioned(self): # OptParser calls presubmit.os.path.exists and is a pain when mocked. self.UnMock(presubmit.os.path, 'exists')