From 668c1d8d1f1a6c26b088b5bff1203e35c74943a0 Mon Sep 17 00:00:00 2001 From: Aaron Gable Date: Tue, 3 Apr 2018 10:19:16 -0700 Subject: [PATCH] Reland "Remove Rietveld support from presubmit" Since no one can upload or land changes from Rietveld anymore, PRESUBMIT and its support files no longer need to be able to support it. Bug: 770408 Change-Id: I4ca6391291e7b0755c78af453c3d006ad3666a17 Reviewed-on: https://chromium-review.googlesource.com/991053 Reviewed-by: Robbie Iannucci Commit-Queue: Aaron Gable --- git_cl.py | 27 ------ presubmit_canned_checks.py | 76 +++------------- presubmit_support.py | 74 ++------------- tests/presubmit_unittest.py | 174 ++++++++++++++---------------------- 4 files changed, 88 insertions(+), 263 deletions(-) diff --git a/git_cl.py b/git_cl.py index d56d0721c..e90b93f5c 100755 --- a/git_cl.py +++ b/git_cl.py @@ -1525,7 +1525,6 @@ class Changelist(object): return presubmit_support.DoPresubmitChecks(change, committing, verbose=verbose, output_stream=sys.stdout, input_stream=sys.stdin, default_presubmit=None, may_prompt=may_prompt, - rietveld_obj=self._codereview_impl.GetRietveldObjForPresubmit(), gerrit_obj=self._codereview_impl.GetGerritObjForPresubmit()) except presubmit_support.PresubmitFailure as e: DieWithError('%s\nMaybe your depot_tools is out of date?' % e) @@ -1807,11 +1806,6 @@ class _ChangelistCodereviewBase(object): """Which branch-specific properties to erase when unsetting issue.""" return [] - def GetRietveldObjForPresubmit(self): - # This is an unfortunate Rietveld-embeddedness in presubmit. - # For non-Rietveld code reviews, this probably should return a dummy object. - raise NotImplementedError() - def GetGerritObjForPresubmit(self): # None is valid return value, otherwise presubmit_support.GerritAccessor. return None @@ -2129,9 +2123,6 @@ class _RietveldChangelistImpl(_ChangelistCodereviewBase): def CodereviewServerConfigKey(cls): return 'rietveldserver' - def GetRietveldObjForPresubmit(self): - return self.RpcServer() - def SetLabels(self, enable_auto_submit, use_commit_queue, cq_dry_run): raise NotImplementedError() @@ -2504,24 +2495,6 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): """Which branch-specific properties to erase when unsetting issue.""" return ['gerritsquashhash'] - def GetRietveldObjForPresubmit(self): - class ThisIsNotRietveldIssue(object): - def __nonzero__(self): - # This is a hack to make presubmit_support think that rietveld is not - # defined, yet still ensure that calls directly result in a decent - # exception message below. - return False - - def __getattr__(self, attr): - print( - 'You aren\'t using Rietveld at the moment, but Gerrit.\n' - 'Using Rietveld in your PRESUBMIT scripts won\'t work.\n' - 'Please, either change your PRESUBMIT to not use rietveld_obj.%s,\n' - 'or use Rietveld for codereview.\n' - 'See also http://crbug.com/579160.' % attr) - raise NotImplementedError() - return ThisIsNotRietveldIssue() - def GetGerritObjForPresubmit(self): return presubmit_support.GerritAccessor(self._GetGerritHost()) diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index d009c9fe1..f1f61834f 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -793,15 +793,6 @@ def RunPylint(input_api, *args, **kwargs): return input_api.RunTests(GetPylint(input_api, *args, **kwargs), False) -def CheckRietveldTryJobExecution(dummy_input_api, output_api, - dummy_host_url, dummy_platforms, - dummy_owner): - return [ - output_api.PresubmitNotifyResult( - 'CheckRietveldTryJobExecution is deprecated, please remove it.') - ] - - def CheckBuildbotPendingBuilds(input_api, output_api, url, max_pendings, ignored): try: @@ -895,25 +886,25 @@ def CheckOwners(input_api, output_api, source_file_filter=None): return [output_fn('Missing LGTM from someone other than %s' % owner_email)] return [] + def GetCodereviewOwnerAndReviewers(input_api, email_regexp, approval_needed): """Return the owner and reviewers of a change, if any. If approval_needed is True, only reviewers who have approved the change will be returned. """ - # Rietveld is default. - func = _RietveldOwnerAndReviewers - if input_api.gerrit: - func = _GerritOwnerAndReviewers - return func(input_api, email_regexp, approval_needed) - - -def _GetRietveldIssueProps(input_api, messages): - """Gets the issue properties from rietveld.""" issue = input_api.change.issue - if issue and input_api.rietveld: - return input_api.rietveld.get_issue_properties( - issue=int(issue), messages=messages) + if not issue: + return None, (set() if approval_needed else + _ReviewersFromChange(input_api.change)) + + owner_email = input_api.gerrit.GetChangeOwner(issue) + reviewers = set( + r for r in input_api.gerrit.GetChangeReviewers(issue, approval_needed) + if _match_reviewer_email(r, owner_email, email_regexp)) + input_api.logging.debug('owner: %s; approvals given by: %s', + owner_email, ', '.join(sorted(reviewers))) + return owner_email, reviewers def _ReviewersFromChange(change): @@ -929,49 +920,6 @@ def _ReviewersFromChange(change): def _match_reviewer_email(r, owner_email, email_regexp): return email_regexp.match(r) and r != owner_email -def _RietveldOwnerAndReviewers(input_api, email_regexp, approval_needed=False): - """Return the owner and reviewers of a change, if any. - - If approval_needed is True, only reviewers who have approved the change - will be returned. - """ - issue_props = _GetRietveldIssueProps(input_api, True) - if not issue_props: - return None, (set() if approval_needed else - _ReviewersFromChange(input_api.change)) - - if not approval_needed: - return issue_props['owner_email'], set(issue_props['reviewers']) - - owner_email = issue_props['owner_email'] - - messages = issue_props.get('messages', []) - approvers = set( - m['sender'] for m in messages - if m.get('approval') and _match_reviewer_email(m['sender'], owner_email, - email_regexp)) - return owner_email, approvers - - -def _GerritOwnerAndReviewers(input_api, email_regexp, approval_needed=False): - """Return the owner and reviewers of a change, if any. - - If approval_needed is True, only reviewers who have approved the change - will be returned. - """ - issue = input_api.change.issue - if not issue: - return None, (set() if approval_needed else - _ReviewersFromChange(input_api.change)) - - owner_email = input_api.gerrit.GetChangeOwner(issue) - reviewers = set( - r for r in input_api.gerrit.GetChangeReviewers(issue, approval_needed) - if _match_reviewer_email(r, owner_email, email_regexp)) - input_api.logging.debug('owner: %s; approvals given by: %s', - owner_email, ', '.join(sorted(reviewers))) - return owner_email, reviewers - def CheckSingletonInHeaders(input_api, output_api, source_file_filter=None): """Deprecated, must be removed.""" diff --git a/presubmit_support.py b/presubmit_support.py index 9f8115d3f..89f7d5ad3 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -43,7 +43,6 @@ import urlparse from warnings import warn # Local imports. -import auth import fix_encoding import gclient_utils import git_footers @@ -51,7 +50,6 @@ import gerrit_util import owners import owners_finder import presubmit_canned_checks -import rietveld import scm import subprocess2 as subprocess # Exposed through the API. @@ -534,14 +532,13 @@ class InputApi(object): ) def __init__(self, change, presubmit_path, is_committing, - rietveld_obj, verbose, gerrit_obj=None, dry_run=None, thread_pool=None): + verbose, gerrit_obj, dry_run=None, thread_pool=None): """Builds an InputApi object. Args: change: A presubmit.Change object. presubmit_path: The path to the presubmit script being processed. is_committing: True if the change is about to be committed. - rietveld_obj: rietveld.Rietveld client object gerrit_obj: provides basic Gerrit codereview functionality. dry_run: if true, some Checks will be skipped. """ @@ -549,13 +546,8 @@ class InputApi(object): self.version = [int(x) for x in __version__.split('.')] self.change = change self.is_committing = is_committing - self.rietveld = rietveld_obj self.gerrit = gerrit_obj self.dry_run = dry_run - # TBD - self.host_url = 'http://codereview.chromium.org' - if self.rietveld: - self.host_url = self.rietveld.url self.thread_pool = thread_pool or ThreadPool() @@ -1406,19 +1398,17 @@ def DoPostUploadExecuter(change, class PresubmitExecuter(object): - def __init__(self, change, committing, rietveld_obj, verbose, - gerrit_obj=None, dry_run=None, thread_pool=None): + def __init__(self, change, committing, verbose, + gerrit_obj, dry_run=None, thread_pool=None): """ Args: change: The Change object. committing: True if 'git cl land' is running, False if 'git cl upload' is. - rietveld_obj: rietveld.Rietveld client object. gerrit_obj: provides basic Gerrit codereview functionality. dry_run: if true, some Checks will be skipped. """ self.change = change self.committing = committing - self.rietveld = rietveld_obj self.gerrit = gerrit_obj self.verbose = verbose self.dry_run = dry_run @@ -1443,9 +1433,8 @@ class PresubmitExecuter(object): # Load the presubmit script into context. input_api = InputApi(self.change, presubmit_path, self.committing, - self.rietveld, self.verbose, - gerrit_obj=self.gerrit, dry_run=self.dry_run, - thread_pool=self.thread_pool) + self.verbose, gerrit_obj=self.gerrit, + dry_run=self.dry_run, thread_pool=self.thread_pool) output_api = OutputApi(self.committing) context = {} try: @@ -1491,8 +1480,7 @@ def DoPresubmitChecks(change, input_stream, default_presubmit, may_prompt, - rietveld_obj, - gerrit_obj=None, + gerrit_obj, dry_run=None): """Runs all presubmit checks that apply to the files in the change. @@ -1512,7 +1500,6 @@ def DoPresubmitChecks(change, default_presubmit: A default presubmit script to execute in any case. may_prompt: Enable (y/n) questions on warning or error. If False, any questions are answered with yes by default. - rietveld_obj: rietveld.Rietveld object. gerrit_obj: provides basic Gerrit codereview functionality. dry_run: if true, some Checks will be skipped. @@ -1542,7 +1529,7 @@ def DoPresubmitChecks(change, output.write("Warning, no PRESUBMIT.py found.\n") results = [] thread_pool = ThreadPool() - executer = PresubmitExecuter(change, committing, rietveld_obj, verbose, + executer = PresubmitExecuter(change, committing, verbose, gerrit_obj, dry_run, thread_pool) if default_presubmit: if verbose: @@ -1701,17 +1688,8 @@ def main(argv=None): parser.add_option("--gerrit_url", help=optparse.SUPPRESS_HELP) parser.add_option("--gerrit_fetch", action='store_true', help=optparse.SUPPRESS_HELP) - parser.add_option("--rietveld_url", help=optparse.SUPPRESS_HELP) - parser.add_option("--rietveld_email", help=optparse.SUPPRESS_HELP) - parser.add_option("--rietveld_fetch", action='store_true', default=False, - help=optparse.SUPPRESS_HELP) - # These are for OAuth2 authentication for bots. See also apply_issue.py - parser.add_option("--rietveld_email_file", help=optparse.SUPPRESS_HELP) - parser.add_option("--rietveld_private_key_file", help=optparse.SUPPRESS_HELP) - auth.add_auth_options(parser) options, args = parser.parse_args(argv) - auth_config = auth.extract_auth_config_from_options(options) if options.verbose >= 2: logging.basicConfig(level=logging.DEBUG) @@ -1720,49 +1698,14 @@ def main(argv=None): else: logging.basicConfig(level=logging.ERROR) - if (any((options.rietveld_url, options.rietveld_email_file, - options.rietveld_fetch, options.rietveld_private_key_file)) - and any((options.gerrit_url, options.gerrit_fetch))): - parser.error('Options for only codereview --rietveld_* or --gerrit_* ' - 'allowed') - - if options.rietveld_email and options.rietveld_email_file: - parser.error("Only one of --rietveld_email or --rietveld_email_file " - "can be passed to this program.") - if options.rietveld_email_file: - with open(options.rietveld_email_file, "rb") as f: - options.rietveld_email = f.read().strip() - change_class, files = load_files(options, args) if not change_class: parser.error('For unversioned directory, is not optional.') logging.info('Found %d file(s).', len(files)) - rietveld_obj, gerrit_obj = None, None - - if options.rietveld_url: - # The empty password is permitted: '' is not None. - if options.rietveld_private_key_file: - rietveld_obj = rietveld.JwtOAuth2Rietveld( - options.rietveld_url, - options.rietveld_email, - options.rietveld_private_key_file) - else: - rietveld_obj = rietveld.CachingRietveld( - options.rietveld_url, - auth_config, - options.rietveld_email) - if options.rietveld_fetch: - assert options.issue - props = rietveld_obj.get_issue_properties(options.issue, False) - options.author = props['owner_email'] - options.description = props['description'] - logging.info('Got author: "%s"', options.author) - logging.info('Got description: """\n%s\n"""', options.description) - + gerrit_obj = None if options.gerrit_url and options.gerrit_fetch: assert options.issue and options.patchset - rietveld_obj = None gerrit_obj = GerritAccessor(urlparse.urlparse(options.gerrit_url).netloc) options.author = gerrit_obj.GetChangeOwner(options.issue) options.description = gerrit_obj.GetChangeDescription(options.issue, @@ -1787,7 +1730,6 @@ def main(argv=None): sys.stdin, options.default_presubmit, options.may_prompt, - rietveld_obj, gerrit_obj, options.dry_run) return not results.should_continue() diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 53ff26ea5..3f3e5d4d3 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -27,7 +27,6 @@ import owners import owners_finder import subprocess2 as subprocess import presubmit_support as presubmit -import rietveld import auth import git_cl import git_common as git @@ -178,11 +177,11 @@ class PresubmitUnittest(PresubmitTestsBase): 'OutputApi', 'ParseFiles', 'PresubmitFailure', 'PresubmitExecuter', 'PresubmitOutput', 'ScanSubDirs', 'SigintHandler', 'ThreadPool', - 'ast', 'auth', 'cPickle', 'cpplint', 'cStringIO', 'contextlib', + 'ast', 'cPickle', 'cpplint', 'cStringIO', 'contextlib', 'canned_check_filter', 'fix_encoding', 'fnmatch', 'gclient_utils', 'git_footers', 'glob', 'inspect', 'json', 'load_files', 'logging', 'marshal', 'normpath', 'optparse', 'os', 'owners', 'owners_finder', - 'pickle', 'presubmit_canned_checks', 'random', 're', 'rietveld', 'scm', + 'pickle', 'presubmit_canned_checks', 'random', 're', 'scm', 'sigint_handler', 'signal', 'subprocess', 'sys', 'tempfile', 'threading', 'time', 'traceback', 'types', 'unittest', @@ -615,7 +614,7 @@ class PresubmitUnittest(PresubmitTestsBase): output = presubmit.DoPresubmitChecks( change=change, committing=False, verbose=True, output_stream=None, input_stream=None, - default_presubmit=None, may_prompt=False, rietveld_obj=None) + default_presubmit=None, may_prompt=False, gerrit_obj=None) self.failUnless(output.should_continue()) self.assertEqual(output.getvalue().count('!!'), 0) self.assertEqual(output.getvalue().count('??'), 0) @@ -650,7 +649,7 @@ class PresubmitUnittest(PresubmitTestsBase): output = presubmit.DoPresubmitChecks( change=change, committing=False, verbose=True, output_stream=None, input_stream=input_buf, - default_presubmit=None, may_prompt=True, rietveld_obj=None) + default_presubmit=None, may_prompt=True, gerrit_obj=None) self.failIf(output.should_continue()) self.assertEqual(output.getvalue().count('??'), 2) @@ -658,7 +657,7 @@ class PresubmitUnittest(PresubmitTestsBase): output = presubmit.DoPresubmitChecks( change=change, committing=False, verbose=True, output_stream=None, input_stream=input_buf, - default_presubmit=None, may_prompt=True, rietveld_obj=None) + default_presubmit=None, may_prompt=True, gerrit_obj=None) self.failUnless(output.should_continue()) self.assertEquals(output.getvalue().count('??'), 2) self.assertEqual(output.getvalue().count( @@ -689,7 +688,7 @@ class PresubmitUnittest(PresubmitTestsBase): output = presubmit.DoPresubmitChecks( change=change, committing=False, verbose=True, output_stream=None, input_stream=None, - default_presubmit=None, may_prompt=False, rietveld_obj=None) + default_presubmit=None, may_prompt=False, gerrit_obj=None) # A warning is printed, and should_continue is True. self.failUnless(output.should_continue()) self.assertEquals(output.getvalue().count('??'), 2) @@ -720,7 +719,7 @@ class PresubmitUnittest(PresubmitTestsBase): output = presubmit.DoPresubmitChecks( change=change, committing=False, verbose=True, output_stream=None, input_stream=None, - default_presubmit=None, may_prompt=True, rietveld_obj=None) + default_presubmit=None, may_prompt=True, gerrit_obj=None) self.failIf(output.should_continue()) self.assertEqual(output.getvalue().count('??'), 0) self.assertEqual(output.getvalue().count('!!'), 2) @@ -754,7 +753,7 @@ def CheckChangeOnCommit(input_api, output_api): change=change, committing=False, verbose=True, output_stream=None, input_stream=input_buf, default_presubmit=always_fail_presubmit_script, - may_prompt=False, rietveld_obj=None) + may_prompt=False, gerrit_obj=None) self.failIf(output.should_continue()) text = ( 'Running presubmit upload checks ...\n' @@ -814,7 +813,7 @@ def CheckChangeOnCommit(input_api, output_api): change=change, committing=False, verbose=True, output_stream=output_buf, input_stream=input_buf, default_presubmit=tag_checker_presubmit_script, - may_prompt=False, rietveld_obj=None) + may_prompt=False, gerrit_obj=None) self.failUnless(presubmit_output) self.assertEquals(output_buf.getvalue(), @@ -956,7 +955,7 @@ def CheckChangeOnCommit(input_api, output_api): presubmit.DoPresubmitChecks(mox.IgnoreArg(), False, False, mox.IgnoreArg(), mox.IgnoreArg(), - None, False, None, None, None).AndReturn(output) + None, False, None, None).AndReturn(output) self.mox.ReplayAll() self.assertEquals( @@ -1016,7 +1015,6 @@ class InputApiUnittest(PresubmitTestsBase): 'environ', 'fnmatch', 'glob', - 'host_url', 'is_committing', 'is_windows', 'json', @@ -1032,7 +1030,6 @@ class InputApiUnittest(PresubmitTestsBase): 'platform', 'python_executable', 're', - 'rietveld', 'subprocess', 'tbr', 'tempfile', @@ -1056,10 +1053,9 @@ class InputApiUnittest(PresubmitTestsBase): api = presubmit.InputApi( self.fake_change, presubmit_path='foo/path/PRESUBMIT.py', - is_committing=False, rietveld_obj=None, verbose=False) + is_committing=False, gerrit_obj=None, verbose=False) self.assertEquals(api.PresubmitLocalPath(), 'foo/path') self.assertEquals(api.change, self.fake_change) - self.assertEquals(api.host_url, 'http://codereview.chromium.org') def testInputApiPresubmitScriptFiltering(self): description_lines = ('Hello there', @@ -1291,7 +1287,7 @@ class InputApiUnittest(PresubmitTestsBase): self.fake_root_dir, 'isdir', 'PRESUBMIT.py') api = presubmit.InputApi( change=change, presubmit_path=presubmit_path, - is_committing=True, rietveld_obj=None, verbose=False) + is_committing=True, gerrit_obj=None, verbose=False) paths_from_api = api.AbsoluteLocalPaths() self.assertEqual(len(paths_from_api), 2) for absolute_paths in [paths_from_change, paths_from_api]: @@ -1368,7 +1364,7 @@ class InputApiUnittest(PresubmitTestsBase): input_api = presubmit.InputApi( self.fake_change, presubmit_path='foo/path/PRESUBMIT.py', - is_committing=False, rietveld_obj=None, verbose=False) + is_committing=False, gerrit_obj=None, verbose=False) input_api.tempfile.NamedTemporaryFile = self.mox.CreateMock( input_api.tempfile.NamedTemporaryFile) input_api.tempfile.NamedTemporaryFile( @@ -1743,8 +1739,7 @@ class CannedChecksUnittest(PresubmitTestsBase): input_api.os_walk = self.mox.CreateMockAnything() input_api.os_path = presubmit.os.path input_api.re = presubmit.re - input_api.rietveld = self.mox.CreateMock(rietveld.Rietveld) - input_api.gerrit = None + input_api.gerrit = self.mox.CreateMock(presubmit.GerritAccessor) input_api.traceback = presubmit.traceback input_api.urllib2 = self.mox.CreateMock(presubmit.urllib2) input_api.unittest = unittest @@ -1758,7 +1753,6 @@ class CannedChecksUnittest(PresubmitTestsBase): input_api.is_windows = False input_api.change = change - input_api.host_url = 'http://localhost' input_api.is_committing = committing input_api.tbr = False input_api.dry_run = None @@ -1795,7 +1789,6 @@ class CannedChecksUnittest(PresubmitTestsBase): 'CheckOwners', 'CheckPatchFormatted', 'CheckGNFormatted', - 'CheckRietveldTryJobExecution', 'CheckSingletonInHeaders', 'CheckVPythonSpec', 'RunPythonUnitTests', 'RunPylint', @@ -2417,8 +2410,7 @@ class CannedChecksUnittest(PresubmitTestsBase): def AssertOwnersWorks(self, tbr=False, issue='1', approvers=None, reviewers=None, is_committing=True, - rietveld_response=None, gerrit_response=None, - uncovered_files=None, expected_output='', + response=None, uncovered_files=None, expected_output='', manually_specified_reviewers=None, dry_run=None, modified_file='foo/xyz.cc'): if approvers is None: @@ -2442,10 +2434,7 @@ class CannedChecksUnittest(PresubmitTestsBase): change.RepositoryRoot = lambda: None affected_file = self.mox.CreateMock(presubmit.GitAffectedFile) input_api = self.MockInputApi(change, False) - if gerrit_response: - assert not rietveld_response - input_api.rietveld = None - input_api.gerrit = presubmit.GerritAccessor('host') + input_api.gerrit = presubmit.GerritAccessor('host') fake_db = self.mox.CreateMock(owners.Database) fake_db.email_regexp = input_api.re.compile(owners.BASIC_EMAIL_REGEXP) @@ -2465,14 +2454,22 @@ class CannedChecksUnittest(PresubmitTestsBase): change.AffectedFiles(file_filter=None).AndReturn([affected_file]) if not is_committing or (not tbr and issue) or ('OWNERS' in modified_file): change.OriginalOwnersFiles().AndReturn({}) - if issue and not rietveld_response and not gerrit_response: - rietveld_response = { - "owner_email": change.author_email, - "messages": [ - {"sender": a, "text": "I approve", "approval": True} - for a in approvers - ], - "reviewers": reviewers + if issue and not response: + response = { + "owner": {"email": change.author_email}, + "labels": {"Code-Review": { + u'all': [ + { + u'email': a, + u'value': +1 + } for a in approvers + ], + u'default_value': 0, + u'values': {u' 0': u'No score', + u'+1': u'Looks good to me', + u'-1': u"I would prefer that you didn't submit this"} + }}, + "reviewers": {"REVIEWER": [{u'email': a}] for a in approvers}, } if is_committing: @@ -2481,12 +2478,7 @@ class CannedChecksUnittest(PresubmitTestsBase): people = reviewers if issue: - if rietveld_response: - input_api.rietveld.get_issue_properties( - issue=int(input_api.change.issue), messages=True).AndReturn( - rietveld_response) - elif gerrit_response: - input_api.gerrit._FetchChangeDetail = lambda _: gerrit_response + input_api.gerrit._FetchChangeDetail = lambda _: response people.add(change.author_email) change.OriginalOwnersFiles().AndReturn({}) @@ -2504,12 +2496,25 @@ class CannedChecksUnittest(PresubmitTestsBase): def testCannedCheckOwners_DryRun(self): response = { - "owner_email": "john@example.com", - "reviewers": ["ben@example.com"], + "owner": {"email": "john@example.com"}, + "labels": {"Code-Review": { + u'all': [ + { + u'email': u'ben@example.com', + u'value': 0 + }, + ], + u'approved': {u'email': u'ben@example.org'}, + u'default_value': 0, + u'values': {u' 0': u'No score', + u'+1': u'Looks good to me', + u'-1': u"I would prefer that you didn't submit this"} + }}, + "reviewers": {"REVIEWER": [{u'email': u'ben@example.com'}]}, } self.AssertOwnersWorks(approvers=set(), dry_run=True, - rietveld_response=response, + response=response, reviewers=set(["ben@example.com"]), expected_output='This is a dry run, but these failures would be ' + 'reported on commit:\nMissing LGTM from someone ' + @@ -2517,10 +2522,10 @@ class CannedChecksUnittest(PresubmitTestsBase): self.AssertOwnersWorks(approvers=set(['ben@example.com']), is_committing=False, - rietveld_response=response, + response=response, expected_output='') - def testCannedCheckOwners_Approved_Gerrit(self): + def testCannedCheckOwners_Approved(self): response = { "owner": {"email": "john@example.com"}, "labels": {"Code-Review": { @@ -2545,13 +2550,13 @@ class CannedChecksUnittest(PresubmitTestsBase): "reviewers": {"REVIEWER": [{u'email': u'ben@example.com'}]}, } self.AssertOwnersWorks(approvers=set(['ben@example.com']), - gerrit_response=response, + response=response, is_committing=True, expected_output='') self.AssertOwnersWorks(approvers=set(['ben@example.com']), is_committing=False, - gerrit_response=response, + response=response, expected_output='') # Testing configuration with on -1..+1. @@ -2573,31 +2578,11 @@ class CannedChecksUnittest(PresubmitTestsBase): "reviewers": {"REVIEWER": [{u'email': u'ben@example.com'}]}, } self.AssertOwnersWorks(approvers=set(['ben@example.com']), - gerrit_response=response, + response=response, is_committing=True, expected_output='') - - def testCannedCheckOwners_Approved(self): - response = { - "owner_email": "john@example.com", - "messages": [ - { - "sender": "ben@example.com", "text": "foo", "approval": True, - }, - ], - "reviewers": ["ben@example.com"], - } - self.AssertOwnersWorks(approvers=set(['ben@example.com']), - rietveld_response=response, - expected_output='') - - self.AssertOwnersWorks(approvers=set(['ben@example.com']), - is_committing=False, - rietveld_response=response, - expected_output='') - - def testCannedCheckOwners_NotApproved_Gerrit(self): + def testCannedCheckOwners_NotApproved(self): response = { "owner": {"email": "john@example.com"}, "labels": {"Code-Review": { @@ -2624,7 +2609,7 @@ class CannedChecksUnittest(PresubmitTestsBase): self.AssertOwnersWorks( approvers=set(), reviewers=set(["ben@example.com"]), - gerrit_response=response, + response=response, is_committing=True, expected_output= 'Missing LGTM from someone other than john@example.com\n') @@ -2633,7 +2618,7 @@ class CannedChecksUnittest(PresubmitTestsBase): approvers=set(), reviewers=set(["ben@example.com"]), is_committing=False, - gerrit_response=response, + response=response, expected_output='') # Testing configuration with on -1..+1. @@ -2657,49 +2642,26 @@ class CannedChecksUnittest(PresubmitTestsBase): self.AssertOwnersWorks( approvers=set(), reviewers=set(["ben@example.com"]), - gerrit_response=response, + response=response, is_committing=True, expected_output= 'Missing LGTM from someone other than john@example.com\n') - def testCannedCheckOwners_NotApproved(self): - response = { - "owner_email": "john@example.com", - "messages": [ - { - "sender": "ben@example.com", "text": "foo", "approval": False, - }, - ], - "reviewers": ["ben@example.com"], - } - self.AssertOwnersWorks( - approvers=set(), - reviewers=set(["ben@example.com"]), - rietveld_response=response, - expected_output= - 'Missing LGTM from someone other than john@example.com\n') - - self.AssertOwnersWorks( - approvers=set(), - reviewers=set(["ben@example.com"]), - is_committing=False, - rietveld_response=response, - expected_output='') - def testCannedCheckOwners_NoReviewers(self): response = { - "owner_email": "john@example.com", - "messages": [ - { - "sender": "ben@example.com", "text": "foo", "approval": False, - }, - ], - "reviewers": [], + "owner": {"email": "john@example.com"}, + "labels": {"Code-Review": { + u'default_value': 0, + u'values': {u' 0': u'No score', + u'+1': u'Looks good to me', + u'-1': u"I would prefer that you didn't submit this"} + }}, + "reviewers": {}, } self.AssertOwnersWorks( approvers=set(), reviewers=set(), - rietveld_response=response, + response=response, expected_output= 'Missing LGTM from someone other than john@example.com\n') @@ -2707,7 +2669,7 @@ class CannedChecksUnittest(PresubmitTestsBase): approvers=set(), reviewers=set(), is_committing=False, - rietveld_response=response, + response=response, expected_output='') def testCannedCheckOwners_NoIssueNoFiles(self):