Commit f6a2232b authored by Edward Lesmes's avatar Edward Lesmes Committed by Commit Bot

Revert "git-cl: Fix some python3 compatibility errors."

This reverts commit c87ed606.

Reason for revert: 
Causes a regressions for CL descriptions on Windows

Original change's description:
> git-cl: Fix some python3 compatibility errors.
> 
> Also, fix bug in git cl status where the 'updated' field was used to compare messages, even though
> it doesn't exist (see [1]). This CL modifies it to use 'date', which does exist.
> 
> [1] https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#change-message-info
> 
> Bug: 1002209
> Change-Id: I5a5e1193b8502c3ad35d94808ea178cad7f44ac6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1891259
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Anthony Polito <apolito@google.com>

TBR=ehmaldonado@chromium.org,apolito@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1002209
Change-Id: I004f202b12c6b99cb6b24cb12a14fba7898569e2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1898547Reviewed-by: 's avatarEdward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
parent 288e358f
...@@ -54,13 +54,16 @@ import subcommand ...@@ -54,13 +54,16 @@ import subcommand
import subprocess2 import subprocess2
import watchlists import watchlists
from third_party import six if sys.version_info.major == 2:
from six.moves import urllib import httplib
import urllib2 as urllib_request
import urllib2 as urllib_error
if sys.version_info.major == 3: import urlparse
basestring = (str,) # pylint: disable=redefined-builtin else:
import http.client as httplib
import urllib.request as urllib_request
import urllib.error as urllib_error
import urllib.parse as urlparse
__version__ = '2.0' __version__ = '2.0'
...@@ -154,15 +157,14 @@ def GetNoGitPagerEnv(): ...@@ -154,15 +157,14 @@ def GetNoGitPagerEnv():
def RunCommand(args, error_ok=False, error_message=None, shell=False, **kwargs): def RunCommand(args, error_ok=False, error_message=None, shell=False, **kwargs):
try: try:
stdout = subprocess2.check_output(args, shell=shell, **kwargs) return subprocess2.check_output(args, shell=shell, **kwargs)
return stdout.decode('utf-8', 'replace')
except subprocess2.CalledProcessError as e: except subprocess2.CalledProcessError as e:
logging.debug('Failed running %s', args) logging.debug('Failed running %s', args)
if not error_ok: if not error_ok:
DieWithError( DieWithError(
'Command "%s" failed.\n%s' % ( 'Command "%s" failed.\n%s' % (
' '.join(args), error_message or e.stdout or '')) ' '.join(args), error_message or e.stdout or ''))
return e.stdout.decode('utf-8', 'replace') return e.stdout
def RunGit(args, **kwargs): def RunGit(args, **kwargs):
...@@ -181,10 +183,10 @@ def RunGitWithCode(args, suppress_stderr=False): ...@@ -181,10 +183,10 @@ def RunGitWithCode(args, suppress_stderr=False):
env=GetNoGitPagerEnv(), env=GetNoGitPagerEnv(),
stdout=subprocess2.PIPE, stdout=subprocess2.PIPE,
stderr=stderr) stderr=stderr)
return code, out.decode('utf-8', 'replace') return code, out
except subprocess2.CalledProcessError as e: except subprocess2.CalledProcessError as e:
logging.debug('Failed running %s', ['git'] + args) logging.debug('Failed running %s', ['git'] + args)
return e.returncode, e.stdout.decode('utf-8', 'replace') return e.returncode, e.stdout
def RunGitSilent(args): def RunGitSilent(args):
...@@ -1015,7 +1017,7 @@ def ParseIssueNumberArgument(arg): ...@@ -1015,7 +1017,7 @@ def ParseIssueNumberArgument(arg):
url = gclient_utils.UpgradeToHttps(arg) url = gclient_utils.UpgradeToHttps(arg)
try: try:
parsed_url = urllib.parse.urlparse(url) parsed_url = urlparse.urlparse(url)
except ValueError: except ValueError:
return fail_result return fail_result
...@@ -1320,7 +1322,7 @@ class Changelist(object): ...@@ -1320,7 +1322,7 @@ class Changelist(object):
url = RunGit(['config', 'remote.%s.url' % remote], error_ok=True).strip() url = RunGit(['config', 'remote.%s.url' % remote], error_ok=True).strip()
# Check if the remote url can be parsed as an URL. # Check if the remote url can be parsed as an URL.
host = urllib.parse.urlparse(url).netloc host = urlparse.urlparse(url).netloc
if host: if host:
self._cached_remote_url = (True, url) self._cached_remote_url = (True, url)
return url return url
...@@ -1340,7 +1342,7 @@ class Changelist(object): ...@@ -1340,7 +1342,7 @@ class Changelist(object):
error_ok=True, error_ok=True,
cwd=url).strip() cwd=url).strip()
host = urllib.parse.urlparse(url).netloc host = urlparse.urlparse(url).netloc
if not host: if not host:
logging.error( logging.error(
'Remote "%(remote)s" for branch "%(branch)s" points to ' 'Remote "%(remote)s" for branch "%(branch)s" points to '
...@@ -1653,7 +1655,7 @@ class Changelist(object): ...@@ -1653,7 +1655,7 @@ class Changelist(object):
if self._gerrit_host and '.' not in self._gerrit_host: if self._gerrit_host and '.' not in self._gerrit_host:
# Abbreviated domain like "chromium" instead of chromium.googlesource.com. # Abbreviated domain like "chromium" instead of chromium.googlesource.com.
# This happens for internal stuff http://crbug.com/614312. # This happens for internal stuff http://crbug.com/614312.
parsed = urllib.parse.urlparse(self.GetRemoteUrl()) parsed = urlparse.urlparse(self.GetRemoteUrl())
if parsed.scheme == 'sso': if parsed.scheme == 'sso':
print('WARNING: using non-https URLs for remote is likely broken\n' print('WARNING: using non-https URLs for remote is likely broken\n'
' Your current remote is: %s' % self.GetRemoteUrl()) ' Your current remote is: %s' % self.GetRemoteUrl())
...@@ -1666,7 +1668,7 @@ class Changelist(object): ...@@ -1666,7 +1668,7 @@ class Changelist(object):
remote_url = self.GetRemoteUrl() remote_url = self.GetRemoteUrl()
if not remote_url: if not remote_url:
return None return None
return urllib.parse.urlparse(remote_url).netloc return urlparse.urlparse(remote_url).netloc
def GetCodereviewServer(self): def GetCodereviewServer(self):
if not self._gerrit_server: if not self._gerrit_server:
...@@ -1676,7 +1678,7 @@ class Changelist(object): ...@@ -1676,7 +1678,7 @@ class Changelist(object):
self._gerrit_server = self._GitGetBranchConfigValue( self._gerrit_server = self._GitGetBranchConfigValue(
self.CodereviewServerConfigKey()) self.CodereviewServerConfigKey())
if self._gerrit_server: if self._gerrit_server:
self._gerrit_host = urllib.parse.urlparse(self._gerrit_server).netloc self._gerrit_host = urlparse.urlparse(self._gerrit_server).netloc
if not self._gerrit_server: if not self._gerrit_server:
# We assume repo to be hosted on Gerrit, and hence Gerrit server # We assume repo to be hosted on Gerrit, and hence Gerrit server
# has "-review" suffix for lowest level subdomain. # has "-review" suffix for lowest level subdomain.
...@@ -1692,7 +1694,7 @@ class Changelist(object): ...@@ -1692,7 +1694,7 @@ class Changelist(object):
if remote_url is None: if remote_url is None:
logging.warn('can\'t detect Gerrit project.') logging.warn('can\'t detect Gerrit project.')
return None return None
project = urllib.parse.urlparse(remote_url).path.strip('/') project = urlparse.urlparse(remote_url).path.strip('/')
if project.endswith('.git'): if project.endswith('.git'):
project = project[:-len('.git')] project = project[:-len('.git')]
# *.googlesource.com hosts ensure that Git/Gerrit projects don't start with # *.googlesource.com hosts ensure that Git/Gerrit projects don't start with
...@@ -1741,7 +1743,7 @@ class Changelist(object): ...@@ -1741,7 +1743,7 @@ class Changelist(object):
if not isinstance(cookie_auth, gerrit_util.CookiesAuthenticator): if not isinstance(cookie_auth, gerrit_util.CookiesAuthenticator):
return return
if urllib.parse.urlparse(self.GetRemoteUrl()).scheme != 'https': if urlparse.urlparse(self.GetRemoteUrl()).scheme != 'https':
print('WARNING: Ignoring branch %s with non-https remote %s' % print('WARNING: Ignoring branch %s with non-https remote %s' %
(self.branch, self.GetRemoteUrl())) (self.branch, self.GetRemoteUrl()))
return return
...@@ -1851,7 +1853,7 @@ class Changelist(object): ...@@ -1851,7 +1853,7 @@ class Changelist(object):
try: try:
data = self._GetChangeDetail([ data = self._GetChangeDetail([
'DETAILED_LABELS', 'CURRENT_REVISION', 'SUBMITTABLE']) 'DETAILED_LABELS', 'CURRENT_REVISION', 'SUBMITTABLE'])
except GerritChangeNotExists: except (httplib.HTTPException, GerritChangeNotExists):
return 'error' return 'error'
if data['status'] in ('ABANDONED', 'MERGED'): if data['status'] in ('ABANDONED', 'MERGED'):
...@@ -1873,7 +1875,7 @@ class Changelist(object): ...@@ -1873,7 +1875,7 @@ class Changelist(object):
return 'unsent' return 'unsent'
owner = data['owner'].get('_account_id') owner = data['owner'].get('_account_id')
messages = sorted(data.get('messages', []), key=lambda m: m.get('date')) messages = sorted(data.get('messages', []), key=lambda m: m.get('updated'))
last_message_author = messages.pop().get('author', {}) last_message_author = messages.pop().get('author', {})
while last_message_author: while last_message_author:
if last_message_author.get('email') == COMMIT_BOT_EMAIL: if last_message_author.get('email') == COMMIT_BOT_EMAIL:
...@@ -1900,7 +1902,8 @@ class Changelist(object): ...@@ -1900,7 +1902,8 @@ class Changelist(object):
data = self._GetChangeDetail(['CURRENT_REVISION', 'CURRENT_COMMIT'], data = self._GetChangeDetail(['CURRENT_REVISION', 'CURRENT_COMMIT'],
no_cache=force) no_cache=force)
current_rev = data['current_revision'] current_rev = data['current_revision']
return data['revisions'][current_rev]['commit']['message'] return data['revisions'][current_rev]['commit']['message'].encode(
'utf-8', 'ignore')
def UpdateDescriptionRemote(self, description, force=False): def UpdateDescriptionRemote(self, description, force=False):
if gerrit_util.HasPendingChangeEdit( if gerrit_util.HasPendingChangeEdit(
...@@ -2321,7 +2324,6 @@ class Changelist(object): ...@@ -2321,7 +2324,6 @@ class Changelist(object):
# Flush after every line: useful for seeing progress when running as # Flush after every line: useful for seeing progress when running as
# recipe. # recipe.
filter_fn=lambda _: sys.stdout.flush()) filter_fn=lambda _: sys.stdout.flush())
push_stdout = push_stdout.decode('utf-8', 'replace')
except subprocess2.CalledProcessError as e: except subprocess2.CalledProcessError as e:
push_returncode = e.returncode push_returncode = e.returncode
DieWithError('Failed to create a change. Please examine output above ' DieWithError('Failed to create a change. Please examine output above '
...@@ -2475,7 +2477,7 @@ class Changelist(object): ...@@ -2475,7 +2477,7 @@ class Changelist(object):
parent = self._ComputeParent(remote, upstream_branch, custom_cl_base, parent = self._ComputeParent(remote, upstream_branch, custom_cl_base,
options.force, change_desc) options.force, change_desc)
tree = RunGit(['rev-parse', 'HEAD:']).strip() tree = RunGit(['rev-parse', 'HEAD:']).strip()
with tempfile.NamedTemporaryFile('w', delete=False) as desc_tempfile: with tempfile.NamedTemporaryFile(delete=False) as desc_tempfile:
desc_tempfile.write(change_desc.description) desc_tempfile.write(change_desc.description)
desc_tempfile.close() desc_tempfile.close()
ref_to_push = RunGit(['commit-tree', tree, '-p', parent, ref_to_push = RunGit(['commit-tree', tree, '-p', parent,
...@@ -2526,7 +2528,7 @@ class Changelist(object): ...@@ -2526,7 +2528,7 @@ class Changelist(object):
# Add cc's from the --cc flag. # Add cc's from the --cc flag.
if options.cc: if options.cc:
cc.extend(options.cc) cc.extend(options.cc)
cc = [email.strip() for email in cc if email.strip()] cc = filter(None, [email.strip() for email in cc])
if change_desc.get_cced(): if change_desc.get_cced():
cc.extend(change_desc.get_cced()) cc.extend(change_desc.get_cced())
if self._GetGerritHost() == 'chromium-review.googlesource.com': if self._GetGerritHost() == 'chromium-review.googlesource.com':
...@@ -2727,7 +2729,7 @@ class Changelist(object): ...@@ -2727,7 +2729,7 @@ class Changelist(object):
def GetGerritChange(self, patchset=None): def GetGerritChange(self, patchset=None):
"""Returns a buildbucket.v2.GerritChange message for the current issue.""" """Returns a buildbucket.v2.GerritChange message for the current issue."""
host = urllib.parse.urlparse(self.GetCodereviewServer()).hostname host = urlparse.urlparse(self.GetCodereviewServer()).hostname
issue = self.GetIssue() issue = self.GetIssue()
patchset = int(patchset or self.GetPatchset()) patchset = int(patchset or self.GetPatchset())
data = self._GetChangeDetail(['ALL_REVISIONS']) data = self._GetChangeDetail(['ALL_REVISIONS'])
...@@ -3191,7 +3193,7 @@ def urlretrieve(source, destination): ...@@ -3191,7 +3193,7 @@ def urlretrieve(source, destination):
This is necessary because urllib is broken for SSL connections via a proxy. This is necessary because urllib is broken for SSL connections via a proxy.
""" """
with open(destination, 'w') as f: with open(destination, 'w') as f:
f.write(urllib.request.urlopen(source).read()) f.write(urllib_request.urlopen(source).read())
def hasSheBang(fname): def hasSheBang(fname):
...@@ -3327,7 +3329,7 @@ class _GitCookiesChecker(object): ...@@ -3327,7 +3329,7 @@ class _GitCookiesChecker(object):
if not hosts: if not hosts:
print('No Git/Gerrit credentials found') print('No Git/Gerrit credentials found')
return return
lengths = [max(map(len, (row[i] for row in hosts))) for i in range(3)] lengths = [max(map(len, (row[i] for row in hosts))) for i in xrange(3)]
header = [('Host', 'User', 'Which file'), header = [('Host', 'User', 'Which file'),
['=' * l for l in lengths]] ['=' * l for l in lengths]]
for row in (header + hosts): for row in (header + hosts):
...@@ -3894,7 +3896,7 @@ def CMDstatus(parser, args): ...@@ -3894,7 +3896,7 @@ def CMDstatus(parser, args):
for cl in sorted(changes, key=lambda c: c.GetBranch()): for cl in sorted(changes, key=lambda c: c.GetBranch()):
branch = cl.GetBranch() branch = cl.GetBranch()
while branch not in branch_statuses: while branch not in branch_statuses:
c, status = next(output) c, status = output.next()
branch_statuses[c.GetBranch()] = status branch_statuses[c.GetBranch()] = status
status = branch_statuses.pop(branch) status = branch_statuses.pop(branch)
url = cl.GetIssueURL() url = cl.GetIssueURL()
...@@ -4079,10 +4081,10 @@ def CMDcomments(parser, args): ...@@ -4079,10 +4081,10 @@ def CMDcomments(parser, args):
if options.json_file: if options.json_file:
def pre_serialize(c): def pre_serialize(c):
dct = c._asdict().copy() dct = c.__dict__.copy()
dct['date'] = dct['date'].strftime('%Y-%m-%d %H:%M:%S.%f') dct['date'] = dct['date'].strftime('%Y-%m-%d %H:%M:%S.%f')
return dct return dct
write_json(options.json_file, [pre_serialize(x) for x in summary]) write_json(options.json_file, map(pre_serialize, summary))
return 0 return 0
...@@ -4664,7 +4666,7 @@ def GetTreeStatus(url=None): ...@@ -4664,7 +4666,7 @@ def GetTreeStatus(url=None):
'unknown' or 'unset'.""" 'unknown' or 'unset'."""
url = url or settings.GetTreeStatusUrl(error_ok=True) url = url or settings.GetTreeStatusUrl(error_ok=True)
if url: if url:
status = urllib.request.urlopen(url).read().lower() status = urllib_request.urlopen(url).read().lower()
if status.find('closed') != -1 or status == '0': if status.find('closed') != -1 or status == '0':
return 'closed' return 'closed'
elif status.find('open') != -1 or status == '1': elif status.find('open') != -1 or status == '1':
...@@ -4678,7 +4680,7 @@ def GetTreeStatusReason(): ...@@ -4678,7 +4680,7 @@ def GetTreeStatusReason():
with the reason for the tree to be opened or closed.""" with the reason for the tree to be opened or closed."""
url = settings.GetTreeStatusUrl() url = settings.GetTreeStatusUrl()
json_url = urlparse.urljoin(url, '/current?format=json') json_url = urlparse.urljoin(url, '/current?format=json')
connection = urllib.request.urlopen(json_url) connection = urllib_request.urlopen(json_url)
status = json.loads(connection.read()) status = json.loads(connection.read())
connection.close() connection.close()
return status['message'] return status['message']
...@@ -5444,7 +5446,7 @@ class OptionParser(optparse.OptionParser): ...@@ -5444,7 +5446,7 @@ class OptionParser(optparse.OptionParser):
# Store the options passed by the user in an _actual_options attribute. # Store the options passed by the user in an _actual_options attribute.
# We store only the keys, and not the values, since the values can contain # We store only the keys, and not the values, since the values can contain
# arbitrary information, which might be PII. # arbitrary information, which might be PII.
metrics.collector.add('arguments', list(actual_options.__dict__.keys())) metrics.collector.add('arguments', actual_options.__dict__.keys())
levels = [logging.WARNING, logging.INFO, logging.DEBUG] levels = [logging.WARNING, logging.INFO, logging.DEBUG]
logging.basicConfig( logging.basicConfig(
...@@ -5467,7 +5469,7 @@ def main(argv): ...@@ -5467,7 +5469,7 @@ def main(argv):
return dispatcher.execute(OptionParser(), argv) return dispatcher.execute(OptionParser(), argv)
except auth.LoginRequiredError as e: except auth.LoginRequiredError as e:
DieWithError(str(e)) DieWithError(str(e))
except urllib.error.HTTPError as e: except urllib_error.HTTPError as e:
if e.code != 500: if e.code != 500:
raise raise
DieWithError( DieWithError(
......
...@@ -10,9 +10,11 @@ import json ...@@ -10,9 +10,11 @@ import json
import logging import logging
import os import os
import shutil import shutil
import StringIO
import sys import sys
import tempfile import tempfile
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__))))
...@@ -29,11 +31,6 @@ import git_common ...@@ -29,11 +31,6 @@ import git_common
import git_footers import git_footers
import subprocess2 import subprocess2
if sys.version_info.major == 2:
from StringIO import StringIO
else:
from io import StringIO
def callError(code=1, cmd='', cwd='', stdout='', stderr=''): def callError(code=1, cmd='', cwd='', stdout='', stderr=''):
return subprocess2.CalledProcessError(code, cmd, cwd, stdout, stderr) return subprocess2.CalledProcessError(code, cmd, cwd, stdout, stderr)
...@@ -574,13 +571,13 @@ class GitCookiesCheckerTest(TestCase): ...@@ -574,13 +571,13 @@ class GitCookiesCheckerTest(TestCase):
def test_report_no_problems(self): def test_report_no_problems(self):
self.test_analysis_nothing() self.test_analysis_nothing()
self.mock(sys, 'stdout', StringIO()) self.mock(sys, 'stdout', StringIO.StringIO())
self.assertFalse(self.c.find_and_report_problems()) self.assertFalse(self.c.find_and_report_problems())
self.assertEqual(sys.stdout.getvalue(), '') self.assertEqual(sys.stdout.getvalue(), '')
def test_report(self): def test_report(self):
self.test_analysis() self.test_analysis()
self.mock(sys, 'stdout', StringIO()) self.mock(sys, 'stdout', StringIO.StringIO())
self.mock(git_cl.gerrit_util.CookiesAuthenticator, 'get_gitcookies_path', self.mock(git_cl.gerrit_util.CookiesAuthenticator, 'get_gitcookies_path',
classmethod(lambda _: '~/.gitcookies')) classmethod(lambda _: '~/.gitcookies'))
self.assertTrue(self.c.find_and_report_problems()) self.assertTrue(self.c.find_and_report_problems())
...@@ -716,7 +713,7 @@ class TestGitCl(TestCase): ...@@ -716,7 +713,7 @@ class TestGitCl(TestCase):
self.assertTrue(git_cl.ask_for_explicit_yes('prompt')) self.assertTrue(git_cl.ask_for_explicit_yes('prompt'))
def test_LoadCodereviewSettingsFromFile_gerrit(self): def test_LoadCodereviewSettingsFromFile_gerrit(self):
codereview_file = StringIO('GERRIT_HOST: true') codereview_file = StringIO.StringIO('GERRIT_HOST: true')
self.calls = [ self.calls = [
((['git', 'config', '--unset-all', 'rietveld.cc'],), CERR1), ((['git', 'config', '--unset-all', 'rietveld.cc'],), CERR1),
((['git', 'config', '--unset-all', 'rietveld.tree-status-url'],), CERR1), ((['git', 'config', '--unset-all', 'rietveld.tree-status-url'],), CERR1),
...@@ -1243,7 +1240,7 @@ class TestGitCl(TestCase): ...@@ -1243,7 +1240,7 @@ class TestGitCl(TestCase):
reviewers = reviewers or [] reviewers = reviewers or []
cc = cc or [] cc = cc or []
self.mock(git_cl.sys, 'stdout', StringIO()) self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
self.mock(git_cl.gerrit_util, 'CookiesAuthenticator', self.mock(git_cl.gerrit_util, 'CookiesAuthenticator',
CookiesAuthenticatorMockFactory( CookiesAuthenticatorMockFactory(
same_auth=('git-owner.example.com', '', 'pass'))) same_auth=('git-owner.example.com', '', 'pass')))
...@@ -1357,7 +1354,7 @@ class TestGitCl(TestCase): ...@@ -1357,7 +1354,7 @@ class TestGitCl(TestCase):
change_id='I123456789') change_id='I123456789')
def test_gerrit_patchset_title_special_chars(self): def test_gerrit_patchset_title_special_chars(self):
self.mock(git_cl.sys, 'stdout', StringIO()) self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
self._run_gerrit_upload_test( self._run_gerrit_upload_test(
['-f', '-t', 'We\'ll escape ^_ ^ special chars...@{u}'], ['-f', '-t', 'We\'ll escape ^_ ^ special chars...@{u}'],
'desc\n\nBUG=\n\nChange-Id: I123456789', 'desc\n\nBUG=\n\nChange-Id: I123456789',
...@@ -1523,7 +1520,7 @@ class TestGitCl(TestCase): ...@@ -1523,7 +1520,7 @@ class TestGitCl(TestCase):
git_cl.sys.stdout.getvalue()) git_cl.sys.stdout.getvalue())
def test_upload_branch_deps(self): def test_upload_branch_deps(self):
self.mock(git_cl.sys, 'stdout', StringIO()) self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
def mock_run_git(*args, **_kwargs): def mock_run_git(*args, **_kwargs):
if args[0] == ['for-each-ref', if args[0] == ['for-each-ref',
'--format=%(refname:short) %(upstream:short)', '--format=%(refname:short) %(upstream:short)',
...@@ -1781,7 +1778,7 @@ class TestGitCl(TestCase): ...@@ -1781,7 +1778,7 @@ class TestGitCl(TestCase):
detect_gerrit_server=False, detect_gerrit_server=False,
actual_codereview=None, actual_codereview=None,
codereview_in_url=False): codereview_in_url=False):
self.mock(git_cl.sys, 'stdout', StringIO()) self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
self.mock(git_cl, 'IsGitVersionAtLeast', lambda *args: True) self.mock(git_cl, 'IsGitVersionAtLeast', lambda *args: True)
if new_branch: if new_branch:
...@@ -1975,13 +1972,13 @@ class TestGitCl(TestCase): ...@@ -1975,13 +1972,13 @@ class TestGitCl(TestCase):
def test_checkout_not_found(self): def test_checkout_not_found(self):
"""Tests git cl checkout <issue>.""" """Tests git cl checkout <issue>."""
self.mock(git_cl.sys, 'stdout', StringIO()) self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
self.calls = self._checkout_calls() self.calls = self._checkout_calls()
self.assertEqual(1, git_cl.main(['checkout', '99999'])) self.assertEqual(1, git_cl.main(['checkout', '99999']))
def test_checkout_no_branch_issues(self): def test_checkout_no_branch_issues(self):
"""Tests git cl checkout <issue>.""" """Tests git cl checkout <issue>."""
self.mock(git_cl.sys, 'stdout', StringIO()) self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
self.calls = [ self.calls = [
((['git', 'config', '--local', '--get-regexp', ((['git', 'config', '--local', '--get-regexp',
'branch\\..*\\.gerritissue'], ), CERR1), 'branch\\..*\\.gerritissue'], ), CERR1),
...@@ -2014,7 +2011,7 @@ class TestGitCl(TestCase): ...@@ -2014,7 +2011,7 @@ class TestGitCl(TestCase):
self.assertIsNone(cl.EnsureAuthenticated(force=False)) self.assertIsNone(cl.EnsureAuthenticated(force=False))
def test_gerrit_ensure_authenticated_conflict(self): def test_gerrit_ensure_authenticated_conflict(self):
self.mock(git_cl.sys, 'stdout', StringIO()) self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
cl = self._test_gerrit_ensure_authenticated_common(auth={ cl = self._test_gerrit_ensure_authenticated_common(auth={
'chromium.googlesource.com': 'chromium.googlesource.com':
('git-one.example.com', None, 'secret1'), ('git-one.example.com', None, 'secret1'),
...@@ -2100,7 +2097,7 @@ class TestGitCl(TestCase): ...@@ -2100,7 +2097,7 @@ class TestGitCl(TestCase):
self.assertEqual(0, git_cl.main(['set-commit'])) self.assertEqual(0, git_cl.main(['set-commit']))
def test_description_display(self): def test_description_display(self):
out = StringIO() out = StringIO.StringIO()
self.mock(git_cl.sys, 'stdout', out) self.mock(git_cl.sys, 'stdout', out)
self.mock(git_cl, 'Changelist', ChangelistMock) self.mock(git_cl, 'Changelist', ChangelistMock)
...@@ -2110,7 +2107,7 @@ class TestGitCl(TestCase): ...@@ -2110,7 +2107,7 @@ class TestGitCl(TestCase):
self.assertEqual('foo\n', out.getvalue()) self.assertEqual('foo\n', out.getvalue())
def test_StatusFieldOverrideIssueMissingArgs(self): def test_StatusFieldOverrideIssueMissingArgs(self):
out = StringIO() out = StringIO.StringIO()
self.mock(git_cl.sys, 'stderr', out) self.mock(git_cl.sys, 'stderr', out)
try: try:
...@@ -2119,7 +2116,7 @@ class TestGitCl(TestCase): ...@@ -2119,7 +2116,7 @@ class TestGitCl(TestCase):
self.assertEqual(ex.code, 2) self.assertEqual(ex.code, 2)
self.assertRegexpMatches(out.getvalue(), r'--field must be specified') self.assertRegexpMatches(out.getvalue(), r'--field must be specified')
out = StringIO() out = StringIO.StringIO()
self.mock(git_cl.sys, 'stderr', out) self.mock(git_cl.sys, 'stderr', out)
try: try:
...@@ -2129,7 +2126,7 @@ class TestGitCl(TestCase): ...@@ -2129,7 +2126,7 @@ class TestGitCl(TestCase):
self.assertRegexpMatches(out.getvalue(), r'--field must be specified') self.assertRegexpMatches(out.getvalue(), r'--field must be specified')
def test_StatusFieldOverrideIssue(self): def test_StatusFieldOverrideIssue(self):
out = StringIO() out = StringIO.StringIO()
self.mock(git_cl.sys, 'stdout', out) self.mock(git_cl.sys, 'stdout', out)
def assertIssue(cl_self, *_args): def assertIssue(cl_self, *_args):
...@@ -2154,7 +2151,7 @@ class TestGitCl(TestCase): ...@@ -2154,7 +2151,7 @@ class TestGitCl(TestCase):
git_cl.main(['set-close', '--issue', '1', '--gerrit']), 0) git_cl.main(['set-close', '--issue', '1', '--gerrit']), 0)
def test_description(self): def test_description(self):
out = StringIO() out = StringIO.StringIO()
self.mock(git_cl.sys, 'stdout', out) self.mock(git_cl.sys, 'stdout', out)
self.calls = [ self.calls = [
((['git', 'symbolic-ref', 'HEAD'],), 'feature'), ((['git', 'symbolic-ref', 'HEAD'],), 'feature'),
...@@ -2178,11 +2175,11 @@ class TestGitCl(TestCase): ...@@ -2178,11 +2175,11 @@ class TestGitCl(TestCase):
self.assertEqual('foobar\n', out.getvalue()) self.assertEqual('foobar\n', out.getvalue())
def test_description_set_raw(self): def test_description_set_raw(self):
out = StringIO() out = StringIO.StringIO()
self.mock(git_cl.sys, 'stdout', out) self.mock(git_cl.sys, 'stdout', out)
self.mock(git_cl, 'Changelist', ChangelistMock) self.mock(git_cl, 'Changelist', ChangelistMock)
self.mock(git_cl.sys, 'stdin', StringIO('hihi')) self.mock(git_cl.sys, 'stdin', StringIO.StringIO('hihi'))
self.assertEqual(0, git_cl.main(['description', '-n', 'hihi'])) self.assertEqual(0, git_cl.main(['description', '-n', 'hihi']))
self.assertEqual('hihi', ChangelistMock.desc) self.assertEqual('hihi', ChangelistMock.desc)
...@@ -2205,7 +2202,7 @@ class TestGitCl(TestCase): ...@@ -2205,7 +2202,7 @@ class TestGitCl(TestCase):
def UpdateDescriptionRemote(_, desc, force=False): def UpdateDescriptionRemote(_, desc, force=False):
self.assertEqual(desc, 'Some.\n\nChange-Id: xxx\nBug: 123') self.assertEqual(desc, 'Some.\n\nChange-Id: xxx\nBug: 123')
self.mock(git_cl.sys, 'stdout', StringIO()) self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
self.mock(git_cl.Changelist, 'GetDescription', self.mock(git_cl.Changelist, 'GetDescription',
lambda *args: current_desc) lambda *args: current_desc)
self.mock(git_cl.Changelist, 'UpdateDescriptionRemote', self.mock(git_cl.Changelist, 'UpdateDescriptionRemote',
...@@ -2235,7 +2232,7 @@ class TestGitCl(TestCase): ...@@ -2235,7 +2232,7 @@ class TestGitCl(TestCase):
desc) desc)
return desc return desc
self.mock(git_cl.sys, 'stdout', StringIO()) self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
self.mock(git_cl.Changelist, 'GetDescription', self.mock(git_cl.Changelist, 'GetDescription',
lambda *args: current_desc) lambda *args: current_desc)
self.mock(git_cl.gclient_utils, 'RunEditor', RunEditor) self.mock(git_cl.gclient_utils, 'RunEditor', RunEditor)
...@@ -2250,17 +2247,17 @@ class TestGitCl(TestCase): ...@@ -2250,17 +2247,17 @@ class TestGitCl(TestCase):
self.assertEqual(0, git_cl.main(['description', '--gerrit'])) self.assertEqual(0, git_cl.main(['description', '--gerrit']))
def test_description_set_stdin(self): def test_description_set_stdin(self):
out = StringIO() out = StringIO.StringIO()
self.mock(git_cl.sys, 'stdout', out) self.mock(git_cl.sys, 'stdout', out)
self.mock(git_cl, 'Changelist', ChangelistMock) self.mock(git_cl, 'Changelist', ChangelistMock)
self.mock(git_cl.sys, 'stdin', StringIO('hi \r\n\t there\n\nman')) self.mock(git_cl.sys, 'stdin', StringIO.StringIO('hi \r\n\t there\n\nman'))
self.assertEqual(0, git_cl.main(['description', '-n', '-'])) self.assertEqual(0, git_cl.main(['description', '-n', '-']))
self.assertEqual('hi\n\t there\n\nman', ChangelistMock.desc) self.assertEqual('hi\n\t there\n\nman', ChangelistMock.desc)
def test_archive(self): def test_archive(self):
self.mock(git_cl.sys, 'stdout', StringIO()) self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
self.calls = [ self.calls = [
((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), ((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],),
...@@ -2279,7 +2276,7 @@ class TestGitCl(TestCase): ...@@ -2279,7 +2276,7 @@ class TestGitCl(TestCase):
self.assertEqual(0, git_cl.main(['archive', '-f'])) self.assertEqual(0, git_cl.main(['archive', '-f']))
def test_archive_current_branch_fails(self): def test_archive_current_branch_fails(self):
self.mock(git_cl.sys, 'stdout', StringIO()) self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
self.calls = [ self.calls = [
((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), ((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],),
'refs/heads/master'), 'refs/heads/master'),
...@@ -2293,7 +2290,7 @@ class TestGitCl(TestCase): ...@@ -2293,7 +2290,7 @@ class TestGitCl(TestCase):
self.assertEqual(1, git_cl.main(['archive', '-f'])) self.assertEqual(1, git_cl.main(['archive', '-f']))
def test_archive_dry_run(self): def test_archive_dry_run(self):
self.mock(git_cl.sys, 'stdout', StringIO()) self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
self.calls = [ self.calls = [
((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), ((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],),
...@@ -2310,7 +2307,7 @@ class TestGitCl(TestCase): ...@@ -2310,7 +2307,7 @@ class TestGitCl(TestCase):
self.assertEqual(0, git_cl.main(['archive', '-f', '--dry-run'])) self.assertEqual(0, git_cl.main(['archive', '-f', '--dry-run']))
def test_archive_no_tags(self): def test_archive_no_tags(self):
self.mock(git_cl.sys, 'stdout', StringIO()) self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
self.calls = [ self.calls = [
((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],), ((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],),
...@@ -2328,7 +2325,7 @@ class TestGitCl(TestCase): ...@@ -2328,7 +2325,7 @@ class TestGitCl(TestCase):
self.assertEqual(0, git_cl.main(['archive', '-f', '--notags'])) self.assertEqual(0, git_cl.main(['archive', '-f', '--notags']))
def test_cmd_issue_erase_existing(self): def test_cmd_issue_erase_existing(self):
out = StringIO() out = StringIO.StringIO()
self.mock(git_cl.sys, 'stdout', out) self.mock(git_cl.sys, 'stdout', out)
self.calls = [ self.calls = [
((['git', 'symbolic-ref', 'HEAD'],), 'feature'), ((['git', 'symbolic-ref', 'HEAD'],), 'feature'),
...@@ -2345,7 +2342,7 @@ class TestGitCl(TestCase): ...@@ -2345,7 +2342,7 @@ class TestGitCl(TestCase):
self.assertEqual(0, git_cl.main(['issue', '0'])) self.assertEqual(0, git_cl.main(['issue', '0']))
def test_cmd_issue_erase_existing_with_change_id(self): def test_cmd_issue_erase_existing_with_change_id(self):
out = StringIO() out = StringIO.StringIO()
self.mock(git_cl.sys, 'stdout', out) self.mock(git_cl.sys, 'stdout', out)
self.mock(git_cl.Changelist, 'GetDescription', self.mock(git_cl.Changelist, 'GetDescription',
lambda _: 'This is a description\n\nChange-Id: Ideadbeef') lambda _: 'This is a description\n\nChange-Id: Ideadbeef')
...@@ -2366,7 +2363,7 @@ class TestGitCl(TestCase): ...@@ -2366,7 +2363,7 @@ class TestGitCl(TestCase):
self.assertEqual(0, git_cl.main(['issue', '0'])) self.assertEqual(0, git_cl.main(['issue', '0']))
def test_cmd_issue_json(self): def test_cmd_issue_json(self):
out = StringIO() out = StringIO.StringIO()
self.mock(git_cl.sys, 'stdout', out) self.mock(git_cl.sys, 'stdout', out)
self.calls = [ self.calls = [
((['git', 'symbolic-ref', 'HEAD'],), 'feature'), ((['git', 'symbolic-ref', 'HEAD'],), 'feature'),
...@@ -2381,7 +2378,7 @@ class TestGitCl(TestCase): ...@@ -2381,7 +2378,7 @@ class TestGitCl(TestCase):
self.assertEqual(0, git_cl.main(['issue', '--json', 'output.json'])) self.assertEqual(0, git_cl.main(['issue', '--json', 'output.json']))
def _common_GerritCommitMsgHookCheck(self): def _common_GerritCommitMsgHookCheck(self):
self.mock(git_cl.sys, 'stdout', StringIO()) self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
self.mock(git_cl.os.path, 'abspath', self.mock(git_cl.os.path, 'abspath',
lambda path: self._mocked_call(['abspath', path])) lambda path: self._mocked_call(['abspath', path]))
self.mock(git_cl.os.path, 'exists', self.mock(git_cl.os.path, 'exists',
...@@ -2444,7 +2441,7 @@ class TestGitCl(TestCase): ...@@ -2444,7 +2441,7 @@ class TestGitCl(TestCase):
'url': 'https://git.googlesource.com/test/+/deadbeef'}], 'url': 'https://git.googlesource.com/test/+/deadbeef'}],
} }
cl.SubmitIssue = lambda wait_for_merge: None cl.SubmitIssue = lambda wait_for_merge: None
out = StringIO() out = StringIO.StringIO()
self.mock(sys, 'stdout', out) self.mock(sys, 'stdout', out)
self.assertEqual(0, cl.CMDLand(force=True, self.assertEqual(0, cl.CMDLand(force=True,
bypass_hooks=True, bypass_hooks=True,
...@@ -2540,7 +2537,7 @@ class TestGitCl(TestCase): ...@@ -2540,7 +2537,7 @@ class TestGitCl(TestCase):
} }
self.mock(git_cl.gerrit_util, 'CookiesAuthenticator', self.mock(git_cl.gerrit_util, 'CookiesAuthenticator',
CookiesAuthenticatorMock) CookiesAuthenticatorMock)
self.mock(sys, 'stdout', StringIO()) self.mock(sys, 'stdout', StringIO.StringIO())
git_cl._GitCookiesChecker().print_current_creds(include_netrc=True) git_cl._GitCookiesChecker().print_current_creds(include_netrc=True)
self.assertEqual(list(sys.stdout.getvalue().splitlines()), [ self.assertEqual(list(sys.stdout.getvalue().splitlines()), [
' Host\t User\t Which file', ' Host\t User\t Which file',
...@@ -2549,8 +2546,7 @@ class TestGitCl(TestCase): ...@@ -2549,8 +2546,7 @@ class TestGitCl(TestCase):
' host.googlesource.com\t user\t.gitcookies', ' host.googlesource.com\t user\t.gitcookies',
' host2.googlesource.com\tuser3\t .netrc', ' host2.googlesource.com\tuser3\t .netrc',
]) ])
sys.stdout.seek(0) sys.stdout.buf = ''
sys.stdout.truncate(0)
git_cl._GitCookiesChecker().print_current_creds(include_netrc=False) git_cl._GitCookiesChecker().print_current_creds(include_netrc=False)
self.assertEqual(list(sys.stdout.getvalue().splitlines()), [ self.assertEqual(list(sys.stdout.getvalue().splitlines()), [
' Host\tUser\t Which file', ' Host\tUser\t Which file',
...@@ -2570,7 +2566,7 @@ class TestGitCl(TestCase): ...@@ -2570,7 +2566,7 @@ class TestGitCl(TestCase):
# git cl also checks for existence other files not relevant to this test. # git cl also checks for existence other files not relevant to this test.
return None return None
self.mock(os.path, 'exists', exists_mock) self.mock(os.path, 'exists', exists_mock)
self.mock(sys, 'stdout', StringIO()) self.mock(sys, 'stdout', StringIO.StringIO())
def test_creds_check_gitcookies_not_configured(self): def test_creds_check_gitcookies_not_configured(self):
self._common_creds_check_mocks() self._common_creds_check_mocks()
...@@ -2635,7 +2631,7 @@ class TestGitCl(TestCase): ...@@ -2635,7 +2631,7 @@ class TestGitCl(TestCase):
'-a', 'msg'])) '-a', 'msg']))
def test_git_cl_comments_fetch_gerrit(self): def test_git_cl_comments_fetch_gerrit(self):
self.mock(sys, 'stdout', StringIO()) self.mock(sys, 'stdout', StringIO.StringIO())
self.calls = [ self.calls = [
((['git', 'config', 'branch.foo.gerritserver'],), ''), ((['git', 'config', 'branch.foo.gerritserver'],), ''),
((['git', 'config', 'branch.foo.merge'],), ''), ((['git', 'config', 'branch.foo.merge'],), ''),
...@@ -2786,7 +2782,7 @@ class TestGitCl(TestCase): ...@@ -2786,7 +2782,7 @@ class TestGitCl(TestCase):
# git cl comments also fetches robot comments (which are considered a type # git cl comments also fetches robot comments (which are considered a type
# of autogenerated comment), and unlike other types of comments, only robot # of autogenerated comment), and unlike other types of comments, only robot
# comments from the latest patchset are shown. # comments from the latest patchset are shown.
self.mock(sys, 'stdout', StringIO()) self.mock(sys, 'stdout', StringIO.StringIO())
self.calls = [ self.calls = [
((['git', 'config', 'branch.foo.gerritserver'],), ''), ((['git', 'config', 'branch.foo.gerritserver'],), ''),
((['git', 'config', 'branch.foo.merge'],), ''), ((['git', 'config', 'branch.foo.merge'],), ''),
...@@ -3041,7 +3037,7 @@ class CMDTestCaseBase(unittest.TestCase): ...@@ -3041,7 +3037,7 @@ class CMDTestCaseBase(unittest.TestCase):
def setUp(self): def setUp(self):
super(CMDTestCaseBase, self).setUp() super(CMDTestCaseBase, self).setUp()
mock.patch('git_cl.sys.stdout', StringIO()).start() mock.patch('git_cl.sys.stdout', StringIO.StringIO()).start()
mock.patch('git_cl.uuid.uuid4', return_value='uuid4').start() mock.patch('git_cl.uuid.uuid4', return_value='uuid4').start()
mock.patch('git_cl.Changelist.GetIssue', return_value=123456).start() mock.patch('git_cl.Changelist.GetIssue', return_value=123456).start()
mock.patch('git_cl.Changelist.GetCodereviewServer', mock.patch('git_cl.Changelist.GetCodereviewServer',
......
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