Commit f3eed001 authored by Aaron Gable's avatar Aaron Gable Committed by Commit Bot

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.

R=tandrii@chromium.org

Bug: 770408
Change-Id: I749092b66fdca16d5cef77e8cefc905aa5375b50
Reviewed-on: https://chromium-review.googlesource.com/693380
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: 's avatarAndrii Shyshkalov <tandrii@chromium.org>
parent 31e7d56c
...@@ -1523,7 +1523,6 @@ class Changelist(object): ...@@ -1523,7 +1523,6 @@ class Changelist(object):
return presubmit_support.DoPresubmitChecks(change, committing, return presubmit_support.DoPresubmitChecks(change, committing,
verbose=verbose, output_stream=sys.stdout, input_stream=sys.stdin, verbose=verbose, output_stream=sys.stdout, input_stream=sys.stdin,
default_presubmit=None, may_prompt=may_prompt, default_presubmit=None, may_prompt=may_prompt,
rietveld_obj=self._codereview_impl.GetRietveldObjForPresubmit(),
gerrit_obj=self._codereview_impl.GetGerritObjForPresubmit()) gerrit_obj=self._codereview_impl.GetGerritObjForPresubmit())
except presubmit_support.PresubmitFailure as e: except presubmit_support.PresubmitFailure as e:
DieWithError('%s\nMaybe your depot_tools is out of date?' % e) DieWithError('%s\nMaybe your depot_tools is out of date?' % e)
...@@ -1805,11 +1804,6 @@ class _ChangelistCodereviewBase(object): ...@@ -1805,11 +1804,6 @@ class _ChangelistCodereviewBase(object):
"""Which branch-specific properties to erase when unsetting issue.""" """Which branch-specific properties to erase when unsetting issue."""
return [] 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): def GetGerritObjForPresubmit(self):
# None is valid return value, otherwise presubmit_support.GerritAccessor. # None is valid return value, otherwise presubmit_support.GerritAccessor.
return None return None
...@@ -2127,9 +2121,6 @@ class _RietveldChangelistImpl(_ChangelistCodereviewBase): ...@@ -2127,9 +2121,6 @@ class _RietveldChangelistImpl(_ChangelistCodereviewBase):
def CodereviewServerConfigKey(cls): def CodereviewServerConfigKey(cls):
return 'rietveldserver' return 'rietveldserver'
def GetRietveldObjForPresubmit(self):
return self.RpcServer()
def SetLabels(self, enable_auto_submit, use_commit_queue, cq_dry_run): def SetLabels(self, enable_auto_submit, use_commit_queue, cq_dry_run):
raise NotImplementedError() raise NotImplementedError()
...@@ -2502,24 +2493,6 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): ...@@ -2502,24 +2493,6 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
"""Which branch-specific properties to erase when unsetting issue.""" """Which branch-specific properties to erase when unsetting issue."""
return ['gerritsquashhash'] 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): def GetGerritObjForPresubmit(self):
return presubmit_support.GerritAccessor(self._GetGerritHost()) return presubmit_support.GerritAccessor(self._GetGerritHost())
......
...@@ -793,15 +793,6 @@ def RunPylint(input_api, *args, **kwargs): ...@@ -793,15 +793,6 @@ def RunPylint(input_api, *args, **kwargs):
return input_api.RunTests(GetPylint(input_api, *args, **kwargs), False) 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, def CheckBuildbotPendingBuilds(input_api, output_api, url, max_pendings,
ignored): ignored):
try: try:
...@@ -895,25 +886,25 @@ def CheckOwners(input_api, output_api, source_file_filter=None): ...@@ -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 [output_fn('Missing LGTM from someone other than %s' % owner_email)]
return [] return []
def GetCodereviewOwnerAndReviewers(input_api, email_regexp, approval_needed): def GetCodereviewOwnerAndReviewers(input_api, email_regexp, approval_needed):
"""Return the owner and reviewers of a change, if any. """Return the owner and reviewers of a change, if any.
If approval_needed is True, only reviewers who have approved the change If approval_needed is True, only reviewers who have approved the change
will be returned. 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 issue = input_api.change.issue
if issue and input_api.rietveld: if not issue:
return input_api.rietveld.get_issue_properties( return None, (set() if approval_needed else
issue=int(issue), messages=messages) _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): def _ReviewersFromChange(change):
...@@ -929,49 +920,6 @@ def _ReviewersFromChange(change): ...@@ -929,49 +920,6 @@ def _ReviewersFromChange(change):
def _match_reviewer_email(r, owner_email, email_regexp): def _match_reviewer_email(r, owner_email, email_regexp):
return email_regexp.match(r) and r != owner_email 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): def CheckSingletonInHeaders(input_api, output_api, source_file_filter=None):
"""Deprecated, must be removed.""" """Deprecated, must be removed."""
......
...@@ -41,7 +41,6 @@ import urlparse ...@@ -41,7 +41,6 @@ import urlparse
from warnings import warn from warnings import warn
# Local imports. # Local imports.
import auth
import fix_encoding import fix_encoding
import gclient_utils import gclient_utils
import git_footers import git_footers
...@@ -49,7 +48,6 @@ import gerrit_util ...@@ -49,7 +48,6 @@ import gerrit_util
import owners import owners
import owners_finder import owners_finder
import presubmit_canned_checks import presubmit_canned_checks
import rietveld
import scm import scm
import subprocess2 as subprocess # Exposed through the API. import subprocess2 as subprocess # Exposed through the API.
...@@ -390,14 +388,13 @@ class InputApi(object): ...@@ -390,14 +388,13 @@ class InputApi(object):
) )
def __init__(self, change, presubmit_path, is_committing, def __init__(self, change, presubmit_path, is_committing,
rietveld_obj, verbose, gerrit_obj=None, dry_run=None): verbose, gerrit_obj, dry_run=None):
"""Builds an InputApi object. """Builds an InputApi object.
Args: Args:
change: A presubmit.Change object. change: A presubmit.Change object.
presubmit_path: The path to the presubmit script being processed. presubmit_path: The path to the presubmit script being processed.
is_committing: True if the change is about to be committed. is_committing: True if the change is about to be committed.
rietveld_obj: rietveld.Rietveld client object
gerrit_obj: provides basic Gerrit codereview functionality. gerrit_obj: provides basic Gerrit codereview functionality.
dry_run: if true, some Checks will be skipped. dry_run: if true, some Checks will be skipped.
""" """
...@@ -405,13 +402,8 @@ class InputApi(object): ...@@ -405,13 +402,8 @@ class InputApi(object):
self.version = [int(x) for x in __version__.split('.')] self.version = [int(x) for x in __version__.split('.')]
self.change = change self.change = change
self.is_committing = is_committing self.is_committing = is_committing
self.rietveld = rietveld_obj
self.gerrit = gerrit_obj self.gerrit = gerrit_obj
self.dry_run = dry_run self.dry_run = dry_run
# TBD
self.host_url = 'http://codereview.chromium.org'
if self.rietveld:
self.host_url = self.rietveld.url
# We expose various modules and functions as attributes of the input_api # We expose various modules and functions as attributes of the input_api
# so that presubmit scripts don't have to import them. # so that presubmit scripts don't have to import them.
...@@ -1272,19 +1264,17 @@ def DoPostUploadExecuter(change, ...@@ -1272,19 +1264,17 @@ def DoPostUploadExecuter(change,
class PresubmitExecuter(object): class PresubmitExecuter(object):
def __init__(self, change, committing, rietveld_obj, verbose, def __init__(self, change, committing, verbose,
gerrit_obj=None, dry_run=None): gerrit_obj, dry_run=None):
""" """
Args: Args:
change: The Change object. change: The Change object.
committing: True if 'git cl land' is running, False if 'git cl upload' is. 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. gerrit_obj: provides basic Gerrit codereview functionality.
dry_run: if true, some Checks will be skipped. dry_run: if true, some Checks will be skipped.
""" """
self.change = change self.change = change
self.committing = committing self.committing = committing
self.rietveld = rietveld_obj
self.gerrit = gerrit_obj self.gerrit = gerrit_obj
self.verbose = verbose self.verbose = verbose
self.dry_run = dry_run self.dry_run = dry_run
...@@ -1308,7 +1298,7 @@ class PresubmitExecuter(object): ...@@ -1308,7 +1298,7 @@ class PresubmitExecuter(object):
# Load the presubmit script into context. # Load the presubmit script into context.
input_api = InputApi(self.change, presubmit_path, self.committing, input_api = InputApi(self.change, presubmit_path, self.committing,
self.rietveld, self.verbose, self.verbose,
gerrit_obj=self.gerrit, dry_run=self.dry_run) gerrit_obj=self.gerrit, dry_run=self.dry_run)
output_api = OutputApi(self.committing) output_api = OutputApi(self.committing)
context = {} context = {}
...@@ -1357,8 +1347,7 @@ def DoPresubmitChecks(change, ...@@ -1357,8 +1347,7 @@ def DoPresubmitChecks(change,
input_stream, input_stream,
default_presubmit, default_presubmit,
may_prompt, may_prompt,
rietveld_obj, gerrit_obj,
gerrit_obj=None,
dry_run=None): dry_run=None):
"""Runs all presubmit checks that apply to the files in the change. """Runs all presubmit checks that apply to the files in the change.
...@@ -1378,7 +1367,6 @@ def DoPresubmitChecks(change, ...@@ -1378,7 +1367,6 @@ def DoPresubmitChecks(change,
default_presubmit: A default presubmit script to execute in any case. default_presubmit: A default presubmit script to execute in any case.
may_prompt: Enable (y/n) questions on warning or error. If False, may_prompt: Enable (y/n) questions on warning or error. If False,
any questions are answered with yes by default. any questions are answered with yes by default.
rietveld_obj: rietveld.Rietveld object.
gerrit_obj: provides basic Gerrit codereview functionality. gerrit_obj: provides basic Gerrit codereview functionality.
dry_run: if true, some Checks will be skipped. dry_run: if true, some Checks will be skipped.
...@@ -1407,7 +1395,7 @@ def DoPresubmitChecks(change, ...@@ -1407,7 +1395,7 @@ def DoPresubmitChecks(change,
if not presubmit_files and verbose: if not presubmit_files and verbose:
output.write("Warning, no PRESUBMIT.py found.\n") output.write("Warning, no PRESUBMIT.py found.\n")
results = [] results = []
executer = PresubmitExecuter(change, committing, rietveld_obj, verbose, executer = PresubmitExecuter(change, committing, verbose,
gerrit_obj, dry_run) gerrit_obj, dry_run)
if default_presubmit: if default_presubmit:
if verbose: if verbose:
...@@ -1602,17 +1590,8 @@ def main(argv=None): ...@@ -1602,17 +1590,8 @@ def main(argv=None):
parser.add_option("--gerrit_url", help=optparse.SUPPRESS_HELP) parser.add_option("--gerrit_url", help=optparse.SUPPRESS_HELP)
parser.add_option("--gerrit_fetch", action='store_true', parser.add_option("--gerrit_fetch", action='store_true',
help=optparse.SUPPRESS_HELP) 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) options, args = parser.parse_args(argv)
auth_config = auth.extract_auth_config_from_options(options)
if options.verbose >= 2: if options.verbose >= 2:
logging.basicConfig(level=logging.DEBUG) logging.basicConfig(level=logging.DEBUG)
...@@ -1621,49 +1600,14 @@ def main(argv=None): ...@@ -1621,49 +1600,14 @@ def main(argv=None):
else: else:
logging.basicConfig(level=logging.ERROR) 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) change_class, files = load_files(options, args)
if not change_class: if not change_class:
parser.error('For unversioned directory, <files> is not optional.') parser.error('For unversioned directory, <files> is not optional.')
logging.info('Found %d file(s).', len(files)) logging.info('Found %d file(s).', len(files))
rietveld_obj, gerrit_obj = None, None gerrit_obj = 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)
if options.gerrit_url and options.gerrit_fetch: if options.gerrit_url and options.gerrit_fetch:
assert options.issue and options.patchset assert options.issue and options.patchset
rietveld_obj = None
gerrit_obj = GerritAccessor(urlparse.urlparse(options.gerrit_url).netloc) gerrit_obj = GerritAccessor(urlparse.urlparse(options.gerrit_url).netloc)
options.author = gerrit_obj.GetChangeOwner(options.issue) options.author = gerrit_obj.GetChangeOwner(options.issue)
options.description = gerrit_obj.GetChangeDescription(options.issue, options.description = gerrit_obj.GetChangeDescription(options.issue,
...@@ -1688,7 +1632,6 @@ def main(argv=None): ...@@ -1688,7 +1632,6 @@ def main(argv=None):
sys.stdin, sys.stdin,
options.default_presubmit, options.default_presubmit,
options.may_prompt, options.may_prompt,
rietveld_obj,
gerrit_obj, gerrit_obj,
options.dry_run) options.dry_run)
return not results.should_continue() return not results.should_continue()
......
...@@ -27,7 +27,6 @@ import owners ...@@ -27,7 +27,6 @@ import owners
import owners_finder import owners_finder
import subprocess2 as subprocess import subprocess2 as subprocess
import presubmit_support as presubmit import presubmit_support as presubmit
import rietveld
import auth import auth
import git_cl import git_cl
import git_common as git import git_common as git
...@@ -177,11 +176,11 @@ class PresubmitUnittest(PresubmitTestsBase): ...@@ -177,11 +176,11 @@ class PresubmitUnittest(PresubmitTestsBase):
'GitChange', 'InputApi', 'ListRelevantPresubmitFiles', 'main', 'GitChange', 'InputApi', 'ListRelevantPresubmitFiles', 'main',
'NonexistantCannedCheckFilter', 'OutputApi', 'ParseFiles', 'NonexistantCannedCheckFilter', 'OutputApi', 'ParseFiles',
'PresubmitFailure', 'PresubmitExecuter', 'PresubmitOutput', 'ScanSubDirs', 'PresubmitFailure', 'PresubmitExecuter', 'PresubmitOutput', 'ScanSubDirs',
'ast', 'auth', 'cPickle', 'cpplint', 'cStringIO', 'contextlib', 'ast', 'cPickle', 'cpplint', 'cStringIO', 'contextlib',
'canned_check_filter', 'fix_encoding', 'fnmatch', 'gclient_utils', 'canned_check_filter', 'fix_encoding', 'fnmatch', 'gclient_utils',
'git_footers', 'glob', 'inspect', 'json', 'load_files', 'logging', 'git_footers', 'glob', 'inspect', 'json', 'load_files', 'logging',
'marshal', 'normpath', 'optparse', 'os', 'owners', 'owners_finder', 'marshal', 'normpath', 'optparse', 'os', 'owners', 'owners_finder',
'pickle', 'presubmit_canned_checks', 'random', 're', 'rietveld', 'scm', 'pickle', 'presubmit_canned_checks', 'random', 're', 'scm',
'subprocess', 'sys', 'tempfile', 'time', 'traceback', 'types', 'unittest', 'subprocess', 'sys', 'tempfile', 'time', 'traceback', 'types', 'unittest',
'urllib2', 'warn', 'multiprocessing', 'DoGetTryMasters', 'urllib2', 'warn', 'multiprocessing', 'DoGetTryMasters',
'GetTryMastersExecuter', 'itertools', 'urlparse', 'gerrit_util', 'GetTryMastersExecuter', 'itertools', 'urlparse', 'gerrit_util',
...@@ -621,7 +620,7 @@ class PresubmitUnittest(PresubmitTestsBase): ...@@ -621,7 +620,7 @@ class PresubmitUnittest(PresubmitTestsBase):
output = presubmit.DoPresubmitChecks( output = presubmit.DoPresubmitChecks(
change=change, committing=False, verbose=True, change=change, committing=False, verbose=True,
output_stream=None, input_stream=None, 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.failUnless(output.should_continue())
self.assertEqual(output.getvalue().count('!!'), 0) self.assertEqual(output.getvalue().count('!!'), 0)
self.assertEqual(output.getvalue().count('??'), 0) self.assertEqual(output.getvalue().count('??'), 0)
...@@ -656,7 +655,7 @@ class PresubmitUnittest(PresubmitTestsBase): ...@@ -656,7 +655,7 @@ class PresubmitUnittest(PresubmitTestsBase):
output = presubmit.DoPresubmitChecks( output = presubmit.DoPresubmitChecks(
change=change, committing=False, verbose=True, change=change, committing=False, verbose=True,
output_stream=None, input_stream=input_buf, 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.failIf(output.should_continue())
self.assertEqual(output.getvalue().count('??'), 2) self.assertEqual(output.getvalue().count('??'), 2)
...@@ -664,7 +663,7 @@ class PresubmitUnittest(PresubmitTestsBase): ...@@ -664,7 +663,7 @@ class PresubmitUnittest(PresubmitTestsBase):
output = presubmit.DoPresubmitChecks( output = presubmit.DoPresubmitChecks(
change=change, committing=False, verbose=True, change=change, committing=False, verbose=True,
output_stream=None, input_stream=input_buf, 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.failUnless(output.should_continue())
self.assertEquals(output.getvalue().count('??'), 2) self.assertEquals(output.getvalue().count('??'), 2)
self.assertEqual(output.getvalue().count( self.assertEqual(output.getvalue().count(
...@@ -695,7 +694,7 @@ class PresubmitUnittest(PresubmitTestsBase): ...@@ -695,7 +694,7 @@ class PresubmitUnittest(PresubmitTestsBase):
output = presubmit.DoPresubmitChecks( output = presubmit.DoPresubmitChecks(
change=change, committing=False, verbose=True, change=change, committing=False, verbose=True,
output_stream=None, input_stream=None, 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. # A warning is printed, and should_continue is True.
self.failUnless(output.should_continue()) self.failUnless(output.should_continue())
self.assertEquals(output.getvalue().count('??'), 2) self.assertEquals(output.getvalue().count('??'), 2)
...@@ -726,7 +725,7 @@ class PresubmitUnittest(PresubmitTestsBase): ...@@ -726,7 +725,7 @@ class PresubmitUnittest(PresubmitTestsBase):
output = presubmit.DoPresubmitChecks( output = presubmit.DoPresubmitChecks(
change=change, committing=False, verbose=True, change=change, committing=False, verbose=True,
output_stream=None, input_stream=None, 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.failIf(output.should_continue())
self.assertEqual(output.getvalue().count('??'), 0) self.assertEqual(output.getvalue().count('??'), 0)
self.assertEqual(output.getvalue().count('!!'), 2) self.assertEqual(output.getvalue().count('!!'), 2)
...@@ -760,7 +759,7 @@ def CheckChangeOnCommit(input_api, output_api): ...@@ -760,7 +759,7 @@ def CheckChangeOnCommit(input_api, output_api):
change=change, committing=False, verbose=True, change=change, committing=False, verbose=True,
output_stream=None, input_stream=input_buf, output_stream=None, input_stream=input_buf,
default_presubmit=always_fail_presubmit_script, default_presubmit=always_fail_presubmit_script,
may_prompt=False, rietveld_obj=None) may_prompt=False, gerrit_obj=None)
self.failIf(output.should_continue()) self.failIf(output.should_continue())
text = ( text = (
'Running presubmit upload checks ...\n' 'Running presubmit upload checks ...\n'
...@@ -820,7 +819,7 @@ def CheckChangeOnCommit(input_api, output_api): ...@@ -820,7 +819,7 @@ def CheckChangeOnCommit(input_api, output_api):
change=change, committing=False, verbose=True, change=change, committing=False, verbose=True,
output_stream=output_buf, input_stream=input_buf, output_stream=output_buf, input_stream=input_buf,
default_presubmit=tag_checker_presubmit_script, default_presubmit=tag_checker_presubmit_script,
may_prompt=False, rietveld_obj=None) may_prompt=False, gerrit_obj=None)
self.failUnless(presubmit_output) self.failUnless(presubmit_output)
self.assertEquals(output_buf.getvalue(), self.assertEquals(output_buf.getvalue(),
...@@ -962,7 +961,7 @@ def CheckChangeOnCommit(input_api, output_api): ...@@ -962,7 +961,7 @@ def CheckChangeOnCommit(input_api, output_api):
presubmit.DoPresubmitChecks(mox.IgnoreArg(), False, False, presubmit.DoPresubmitChecks(mox.IgnoreArg(), False, False,
mox.IgnoreArg(), mox.IgnoreArg(),
mox.IgnoreArg(), mox.IgnoreArg(),
None, False, None, None, None).AndReturn(output) None, False, None, None).AndReturn(output)
self.mox.ReplayAll() self.mox.ReplayAll()
self.assertEquals( self.assertEquals(
...@@ -1023,7 +1022,6 @@ class InputApiUnittest(PresubmitTestsBase): ...@@ -1023,7 +1022,6 @@ class InputApiUnittest(PresubmitTestsBase):
'environ', 'environ',
'fnmatch', 'fnmatch',
'glob', 'glob',
'host_url',
'is_committing', 'is_committing',
'is_windows', 'is_windows',
'json', 'json',
...@@ -1039,7 +1037,6 @@ class InputApiUnittest(PresubmitTestsBase): ...@@ -1039,7 +1037,6 @@ class InputApiUnittest(PresubmitTestsBase):
'platform', 'platform',
'python_executable', 'python_executable',
're', 're',
'rietveld',
'subprocess', 'subprocess',
'tbr', 'tbr',
'tempfile', 'tempfile',
...@@ -1062,10 +1059,9 @@ class InputApiUnittest(PresubmitTestsBase): ...@@ -1062,10 +1059,9 @@ class InputApiUnittest(PresubmitTestsBase):
api = presubmit.InputApi( api = presubmit.InputApi(
self.fake_change, self.fake_change,
presubmit_path='foo/path/PRESUBMIT.py', 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.PresubmitLocalPath(), 'foo/path')
self.assertEquals(api.change, self.fake_change) self.assertEquals(api.change, self.fake_change)
self.assertEquals(api.host_url, 'http://codereview.chromium.org')
def testInputApiPresubmitScriptFiltering(self): def testInputApiPresubmitScriptFiltering(self):
description_lines = ('Hello there', description_lines = ('Hello there',
...@@ -1297,7 +1293,7 @@ class InputApiUnittest(PresubmitTestsBase): ...@@ -1297,7 +1293,7 @@ class InputApiUnittest(PresubmitTestsBase):
self.fake_root_dir, 'isdir', 'PRESUBMIT.py') self.fake_root_dir, 'isdir', 'PRESUBMIT.py')
api = presubmit.InputApi( api = presubmit.InputApi(
change=change, presubmit_path=presubmit_path, 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() paths_from_api = api.AbsoluteLocalPaths()
self.assertEqual(len(paths_from_api), 2) self.assertEqual(len(paths_from_api), 2)
for absolute_paths in [paths_from_change, paths_from_api]: for absolute_paths in [paths_from_change, paths_from_api]:
...@@ -1374,7 +1370,7 @@ class InputApiUnittest(PresubmitTestsBase): ...@@ -1374,7 +1370,7 @@ class InputApiUnittest(PresubmitTestsBase):
input_api = presubmit.InputApi( input_api = presubmit.InputApi(
self.fake_change, self.fake_change,
presubmit_path='foo/path/PRESUBMIT.py', 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 = self.mox.CreateMock(
input_api.tempfile.NamedTemporaryFile) input_api.tempfile.NamedTemporaryFile)
input_api.tempfile.NamedTemporaryFile( input_api.tempfile.NamedTemporaryFile(
...@@ -1740,8 +1736,7 @@ class CannedChecksUnittest(PresubmitTestsBase): ...@@ -1740,8 +1736,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
input_api.os_walk = self.mox.CreateMockAnything() input_api.os_walk = self.mox.CreateMockAnything()
input_api.os_path = presubmit.os.path input_api.os_path = presubmit.os.path
input_api.re = presubmit.re input_api.re = presubmit.re
input_api.rietveld = self.mox.CreateMock(rietveld.Rietveld) input_api.gerrit = self.mox.CreateMock(presubmit.GerritAccessor)
input_api.gerrit = None
input_api.traceback = presubmit.traceback input_api.traceback = presubmit.traceback
input_api.urllib2 = self.mox.CreateMock(presubmit.urllib2) input_api.urllib2 = self.mox.CreateMock(presubmit.urllib2)
input_api.unittest = unittest input_api.unittest = unittest
...@@ -1755,7 +1750,6 @@ class CannedChecksUnittest(PresubmitTestsBase): ...@@ -1755,7 +1750,6 @@ class CannedChecksUnittest(PresubmitTestsBase):
input_api.is_windows = False input_api.is_windows = False
input_api.change = change input_api.change = change
input_api.host_url = 'http://localhost'
input_api.is_committing = committing input_api.is_committing = committing
input_api.tbr = False input_api.tbr = False
input_api.dry_run = None input_api.dry_run = None
...@@ -1791,7 +1785,6 @@ class CannedChecksUnittest(PresubmitTestsBase): ...@@ -1791,7 +1785,6 @@ class CannedChecksUnittest(PresubmitTestsBase):
'CheckOwners', 'CheckOwners',
'CheckPatchFormatted', 'CheckPatchFormatted',
'CheckGNFormatted', 'CheckGNFormatted',
'CheckRietveldTryJobExecution',
'CheckSingletonInHeaders', 'CheckSingletonInHeaders',
'CheckVPythonSpec', 'CheckVPythonSpec',
'RunPythonUnitTests', 'RunPylint', 'RunPythonUnitTests', 'RunPylint',
...@@ -2406,8 +2399,7 @@ class CannedChecksUnittest(PresubmitTestsBase): ...@@ -2406,8 +2399,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
def AssertOwnersWorks(self, tbr=False, issue='1', approvers=None, def AssertOwnersWorks(self, tbr=False, issue='1', approvers=None,
reviewers=None, is_committing=True, reviewers=None, is_committing=True,
rietveld_response=None, gerrit_response=None, response=None, uncovered_files=None, expected_output='',
uncovered_files=None, expected_output='',
manually_specified_reviewers=None, dry_run=None): manually_specified_reviewers=None, dry_run=None):
if approvers is None: if approvers is None:
# The set of people who lgtm'ed a change. # The set of people who lgtm'ed a change.
...@@ -2430,9 +2422,6 @@ class CannedChecksUnittest(PresubmitTestsBase): ...@@ -2430,9 +2422,6 @@ class CannedChecksUnittest(PresubmitTestsBase):
change.RepositoryRoot = lambda: None change.RepositoryRoot = lambda: None
affected_file = self.mox.CreateMock(presubmit.GitAffectedFile) affected_file = self.mox.CreateMock(presubmit.GitAffectedFile)
input_api = self.MockInputApi(change, False) 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 = self.mox.CreateMock(owners.Database)
...@@ -2453,14 +2442,22 @@ class CannedChecksUnittest(PresubmitTestsBase): ...@@ -2453,14 +2442,22 @@ class CannedChecksUnittest(PresubmitTestsBase):
affected_file.LocalPath().AndReturn('foo/xyz.cc') affected_file.LocalPath().AndReturn('foo/xyz.cc')
change.AffectedFiles(file_filter=None).AndReturn([affected_file]) change.AffectedFiles(file_filter=None).AndReturn([affected_file])
change.OriginalOwnersFiles().AndReturn({}) change.OriginalOwnersFiles().AndReturn({})
if issue and not rietveld_response and not gerrit_response: if issue and not response:
rietveld_response = { response = {
"owner_email": change.author_email, "owner": {"email": change.author_email},
"messages": [ "labels": {"Code-Review": {
{"sender": a, "text": "I approve", "approval": True} u'all': [
for a in approvers {
u'email': a,
u'value': +1
} for a in approvers
], ],
"reviewers": reviewers 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: if is_committing:
...@@ -2469,12 +2466,7 @@ class CannedChecksUnittest(PresubmitTestsBase): ...@@ -2469,12 +2466,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
people = reviewers people = reviewers
if issue: if issue:
if rietveld_response: input_api.gerrit._FetchChangeDetail = lambda _: 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
people.add(change.author_email) people.add(change.author_email)
change.OriginalOwnersFiles().AndReturn({}) change.OriginalOwnersFiles().AndReturn({})
...@@ -2492,12 +2484,25 @@ class CannedChecksUnittest(PresubmitTestsBase): ...@@ -2492,12 +2484,25 @@ class CannedChecksUnittest(PresubmitTestsBase):
def testCannedCheckOwners_DryRun(self): def testCannedCheckOwners_DryRun(self):
response = { response = {
"owner_email": "john@example.com", "owner": {"email": "john@example.com"},
"reviewers": ["ben@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(), self.AssertOwnersWorks(approvers=set(),
dry_run=True, dry_run=True,
rietveld_response=response, response=response,
reviewers=set(["ben@example.com"]), reviewers=set(["ben@example.com"]),
expected_output='This is a dry run, but these failures would be ' + expected_output='This is a dry run, but these failures would be ' +
'reported on commit:\nMissing LGTM from someone ' + 'reported on commit:\nMissing LGTM from someone ' +
...@@ -2505,10 +2510,10 @@ class CannedChecksUnittest(PresubmitTestsBase): ...@@ -2505,10 +2510,10 @@ class CannedChecksUnittest(PresubmitTestsBase):
self.AssertOwnersWorks(approvers=set(['ben@example.com']), self.AssertOwnersWorks(approvers=set(['ben@example.com']),
is_committing=False, is_committing=False,
rietveld_response=response, response=response,
expected_output='') expected_output='')
def testCannedCheckOwners_Approved_Gerrit(self): def testCannedCheckOwners_Approved(self):
response = { response = {
"owner": {"email": "john@example.com"}, "owner": {"email": "john@example.com"},
"labels": {"Code-Review": { "labels": {"Code-Review": {
...@@ -2533,13 +2538,13 @@ class CannedChecksUnittest(PresubmitTestsBase): ...@@ -2533,13 +2538,13 @@ class CannedChecksUnittest(PresubmitTestsBase):
"reviewers": {"REVIEWER": [{u'email': u'ben@example.com'}]}, "reviewers": {"REVIEWER": [{u'email': u'ben@example.com'}]},
} }
self.AssertOwnersWorks(approvers=set(['ben@example.com']), self.AssertOwnersWorks(approvers=set(['ben@example.com']),
gerrit_response=response, response=response,
is_committing=True, is_committing=True,
expected_output='') expected_output='')
self.AssertOwnersWorks(approvers=set(['ben@example.com']), self.AssertOwnersWorks(approvers=set(['ben@example.com']),
is_committing=False, is_committing=False,
gerrit_response=response, response=response,
expected_output='') expected_output='')
# Testing configuration with on -1..+1. # Testing configuration with on -1..+1.
...@@ -2561,31 +2566,11 @@ class CannedChecksUnittest(PresubmitTestsBase): ...@@ -2561,31 +2566,11 @@ class CannedChecksUnittest(PresubmitTestsBase):
"reviewers": {"REVIEWER": [{u'email': u'ben@example.com'}]}, "reviewers": {"REVIEWER": [{u'email': u'ben@example.com'}]},
} }
self.AssertOwnersWorks(approvers=set(['ben@example.com']), self.AssertOwnersWorks(approvers=set(['ben@example.com']),
gerrit_response=response, response=response,
is_committing=True, is_committing=True,
expected_output='') expected_output='')
def testCannedCheckOwners_NotApproved(self):
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):
response = { response = {
"owner": {"email": "john@example.com"}, "owner": {"email": "john@example.com"},
"labels": {"Code-Review": { "labels": {"Code-Review": {
...@@ -2612,7 +2597,7 @@ class CannedChecksUnittest(PresubmitTestsBase): ...@@ -2612,7 +2597,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
self.AssertOwnersWorks( self.AssertOwnersWorks(
approvers=set(), approvers=set(),
reviewers=set(["ben@example.com"]), reviewers=set(["ben@example.com"]),
gerrit_response=response, response=response,
is_committing=True, is_committing=True,
expected_output= expected_output=
'Missing LGTM from someone other than john@example.com\n') 'Missing LGTM from someone other than john@example.com\n')
...@@ -2621,7 +2606,7 @@ class CannedChecksUnittest(PresubmitTestsBase): ...@@ -2621,7 +2606,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
approvers=set(), approvers=set(),
reviewers=set(["ben@example.com"]), reviewers=set(["ben@example.com"]),
is_committing=False, is_committing=False,
gerrit_response=response, response=response,
expected_output='') expected_output='')
# Testing configuration with on -1..+1. # Testing configuration with on -1..+1.
...@@ -2645,49 +2630,26 @@ class CannedChecksUnittest(PresubmitTestsBase): ...@@ -2645,49 +2630,26 @@ class CannedChecksUnittest(PresubmitTestsBase):
self.AssertOwnersWorks( self.AssertOwnersWorks(
approvers=set(), approvers=set(),
reviewers=set(["ben@example.com"]), reviewers=set(["ben@example.com"]),
gerrit_response=response, response=response,
is_committing=True, is_committing=True,
expected_output= expected_output=
'Missing LGTM from someone other than john@example.com\n') '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): def testCannedCheckOwners_NoReviewers(self):
response = { response = {
"owner_email": "john@example.com", "owner": {"email": "john@example.com"},
"messages": [ "labels": {"Code-Review": {
{ u'default_value': 0,
"sender": "ben@example.com", "text": "foo", "approval": False, 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": [], }},
"reviewers": {},
} }
self.AssertOwnersWorks( self.AssertOwnersWorks(
approvers=set(), approvers=set(),
reviewers=set(), reviewers=set(),
rietveld_response=response, response=response,
expected_output= expected_output=
'Missing LGTM from someone other than john@example.com\n') 'Missing LGTM from someone other than john@example.com\n')
...@@ -2695,7 +2657,7 @@ class CannedChecksUnittest(PresubmitTestsBase): ...@@ -2695,7 +2657,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
approvers=set(), approvers=set(),
reviewers=set(), reviewers=set(),
is_committing=False, is_committing=False,
rietveld_response=response, response=response,
expected_output='') expected_output='')
def testCannedCheckOwners_NoIssueNoFiles(self): def testCannedCheckOwners_NoIssueNoFiles(self):
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment