Skip to content

C
Chromium_Depot_tools
Commit e0a10585 authored by tandrii@chromium.org's avatar tandrii@chromium.org
Browse files
Options

Revert of Gerrit git cl: implement git cl patch. (patchset #7 id:120001 of... 

Revert of Gerrit git cl: implement git cl patch. (patchset #7 id:120001 of https://codereview.chromium.org/1852593002/ )

Reason for revert:
just in case.

Original issue's description:
> Gerrit git cl: implement git cl patch.
> 
> BUG=579182
> 
> Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299644

TBR=andybons@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=579182

Review URL: https://codereview.chromium.org/1848393002

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@299645 0039d316-1c4b-4281-b951-d872f2087c98
parent cc0856a3
Show whitespace changes
Inline Side-by-side
Showing with 125 additions and 452 deletions
git_cl.py
View file @ e0a10585
...@@ -834,43 +834,6 @@ def GetCurrentBranch(): ...@@ -834,43 +834,6 @@ def GetCurrentBranch():
return None return None
class _ParsedIssueNumberArgument(object):
def __init__(self, issue=None, patchset=None, hostname=None):
self.issue = issue
self.patchset = patchset
self.hostname = hostname
@property
def valid(self):
return self.issue is not None
class _RietveldParsedIssueNumberArgument(_ParsedIssueNumberArgument):
def __init__(self, *args, **kwargs):
self.patch_url = kwargs.pop('patch_url', None)
super(_RietveldParsedIssueNumberArgument, self).__init__(*args, **kwargs)
def ParseIssueNumberArgument(arg):
"""Parses the issue argument and returns _ParsedIssueNumberArgument."""
fail_result = _ParsedIssueNumberArgument()
if arg.isdigit():
return _ParsedIssueNumberArgument(issue=int(arg))
if not arg.startswith('http'):
return fail_result
url = gclient_utils.UpgradeToHttps(arg)
try:
parsed_url = urlparse.urlparse(url)
except ValueError:
return fail_result
for cls in _CODEREVIEW_IMPLEMENTATIONS.itervalues():
tmp = cls.ParseIssueURL(parsed_url)
if tmp is not None:
return tmp
return fail_result
class Changelist(object): class Changelist(object):
"""Changelist works with one changelist in local branch. """Changelist works with one changelist in local branch.
...@@ -1294,20 +1257,6 @@ class Changelist(object): ...@@ -1294,20 +1257,6 @@ class Changelist(object):
('%s\nMaybe your depot_tools is out of date?\n' ('%s\nMaybe your depot_tools is out of date?\n'
'If all fails, contact maruel@') % e) 'If all fails, contact maruel@') % e)
def CMDPatchIssue(self, issue_arg, reject, nocommit, directory):
"""Fetches and applies the issue patch from codereview to local branch."""
if issue_arg.isdigit():
parsed_issue_arg = _RietveldParsedIssueNumberArgument(int(issue_arg))
else:
# Assume url.
parsed_issue_arg = self._codereview_impl.ParseIssueURL(
urlparse.urlparse(issue_arg))
if not parsed_issue_arg or not parsed_issue_arg.valid:
DieWithError('Failed to parse issue argument "%s". '
'Must be an issue number or a valid URL.' % issue_arg)
return self._codereview_impl.CMDPatchWithParsedIssue(
parsed_issue_arg, reject, nocommit, directory)
# Forward methods to codereview specific implementation. # Forward methods to codereview specific implementation.
def CloseIssue(self): def CloseIssue(self):
...@@ -1397,26 +1346,6 @@ class _ChangelistCodereviewBase(object): ...@@ -1397,26 +1346,6 @@ class _ChangelistCodereviewBase(object):
"""Returns the most recent patchset number from the codereview site.""" """Returns the most recent patchset number from the codereview site."""
raise NotImplementedError() raise NotImplementedError()
def CMDPatchWithParsedIssue(self, parsed_issue_arg, reject, nocommit,
directory):
"""Fetches and applies the issue.
Arguments:
parsed_issue_arg: instance of _ParsedIssueNumberArgument.
reject: if True, reject the failed patch instead of switching to 3-way
merge. Rietveld only.
nocommit: do not commit the patch, thus leave the tree dirty. Rietveld
only.
directory: switch to directory before applying the patch. Rietveld only.
"""
raise NotImplementedError()
@staticmethod
def ParseIssueURL(parsed_url):
"""Parses url and returns instance of _ParsedIssueNumberArgument or None if
failed."""
raise NotImplementedError()
class _RietveldChangelistImpl(_ChangelistCodereviewBase): class _RietveldChangelistImpl(_ChangelistCodereviewBase):
def __init__(self, changelist, auth_config=None, rietveld_server=None): def __init__(self, changelist, auth_config=None, rietveld_server=None):
...@@ -1589,94 +1518,6 @@ class _RietveldChangelistImpl(_ChangelistCodereviewBase): ...@@ -1589,94 +1518,6 @@ class _RietveldChangelistImpl(_ChangelistCodereviewBase):
def GetRieveldObjForPresubmit(self): def GetRieveldObjForPresubmit(self):
return self.RpcServer() return self.RpcServer()
def CMDPatchWithParsedIssue(self, parsed_issue_arg, reject, nocommit,
directory):
# TODO(maruel): Use apply_issue.py
# PatchIssue should never be called with a dirty tree. It is up to the
# caller to check this, but just in case we assert here since the
# consequences of the caller not checking this could be dire.
assert(not git_common.is_dirty_git_tree('apply'))
assert(parsed_issue_arg.valid)
self._changelist.issue = parsed_issue_arg.issue
if parsed_issue_arg.hostname:
self._rietveld_server = 'https://%s' % parsed_issue_arg.hostname
if parsed_issue_arg.patch_url:
assert parsed_issue_arg.patchset
patchset = parsed_issue_arg.patchset
patch_data = urllib2.urlopen(parsed_issue_arg.patch_url).read()
else:
patchset = parsed_issue_arg.patchset or self.GetMostRecentPatchset()
patch_data = self.GetPatchSetDiff(self.GetIssue(), patchset)
# Switch up to the top-level directory, if necessary, in preparation for
# applying the patch.
top = settings.GetRelativeRoot()
if top:
os.chdir(top)
# Git patches have a/ at the beginning of source paths. We strip that out
# with a sed script rather than the -p flag to patch so we can feed either
# Git or svn-style patches into the same apply command.
# re.sub() should be used but flags=re.MULTILINE is only in python 2.7.
try:
patch_data = subprocess2.check_output(
['sed', '-e', 's|^--- a/|--- |; s|^+++ b/|+++ |'], stdin=patch_data)
except subprocess2.CalledProcessError:
DieWithError('Git patch mungling failed.')
logging.info(patch_data)
# We use "git apply" to apply the patch instead of "patch" so that we can
# pick up file adds.
# The --index flag means: also insert into the index (so we catch adds).
cmd = ['git', 'apply', '--index', '-p0']
if directory:
cmd.extend(('--directory', directory))
if reject:
cmd.append('--reject')
elif IsGitVersionAtLeast('1.7.12'):
cmd.append('--3way')
try:
subprocess2.check_call(cmd, env=GetNoGitPagerEnv(),
stdin=patch_data, stdout=subprocess2.VOID)
except subprocess2.CalledProcessError:
print 'Failed to apply the patch'
return 1
# If we had an issue, commit the current state and register the issue.
if not nocommit:
RunGit(['commit', '-m', (self.GetDescription() + '\n\n' +
'patch from issue %(i)s at patchset '
'%(p)s (http://crrev.com/%(i)s#ps%(p)s)'
% {'i': self.GetIssue(), 'p': patchset})])
self.SetIssue(self.GetIssue())
self.SetPatchset(patchset)
print "Committed patch locally."
else:
print "Patch applied to index."
return 0
@staticmethod
def ParseIssueURL(parsed_url):
if not parsed_url.scheme or not parsed_url.scheme.startswith('http'):
return None
# Typical url: https://domain/<issue_number>[/[other]]
match = re.match('/(\d+)(/.*)?$', parsed_url.path)
if match:
return _RietveldParsedIssueNumberArgument(
issue=int(match.group(1)),
hostname=parsed_url.netloc)
# Rietveld patch: https://domain/download/issue<number>_<patchset>.diff
match = re.match(r'/download/issue(\d+)_(\d+).diff$', parsed_url.path)
if match:
return _RietveldParsedIssueNumberArgument(
issue=int(match.group(1)),
patchset=int(match.group(2)),
hostname=parsed_url.netloc,
patch_url=gclient_utils.UpgradeToHttps(parsed_url.geturl()))
return None
class _GerritChangelistImpl(_ChangelistCodereviewBase): class _GerritChangelistImpl(_ChangelistCodereviewBase):
def __init__(self, changelist, auth_config=None): def __init__(self, changelist, auth_config=None):
...@@ -1855,63 +1696,6 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): ...@@ -1855,63 +1696,6 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
print('Issue %s has been submitted.' % self.GetIssueURL()) print('Issue %s has been submitted.' % self.GetIssueURL())
return 0 return 0
def CMDPatchWithParsedIssue(self, parsed_issue_arg, reject, nocommit,
directory):
assert not reject
assert not nocommit
assert not directory
assert parsed_issue_arg.valid
self._changelist.issue = parsed_issue_arg.issue
if parsed_issue_arg.hostname:
self._gerrit_host = parsed_issue_arg.hostname
self._gerrit_server = 'https://%s' % self._gerrit_host
detail = self._GetChangeDetail(['ALL_REVISIONS'])
if not parsed_issue_arg.patchset:
# Use current revision by default.
revision_info = detail['revisions'][detail['current_revision']]
patchset = int(revision_info['_number'])
else:
patchset = parsed_issue_arg.patchset
for revision_info in detail['revisions'].itervalues():
if int(revision_info['_number']) == parsed_issue_arg.patchset:
break
else:
DieWithError('Couldn\'t find patchset %i in issue %i' %
(parsed_issue_arg.patchset, self.GetIssue()))
fetch_info = revision_info['fetch']['http']
RunGit(['fetch', fetch_info['url'], fetch_info['ref']])
RunGit(['cherry-pick', 'FETCH_HEAD'])
self.SetIssue(self.GetIssue())
self.SetPatchset(patchset)
print('Committed patch for issue %i pathset %i locally' %
(self.GetIssue(), self.GetPatchset()))
return 0
@staticmethod
def ParseIssueURL(parsed_url):
if not parsed_url.scheme or not parsed_url.scheme.startswith('http'):
return None
# Gerrit's new UI is https://domain/c/<issue_number>[/[patchset]]
# But current GWT UI is https://domain/#/c/<issue_number>[/[patchset]]
# Short urls like https://domain/<issue_number> can be used, but don't allow
# specifying the patchset (you'd 404), but we allow that here.
if parsed_url.path == '/':
part = parsed_url.fragment
else:
part = parsed_url.path
match = re.match('(/c)?/(\d+)(/(\d+)?/?)?$', part)
if match:
return _ParsedIssueNumberArgument(
issue=int(match.group(2)),
patchset=int(match.group(4)) if match.group(4) else None,
hostname=parsed_url.netloc)
return None
_CODEREVIEW_IMPLEMENTATIONS = { _CODEREVIEW_IMPLEMENTATIONS = {
'rietveld': _RietveldChangelistImpl, 'rietveld': _RietveldChangelistImpl,
...@@ -3809,6 +3593,15 @@ def CMDland(parser, args): ...@@ -3809,6 +3593,15 @@ def CMDland(parser, args):
return SendUpstream(parser, args, 'land') return SendUpstream(parser, args, 'land')
def ParseIssueNum(arg):
"""Parses the issue number from args if present otherwise returns None."""
if re.match(r'\d+', arg):
return arg
if arg.startswith('http'):
return re.sub(r'.*/(\d+)/?', r'\1', arg)
return None
@subcommand.usage('<patch url or issue id or issue url>') @subcommand.usage('<patch url or issue id or issue url>')
def CMDpatch(parser, args): def CMDpatch(parser, args):
"""Patches in a code review.""" """Patches in a code review."""
...@@ -3818,69 +3611,64 @@ def CMDpatch(parser, args): ...@@ -3818,69 +3611,64 @@ def CMDpatch(parser, args):
help='with -b, clobber any existing branch') help='with -b, clobber any existing branch')
parser.add_option('-d', '--directory', action='store', metavar='DIR', parser.add_option('-d', '--directory', action='store', metavar='DIR',
help='Change to the directory DIR immediately, ' help='Change to the directory DIR immediately, '
'before doing anything else. Rietveld only.') 'before doing anything else.')
parser.add_option('--reject', action='store_true', parser.add_option('--reject', action='store_true',
help='failed patches spew .rej files rather than ' help='failed patches spew .rej files rather than '
'attempting a 3-way merge. Rietveld only.') 'attempting a 3-way merge')
parser.add_option('-n', '--no-commit', action='store_true', dest='nocommit', parser.add_option('-n', '--no-commit', action='store_true', dest='nocommit',
help='don\'t commit after patch applies. Rietveld only.') help="don't commit after patch applies")
group = optparse.OptionGroup(parser,
group = optparse.OptionGroup( """Options for continuing work on the current issue uploaded
parser, from a different clone (e.g. different machine). Must be used independently from
'Options for continuing work on the current issue uploaded from a ' the other options. No issue number should be specified, and the branch must have
'different clone (e.g. different machine). Must be used independently ' an issue number associated with it""")
'from the other options. No issue number should be specified, and the ' group.add_option('--reapply', action='store_true',
'branch must have an issue number associated with it') dest='reapply',
group.add_option('--reapply', action='store_true', dest='reapply', help="""Reset the branch and reapply the issue.
help='Reset the branch and reapply the issue.\n' CAUTION: This will undo any local changes in this branch""")
'CAUTION: This will undo any local changes in this '
'branch')
group.add_option('--pull', action='store_true', dest='pull', group.add_option('--pull', action='store_true', dest='pull',
help='Performs a pull before reapplying.') help="Performs a pull before reapplying.")
parser.add_option_group(group) parser.add_option_group(group)
auth.add_auth_options(parser) auth.add_auth_options(parser)
(options, args) = parser.parse_args(args) (options, args) = parser.parse_args(args)
auth_config = auth.extract_auth_config_from_options(options) auth_config = auth.extract_auth_config_from_options(options)
cl = Changelist(auth_config=auth_config)
issue_arg = None issue_arg = None
if options.reapply : if options.reapply :
if len(args) > 0: if len(args) > 0:
parser.error('--reapply implies no additional arguments.') parser.error("--reapply implies no additional arguments.")
cl = Changelist()
issue_arg = cl.GetIssue() issue_arg = cl.GetIssue()
upstream = cl.GetUpstreamBranch() upstream = cl.GetUpstreamBranch()
if upstream == None: if upstream == None:
parser.error('No upstream branch specified. Cannot reset branch') parser.error("No upstream branch specified. Cannot reset branch")
RunGit(['reset', '--hard', upstream]) RunGit(['reset', '--hard', upstream])
if options.pull: if options.pull:
RunGit(['pull']) RunGit(['pull'])
else: else:
if len(args) != 1: if len(args) != 1:
parser.error('Must specify issue number or url') parser.error("Must specify issue number")
issue_arg = args[0]
issue_arg = ParseIssueNum(args[0])
if not issue_arg: # The patch URL works because ParseIssueNum won't do any substitution
# as the re.sub pattern fails to match and just returns it.
if issue_arg == None:
parser.print_help() parser.print_help()
return 1 return 1
if cl.IsGerrit():
if options.reject:
parser.error('--reject is not supported with Gerrit codereview.')
if options.nocommit:
parser.error('--nocommit is not supported with Gerrit codereview.')
if options.directory:
parser.error('--directory is not supported with Gerrit codereview.')
# We don't want uncommitted changes mixed up with the patch. # We don't want uncommitted changes mixed up with the patch.
if git_common.is_dirty_git_tree('patch'): if git_common.is_dirty_git_tree('patch'):
return 1 return 1
# TODO(maruel): Use apply_issue.py
# TODO(ukai): use gerrit-cherry-pick for gerrit repository?
if options.newbranch: if options.newbranch:
if options.reapply: if options.reapply:
parser.error("--reapply excludes any option other than --pull") parser.error("--reapply excludes any option other than --pull")
...@@ -3890,8 +3678,84 @@ def CMDpatch(parser, args): ...@@ -3890,8 +3678,84 @@ def CMDpatch(parser, args):
RunGit(['checkout', '-b', options.newbranch, RunGit(['checkout', '-b', options.newbranch,
Changelist().GetUpstreamBranch()]) Changelist().GetUpstreamBranch()])
return cl.CMDPatchIssue(issue_arg, options.reject, options.nocommit, return PatchIssue(issue_arg, options.reject, options.nocommit,
options.directory) options.directory, auth_config)
def PatchIssue(issue_arg, reject, nocommit, directory, auth_config):
# PatchIssue should never be called with a dirty tree. It is up to the
# caller to check this, but just in case we assert here since the
# consequences of the caller not checking this could be dire.
assert(not git_common.is_dirty_git_tree('apply'))
# TODO(tandrii): implement for Gerrit.
if type(issue_arg) is int or issue_arg.isdigit():
# Input is an issue id. Figure out the URL.
issue = int(issue_arg)
cl = Changelist(issue=issue, codereview='rietveld', auth_config=auth_config)
patchset = cl.GetMostRecentPatchset()
patch_data = cl._codereview_impl.GetPatchSetDiff(issue, patchset)
else:
# Assume it's a URL to the patch. Default to https.
issue_url = gclient_utils.UpgradeToHttps(issue_arg)
match = re.match(r'(.*?)/download/issue(\d+)_(\d+).diff', issue_url)
if not match:
DieWithError('Must pass an issue ID or full URL for '
'\'Download raw patch set\'')
issue = int(match.group(2))
cl = Changelist(issue=issue, codereview='rietveld',
rietveld_server=match.group(1), auth_config=auth_config)
patchset = int(match.group(3))
patch_data = urllib2.urlopen(issue_arg).read()
# Switch up to the top-level directory, if necessary, in preparation for
# applying the patch.
top = settings.GetRelativeRoot()
if top:
os.chdir(top)
# Git patches have a/ at the beginning of source paths. We strip that out
# with a sed script rather than the -p flag to patch so we can feed either
# Git or svn-style patches into the same apply command.
# re.sub() should be used but flags=re.MULTILINE is only in python 2.7.
try:
patch_data = subprocess2.check_output(
['sed', '-e', 's|^--- a/|--- |; s|^+++ b/|+++ |'], stdin=patch_data)
except subprocess2.CalledProcessError:
DieWithError('Git patch mungling failed.')
logging.info(patch_data)
# We use "git apply" to apply the patch instead of "patch" so that we can
# pick up file adds.
# The --index flag means: also insert into the index (so we catch adds).
cmd = ['git', 'apply', '--index', '-p0']
if directory:
cmd.extend(('--directory', directory))
if reject:
cmd.append('--reject')
elif IsGitVersionAtLeast('1.7.12'):
cmd.append('--3way')
try:
subprocess2.check_call(cmd, env=GetNoGitPagerEnv(),
stdin=patch_data, stdout=subprocess2.VOID)
except subprocess2.CalledProcessError:
print 'Failed to apply the patch'
return 1
# If we had an issue, commit the current state and register the issue.
if not nocommit:
RunGit(['commit', '-m', (cl.GetDescription() + '\n\n' +
'patch from issue %(i)s at patchset '
'%(p)s (http://crrev.com/%(i)s#ps%(p)s)'
% {'i': issue, 'p': patchset})])
cl = Changelist(codereview='rietveld', auth_config=auth_config,
rietveld_server=cl.GetCodereviewServer())
cl.SetIssue(issue)
cl.SetPatchset(patchset)
print "Committed patch locally."
else:
print "Patch applied to index."
return 0
def CMDrebase(parser, args): def CMDrebase(parser, args):
...@@ -4293,7 +4157,7 @@ def CMDdiff(parser, args): ...@@ -4293,7 +4157,7 @@ def CMDdiff(parser, args):
parser.error('Unrecognized args: %s' % ' '.join(args)) parser.error('Unrecognized args: %s' % ' '.join(args))
# Uncommitted (staged and unstaged) changes will be destroyed by # Uncommitted (staged and unstaged) changes will be destroyed by
# "git reset --hard" if there are merging conflicts in CMDPatchIssue(). # "git reset --hard" if there are merging conflicts in PatchIssue().
# Staged changes would be committed along with the patch from last # Staged changes would be committed along with the patch from last
# upload, hence counted toward the "last upload" side in the final # upload, hence counted toward the "last upload" side in the final
# diff output, and this is not what we want. # diff output, and this is not what we want.
...@@ -4311,7 +4175,8 @@ def CMDdiff(parser, args): ...@@ -4311,7 +4175,8 @@ def CMDdiff(parser, args):
# Create a new branch based on the merge-base # Create a new branch based on the merge-base
RunGit(['checkout', '-q', '-b', TMP_BRANCH, base_branch]) RunGit(['checkout', '-q', '-b', TMP_BRANCH, base_branch])
try: try:
rtn = cl.CMDPatchIssue(issue, reject=False, nocommit=False, directory=None) # Patch in the latest changes from rietveld.
rtn = PatchIssue(issue, False, False, None, auth_config)
if rtn != 0: if rtn != 0:
RunGit(['reset', '--hard']) RunGit(['reset', '--hard'])
return rtn return rtn
...@@ -4519,18 +4384,16 @@ def CMDformat(parser, args): ...@@ -4519,18 +4384,16 @@ def CMDformat(parser, args):
@subcommand.usage('<codereview url or issue id>') @subcommand.usage('<codereview url or issue id>')
def CMDcheckout(parser, args): def CMDcheckout(parser, args):
"""Checks out a branch associated with a given Rietveld issue.""" """Checks out a branch associated with a given Rietveld issue."""
# TODO(tandrii): consider adding this for Gerrit?
_, args = parser.parse_args(args) _, args = parser.parse_args(args)
if len(args) != 1: if len(args) != 1:
parser.print_help() parser.print_help()
return 1 return 1
issue_arg = ParseIssueNumberArgument(args[0]) target_issue = ParseIssueNum(args[0])
if issue_arg.valid: if target_issue == None:
parser.print_help() parser.print_help()
return 1 return 1
target_issue = issue_arg.issue
key_and_issues = [x.split() for x in RunGit( key_and_issues = [x.split() for x in RunGit(
['config', '--local', '--get-regexp', r'branch\..*\.rietveldissue']) ['config', '--local', '--get-regexp', r'branch\..*\.rietveldissue'])
......
tests/git_cl_test.py
View file @ e0a10585
...@@ -10,7 +10,6 @@ import StringIO ...@@ -10,7 +10,6 @@ import StringIO
import stat import stat
import sys import sys
import unittest import unittest
import urlparse
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
...@@ -74,100 +73,6 @@ class AuthenticatorMock(object): ...@@ -74,100 +73,6 @@ class AuthenticatorMock(object):
return True return True
class TestGitClBasic(unittest.TestCase):
def _test_ParseIssueUrl(self, func, url, issue, patchset, hostname, fail):
parsed = urlparse.urlparse(url)
result = func(parsed)
if fail:
self.assertIsNone(result)
return None
self.assertIsNotNone(result)
self.assertEqual(result.issue, issue)
self.assertEqual(result.patchset, patchset)
self.assertEqual(result.hostname, hostname)
return result
def test_ParseIssueURL_rietveld(self):
def test(url, issue=None, patchset=None, hostname=None, patch_url=None,
fail=None):
result = self._test_ParseIssueUrl(
git_cl._RietveldChangelistImpl.ParseIssueURL,
url, issue, patchset, hostname, fail)
if not fail:
self.assertEqual(result.patch_url, patch_url)
test('http://codereview.chromium.org/123',
123, None, 'codereview.chromium.org')
test('https://codereview.chromium.org/123',
123, None, 'codereview.chromium.org')
test('https://codereview.chromium.org/123/',
123, None, 'codereview.chromium.org')
test('https://codereview.chromium.org/123/whatever',
123, None, 'codereview.chromium.org')
test('http://codereview.chromium.org/download/issue123_4.diff',
123, 4, 'codereview.chromium.org',
patch_url='https://codereview.chromium.org/download/issue123_4.diff')
# This looks like bad Gerrit, but is actually valid Rietveld.
test('https://chrome-review.source.com/123/4/',
123, None, 'chrome-review.source.com')
test('https://codereview.chromium.org/deadbeaf', fail=True)
test('https://codereview.chromium.org/api/123', fail=True)
test('bad://codereview.chromium.org/123', fail=True)
test('http://codereview.chromium.org/download/issue123_4.diffff', fail=True)
def test_ParseIssueURL_gerrit(self):
def test(url, issue=None, patchset=None, hostname=None, fail=None):
self._test_ParseIssueUrl(
git_cl._GerritChangelistImpl.ParseIssueURL,
url, issue, patchset, hostname, fail)
test('http://chrome-review.source.com/c/123',
123, None, 'chrome-review.source.com')
test('https://chrome-review.source.com/c/123/',
123, None, 'chrome-review.source.com')
test('https://chrome-review.source.com/c/123/4',
123, 4, 'chrome-review.source.com')
test('https://chrome-review.source.com/#/c/123/4',
123, 4, 'chrome-review.source.com')
test('https://chrome-review.source.com/c/123/4',
123, 4, 'chrome-review.source.com')
test('https://chrome-review.source.com/123',
123, None, 'chrome-review.source.com')
test('https://chrome-review.source.com/123/4',
123, 4, 'chrome-review.source.com')
test('https://chrome-review.source.com/c/123/1/whatisthis', fail=True)
test('https://chrome-review.source.com/c/abc/', fail=True)
test('ssh://chrome-review.source.com/c/123/1/', fail=True)
def test_ParseIssueNumberArgument(self):
def test(arg, issue=None, patchset=None, hostname=None, fail=False):
result = git_cl.ParseIssueNumberArgument(arg)
self.assertIsNotNone(result)
if fail:
self.assertFalse(result.valid)
else:
self.assertEqual(result.issue, issue)
self.assertEqual(result.patchset, patchset)
self.assertEqual(result.hostname, hostname)
test('123', 123)
test('', fail=True)
test('abc', fail=True)
test('123/1', fail=True)
test('123a', fail=True)
test('ssh://chrome-review.source.com/#/c/123/4/', fail=True)
# Rietveld.
test('https://codereview.source.com/123',
123, None, 'codereview.source.com')
test('https://codereview.source.com/www123', fail=True)
# Gerrrit.
test('https://chrome-review.source.com/c/123/4',
123, 4, 'chrome-review.source.com')
test('https://chrome-review.source.com/bad/123/4', fail=True)
class TestGitCl(TestCase): class TestGitCl(TestCase):
def setUp(self): def setUp(self):
super(TestGitCl, self).setUp() super(TestGitCl, self).setUp()
...@@ -194,12 +99,8 @@ class TestGitCl(TestCase): ...@@ -194,12 +99,8 @@ class TestGitCl(TestCase):
# It's important to reset settings to not have inter-tests interference. # It's important to reset settings to not have inter-tests interference.
git_cl.settings = None git_cl.settings = None
def tearDown(self): def tearDown(self):
try: try:
# Note: has_failed returns True if at least 1 test ran so far, current
# included, has failed. That means current test may have actually ran
# fine, and the check for no leftover calls would be skipped.
if not self.has_failed(): if not self.has_failed():
self.assertEquals([], self.calls) self.assertEquals([], self.calls)
finally: finally:
...@@ -1015,14 +916,6 @@ class TestGitCl(TestCase): ...@@ -1015,14 +916,6 @@ class TestGitCl(TestCase):
def test_patch_when_dirty(self): def test_patch_when_dirty(self):
# Patch when local tree is dirty # Patch when local tree is dirty
self.mock(git_common, 'is_dirty_git_tree', lambda x: True) self.mock(git_common, 'is_dirty_git_tree', lambda x: True)
self.calls = [
((['git', 'symbolic-ref', 'HEAD'],), 'master'),
((['git', 'config', 'branch.master.rietveldissue'],), ''),
((['git', 'config', 'branch.master.gerritissue'],), ''),
((['git', 'config', 'rietveld.autoupdate'],), ''),
((['git', 'config', 'gerrit.host'],), ''),
((['git', 'config', 'rietveld.server'],), 'codereview.example.com'),
]
self.assertNotEqual(git_cl.main(['patch', '123456']), 0) self.assertNotEqual(git_cl.main(['patch', '123456']), 0)
def test_diff_when_dirty(self): def test_diff_when_dirty(self):
...@@ -1030,48 +923,19 @@ class TestGitCl(TestCase): ...@@ -1030,48 +923,19 @@ class TestGitCl(TestCase):
self.mock(git_common, 'is_dirty_git_tree', lambda x: True) self.mock(git_common, 'is_dirty_git_tree', lambda x: True)
self.assertNotEqual(git_cl.main(['diff']), 0) self.assertNotEqual(git_cl.main(['diff']), 0)
def _patch_common(self, is_gerrit=False): def _patch_common(self):
self.mock(git_cl._RietveldChangelistImpl, 'GetMostRecentPatchset', self.mock(git_cl._RietveldChangelistImpl, 'GetMostRecentPatchset',
lambda x: '60001') lambda x: '60001')
self.mock(git_cl._RietveldChangelistImpl, 'GetPatchSetDiff', self.mock(git_cl._RietveldChangelistImpl, 'GetPatchSetDiff',
lambda *args: None) lambda *args: None)
self.mock(git_cl._GerritChangelistImpl, '_GetChangeDetail',
lambda *args: {
'current_revision': '7777777777',
'revisions': {
'1111111111': {
'_number': 1,
'fetch': {'http': {
'url': 'https://chromium.googlesource.com/my/repo',
'ref': 'refs/changes/56/123456/1',
}},
},
'7777777777': {
'_number': 7,
'fetch': {'http': {
'url': 'https://chromium.googlesource.com/my/repo',
'ref': 'refs/changes/56/123456/7',
}},
},
},
})
self.mock(git_cl.Changelist, 'GetDescription', self.mock(git_cl.Changelist, 'GetDescription',
lambda *args: 'Description') lambda *args: 'Description')
self.mock(git_cl.Changelist, 'SetIssue', lambda *args: None)
self.mock(git_cl.Changelist, 'SetPatchset', lambda *args: None)
self.mock(git_cl, 'IsGitVersionAtLeast', lambda *args: True) self.mock(git_cl, 'IsGitVersionAtLeast', lambda *args: True)
self.calls = [ self.calls = [
((['git', 'symbolic-ref', 'HEAD'],), 'master'),
((['git', 'config', 'branch.master.rietveldissue'],), ''),
((['git', 'config', 'branch.master.gerritissue'],), ''),
((['git', 'config', 'rietveld.autoupdate'],), ''), ((['git', 'config', 'rietveld.autoupdate'],), ''),
]
if is_gerrit:
self.calls += [
((['git', 'config', 'gerrit.host'],), 'true'),
]
else:
self.calls += [
((['git', 'config', 'gerrit.host'],), ''),
((['git', 'config', 'rietveld.server'],), 'codereview.example.com'), ((['git', 'config', 'rietveld.server'],), 'codereview.example.com'),
((['git', 'rev-parse', '--show-cdup'],), ''), ((['git', 'rev-parse', '--show-cdup'],), ''),
((['sed', '-e', 's|^--- a/|--- |; s|^+++ b/|+++ |'],), ''), ((['sed', '-e', 's|^--- a/|--- |; s|^+++ b/|+++ |'],), ''),
...@@ -1085,11 +949,8 @@ class TestGitCl(TestCase): ...@@ -1085,11 +949,8 @@ class TestGitCl(TestCase):
'Description\n\n' + 'Description\n\n' +
'patch from issue 123456 at patchset 60001 ' + 'patch from issue 123456 at patchset 60001 ' +
'(http://crrev.com/123456#ps60001)'],), ''), '(http://crrev.com/123456#ps60001)'],), ''),
((['git', 'config', 'branch.master.rietveldissue', '123456'],), ''), ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
((['git', 'config', 'branch.master.rietveldserver'],), ''), ((['git', 'config', 'branch.master.rietveldserver'],), ''),
((['git', 'config', 'branch.master.rietveldserver',
'https://codereview.example.com'],), ''),
((['git', 'config', 'branch.master.rietveldpatchset', '60001'],), ''),
] ]
self.assertEqual(git_cl.main(['patch', '123456']), 0) self.assertEqual(git_cl.main(['patch', '123456']), 0)
...@@ -1101,57 +962,6 @@ class TestGitCl(TestCase): ...@@ -1101,57 +962,6 @@ class TestGitCl(TestCase):
] ]
self.assertNotEqual(git_cl.main(['patch', '123456']), 0) self.assertNotEqual(git_cl.main(['patch', '123456']), 0)
def test_gerrit_patch_successful(self):
self._patch_common(is_gerrit=True)
self.calls += [
((['git', 'fetch', 'https://chromium.googlesource.com/my/repo',
'refs/changes/56/123456/7'],), ''),
((['git', 'cherry-pick', 'FETCH_HEAD'],), ''),
((['git', 'config', 'branch.master.gerritissue', '123456'],), ''),
((['git', 'config', 'branch.master.gerritserver'],), ''),
((['git', 'config', 'branch.master.merge'],), 'master'),
((['git', 'config', 'branch.master.remote'],), 'origin'),
((['git', 'config', 'remote.origin.url'],),
'https://chromium.googlesource.com/my/repo'),
((['git', 'config', 'branch.master.gerritserver',
'https://chromium-review.googlesource.com'],), ''),
((['git', 'config', 'branch.master.gerritpatchset', '7'],), ''),
]
self.assertEqual(git_cl.main(['patch', '123456']), 0)
def test_gerrit_patch_url_successful(self):
self._patch_common(is_gerrit=True)
self.calls += [
((['git', 'fetch', 'https://chromium.googlesource.com/my/repo',
'refs/changes/56/123456/1'],), ''),
((['git', 'cherry-pick', 'FETCH_HEAD'],), ''),
((['git', 'config', 'branch.master.gerritissue', '123456'],), ''),
((['git', 'config', 'branch.master.gerritserver',
'https://chromium-review.googlesource.com'],), ''),
((['git', 'config', 'branch.master.gerritpatchset', '1'],), ''),
]
self.assertEqual(git_cl.main(
['patch', 'https://chromium-review.googlesource.com/#/c/123456/1']), 0)
def test_gerrit_patch_conflict(self):
self._patch_common(is_gerrit=True)
self.mock(git_cl, 'DieWithError',
lambda msg: self._mocked_call(['DieWithError', msg]))
class SystemExitMock(Exception):
pass
self.calls += [
((['git', 'fetch', 'https://chromium.googlesource.com/my/repo',
'refs/changes/56/123456/1'],), ''),
((['git', 'cherry-pick', 'FETCH_HEAD'],),
'', subprocess2.CalledProcessError(1, '', '', '', '')),
((['DieWithError', 'git cherry-pick FETCH_HEAD" failed.\n'],),
'', SystemExitMock()),
]
with self.assertRaises(SystemExitMock):
git_cl.main(['patch',
'https://chromium-review.googlesource.com/#/c/123456/1'])
if __name__ == '__main__': if __name__ == '__main__':
git_cl.logging.basicConfig( git_cl.logging.basicConfig(
level=git_cl.logging.DEBUG if '-v' in sys.argv else git_cl.logging.ERROR) level=git_cl.logging.DEBUG if '-v' in sys.argv else git_cl.logging.ERROR)
......
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