From 136b49ff7e09d851bc677495a37bea1d87e102a7 Mon Sep 17 00:00:00 2001 From: qyearsley Date: Mon, 31 Oct 2016 09:02:26 -0700 Subject: [PATCH] depot_tools: Remove DoGetTrySlaves, GetTrySlavesExecuter, GetPreferredTrySlaves Note: This CL originally just removed a deprecated use of Remove use of DoGetTrySlaves, suggested in http://crrev.com/2442153002, then was expanded to remove DoGetTrySlaves, GetPreferredTrySlaves and GetTrySlavesExecuter since these are all deprecated and unused. BUG=660453 Review-Url: https://codereview.chromium.org/2453823002 --- git_cl.py | 24 +----- presubmit_support.py | 135 ---------------------------------- tests/git_cl_test.py | 4 - tests/presubmit_unittest.py | 141 +----------------------------------- 4 files changed, 4 insertions(+), 300 deletions(-) diff --git a/git_cl.py b/git_cl.py index 1321d2f49..79fceab24 100755 --- a/git_cl.py +++ b/git_cl.py @@ -349,8 +349,8 @@ def _get_bucket_map(changelist, options, option_parser): if not options.bot: change = changelist.GetChange( changelist.GetCommonAncestorWithUpstream(), None) - - masters = presubmit_support.DoGetTryMasters( + # Get try masters from PRESUBMIT.py files. + return presubmit_support.DoGetTryMasters( change=change, changed_files=change.LocalPaths(), repository_root=settings.GetRoot(), @@ -359,26 +359,6 @@ def _get_bucket_map(changelist, options, option_parser): verbose=options.verbose, output_stream=sys.stdout) - if masters: - return masters - - # Fall back to deprecated method: get try slaves from PRESUBMIT.py - # files. - # TODO(qyearsley): Remove this. - options.bot = presubmit_support.DoGetTrySlaves( - change=change, - changed_files=change.LocalPaths(), - repository_root=settings.GetRoot(), - default_presubmit=None, - project=None, - verbose=options.verbose, - output_stream=sys.stdout) - - if not options.bot: - return {} - - # If a bucket or master is passed, then we assume all bots are under - # that one master. if options.bucket: return {options.bucket: {b: [] for b in options.bot}} if options.master: diff --git a/presubmit_support.py b/presubmit_support.py index fe3de7b05..ca54dd14b 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -1084,83 +1084,6 @@ def ListRelevantPresubmitFiles(files, root): return results -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. - presubmit_path: Project script to run. - project: Project name to pass to presubmit script for bot selection. - - Return: - A list of try slaves. - """ - context = {} - main_path = os.getcwd() - try: - os.chdir(os.path.dirname(presubmit_path)) - exec script_text in context - except Exception, e: - raise PresubmitFailure('"%s" had an exception.\n%s' % (presubmit_path, e)) - finally: - os.chdir(main_path) - - function_name = 'GetPreferredTrySlaves' - if function_name in context: - get_preferred_try_slaves = context[function_name] - function_info = inspect.getargspec(get_preferred_try_slaves) - if len(function_info[0]) == 1: - result = get_preferred_try_slaves(project) - elif len(function_info[0]) == 2: - result = get_preferred_try_slaves(project, change) - else: - result = get_preferred_try_slaves() - if not isinstance(result, types.ListType): - raise PresubmitFailure( - 'Presubmit functions must return a list, got a %s instead: %s' % - (type(result), str(result))) - for item in result: - if isinstance(item, basestring): - # Old-style ['bot'] format. - botname = item - elif isinstance(item, tuple): - # New-style [('bot', set(['tests']))] format. - botname = item[0] - else: - raise PresubmitFailure('PRESUBMIT.py returned invalid tryslave/test' - ' format.') - - if botname != botname.strip(): - raise PresubmitFailure( - 'Try slave names cannot start/end with whitespace') - if ',' in botname: - raise PresubmitFailure( - 'Do not use \',\' separated builder or test names: %s' % botname) - else: - result = [] - - def valid_oldstyle(result): - return all(isinstance(i, basestring) for i in result) - - def valid_newstyle(result): - return (all(isinstance(i, tuple) for i in result) and - all(len(i) == 2 for i in result) and - all(isinstance(i[0], basestring) for i in result) and - all(isinstance(i[1], set) for i in result) - ) - - # Ensure it's either all old-style or all new-style. - if not valid_oldstyle(result) and not valid_newstyle(result): - raise PresubmitFailure( - 'PRESUBMIT.py returned invalid trybot specification!') - - return result - - class GetTryMastersExecuter(object): @staticmethod def ExecPresubmitScript(script_text, presubmit_path, project, change): @@ -1222,64 +1145,6 @@ class GetPostUploadExecuter(object): return post_upload_hook(cl, change, OutputApi(False)) -def DoGetTrySlaves(change, - changed_files, - repository_root, - default_presubmit, - project, - verbose, - output_stream): - """Get the list of try servers from the presubmit scripts (deprecated). - - 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: - List of try slaves - """ - presubmit_files = ListRelevantPresubmitFiles(changed_files, repository_root) - if not presubmit_files and verbose: - output_stream.write("Warning, no PRESUBMIT.py found.\n") - results = [] - executer = GetTrySlavesExecuter() - - if default_presubmit: - if verbose: - output_stream.write("Running default presubmit script.\n") - fake_path = os.path.join(repository_root, 'PRESUBMIT.py') - results.extend(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.extend(executer.ExecPresubmitScript( - presubmit_script, filename, project, change)) - - - slave_dict = {} - old_style = filter(lambda x: isinstance(x, basestring), results) - new_style = filter(lambda x: isinstance(x, tuple), results) - - for result in new_style: - slave_dict.setdefault(result[0], set()).update(result[1]) - slaves = list(slave_dict.items()) - - slaves.extend(set(old_style)) - - if slaves and verbose: - output_stream.write(', '.join((str(x) for x in slaves))) - output_stream.write('\n') - return slaves - - def _MergeMasters(masters1, masters2): """Merges two master maps. Merges also the tests of each builder.""" result = {} diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 4b4e295e2..8694f7b2f 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -1885,9 +1885,6 @@ class TestGitCl(TestCase): self.mock(git_cl.presubmit_support, 'DoGetTryMasters', lambda *_, **__: ( self._mocked_call(['DoGetTryMasters']))) - self.mock(git_cl.presubmit_support, 'DoGetTrySlaves', - lambda *_, **__: ( - self._mocked_call(['DoGetTrySlaves']))) self.mock(git_cl._RietveldChangelistImpl, 'SetCQState', lambda _, s: self._mocked_call(['SetCQState', s])) self.calls = [ @@ -1906,7 +1903,6 @@ class TestGitCl(TestCase): '', '', '', '', '', '', '', '')), ((['git', 'rev-parse', '--show-cdup'],), '../'), ((['DoGetTryMasters'], ), None), - ((['DoGetTrySlaves'], ), None), ((['SetCQState', git_cl._CQState.DRY_RUN], ), None), ] out = StringIO.StringIO() diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 807bad337..ad1219b30 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -49,18 +49,6 @@ def CheckChangeOnUpload(input_api, output_api): else: return () """ - presubmit_tryslave = """ -def GetPreferredTrySlaves(): - return %s -""" - - presubmit_tryslave_project = """ -def GetPreferredTrySlaves(project): - if project == %s: - return %s - else: - return %s -""" presubmit_trymaster = """ def GetPreferredTryMasters(project, change): @@ -170,9 +158,9 @@ class PresubmitUnittest(PresubmitTestsBase): def testMembersChanged(self): self.mox.ReplayAll() members = [ - 'AffectedFile', 'Change', 'DoGetTrySlaves', + 'AffectedFile', 'Change', 'DoPostUploadExecuter', 'DoPresubmitChecks', 'GetPostUploadExecuter', - 'GetTrySlavesExecuter', 'GitAffectedFile', 'CallCommand', 'CommandData', + 'GitAffectedFile', 'CallCommand', 'CommandData', 'GitChange', 'InputApi', 'ListRelevantPresubmitFiles', 'main', 'NonexistantCannedCheckFilter', 'OutputApi', 'ParseFiles', 'PresubmitFailure', 'PresubmitExecuter', 'PresubmitOutput', 'ScanSubDirs', @@ -963,131 +951,6 @@ def CheckChangeOnCommit(input_api, output_api): '\n' 'Presubmit checks passed.\n')) - def testGetTrySlavesExecuter(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.GetTrySlavesExecuter() - self.assertEqual([], executer.ExecPresubmitScript('', '', '', change)) - self.assertEqual([], - executer.ExecPresubmitScript('def foo():\n return\n', '', '', change)) - - # bad results - starts_with_space_result = [' starts_with_space'] - not_list_result1 = "'foo'" - not_list_result2 = "('a', 'tuple')" - mixed_old_and_new = ['bot', ('bot2', set(['test']))] - not_set = [('bot2', ['test'])] - for result in ( - starts_with_space_result, not_list_result1, not_list_result2, - mixed_old_and_new, not_set): - self.assertRaises(presubmit.PresubmitFailure, - executer.ExecPresubmitScript, - self.presubmit_tryslave % result, '', '', change) - - # good results - expected_result = ['1', '2', '3'] - empty_result = [] - space_in_name_result = ['foo bar', '1\t2 3'] - new_style = [('bot', set(['cool', 'tests']))] - for result in ( - expected_result, empty_result, space_in_name_result, new_style): - self.assertEqual( - result, - executer.ExecPresubmitScript( - self.presubmit_tryslave % result, '', '', change)) - - def testGetTrySlavesExecuterWithProject(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.GetTrySlavesExecuter() - expected_result1 = ['1', '2'] - expected_result2 = ['a', 'b', 'c'] - script = self.presubmit_tryslave_project % ( - repr('foo'), repr(expected_result1), repr(expected_result2)) - self.assertEqual( - expected_result1, executer.ExecPresubmitScript(script, '', 'foo', - change)) - self.assertEqual( - expected_result2, executer.ExecPresubmitScript(script, '', 'bar', - change)) - - def testDoGetTrySlaves(self): - join = presubmit.os.path.join - 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 = presubmit.os.path.join(self.fake_root_dir, - self._INHERIT_SETTINGS) - - presubmit.os.path.isfile(inherit_path).AndReturn(False) - presubmit.os.listdir(self.fake_root_dir).AndReturn(['PRESUBMIT.py']) - presubmit.os.path.isfile(root_presubmit).AndReturn(True) - presubmit.gclient_utils.FileRead(root_presubmit, 'rU').AndReturn( - self.presubmit_tryslave % '["win"]') - - presubmit.os.path.isfile(inherit_path).AndReturn(False) - presubmit.os.listdir(self.fake_root_dir).AndReturn(['PRESUBMIT.py']) - presubmit.os.path.isfile(root_presubmit).AndReturn(True) - presubmit.os.listdir(join(self.fake_root_dir, 'linux_only')).AndReturn( - ['PRESUBMIT.py']) - presubmit.os.path.isfile(linux_presubmit).AndReturn(True) - presubmit.gclient_utils.FileRead(root_presubmit, 'rU').AndReturn( - self.presubmit_tryslave % '["win"]') - presubmit.gclient_utils.FileRead(linux_presubmit, 'rU').AndReturn( - self.presubmit_tryslave % '["linux"]') - self.mox.ReplayAll() - - change = presubmit.Change( - 'mychange', '', self.fake_root_dir, [], 0, 0, None) - - output = StringIO.StringIO() - self.assertEqual(['win'], - presubmit.DoGetTrySlaves(change, [filename], - self.fake_root_dir, - None, None, False, output)) - output = StringIO.StringIO() - self.assertEqual(['win', 'linux'], - presubmit.DoGetTrySlaves(change, - [filename, filename_linux], - self.fake_root_dir, None, None, - False, output)) - - def testGetTrySlavesExecuter_ok(self): - script_text = ( - 'def GetPreferredTrySlaves():\n' - ' return ["foo", "bar"]\n') - results = presubmit.GetTrySlavesExecuter.ExecPresubmitScript( - script_text, 'path', 'project', None) - self.assertEquals(['foo', 'bar'], results) - - def testGetTrySlavesExecuter_comma(self): - script_text = ( - 'def GetPreferredTrySlaves():\n' - ' return ["foo,bar"]\n') - try: - presubmit.GetTrySlavesExecuter.ExecPresubmitScript( - script_text, 'path', 'project', None) - self.fail() - except presubmit.PresubmitFailure: - pass - def testGetTryMastersExecuter(self): self.mox.ReplayAll() change = presubmit.Change(