aiden
/
depot_tools
mirror of https://chromium.googlesource.com/chromium/tools/depot_tools
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

git-cl: Fix Python 3 bugs

Browse Source 
- Don't encode description when calling presubmit,
  codecs.open already takes care of that.
- Don't use raw_input on Python 3
- Encode stdin when calling clang-format.

Bug: 1058318, 1058318
Change-Id: I825422e160c00b33b2c52b6e64e5f0a3e44606f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2086631
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
changes/31/2086631/7
Edward Lemur 5 years ago committed by LUCI CQ
parent ec2a6ce270
commit 1a83da1f41
2 changed files with 54 additions and 33 deletions
Show Stats Download Patch File Download Diff File

25
git_cl.py
Unescape Escape View File

@ -228,9 +228,16 @@ def datetime_now():
return datetime.datetime.now()
def _raw_input(message):
# Use this so that it can be mocked in tests on Python 2 and 3.
if sys.version_info.major == 2:
return raw_input(message)
return input(message)
def ask_for_data(prompt):
try:
return raw_input(prompt)
return _raw_input(prompt)
except KeyboardInterrupt:
# Hide the exception.
sys.exit(1)
@ -1433,8 +1440,8 @@ class Changelist(object):
with gclient_utils.temporary_file() as description_file:
with gclient_utils.temporary_file() as json_output:
gclient_utils.FileWrite(
description_file, description.encode('utf-8'), mode='wb')
gclient_utils.FileWrite(description_file, description)
args.extend(['--json_output', json_output])
args.extend(['--description_file', description_file])
@ -1458,8 +1465,7 @@ class Changelist(object):
args.append('--post_upload')
with gclient_utils.temporary_file() as description_file:
gclient_utils.FileWrite(
description_file, description.encode('utf-8'), mode='wb')
gclient_utils.FileWrite(description_file, description)
args.extend(['--description_file', description_file])
p = subprocess2.Popen(['vpython', PRESUBMIT_SUPPORT] + args)
p.wait()
@ -1490,7 +1496,10 @@ class Changelist(object):
if not options.bypass_watchlists:
self.ExtendCC(watchlist.GetWatchersForPaths(files))
description = change.FullDescriptionText()
if self.GetIssue():
description = self.FetchDescription()
else:
description = self.GetLocalDescription(base_branch)
if options.reviewers or options.tbrs or options.add_owners_to:
# Set the reviewer list now so that presubmit checks can access it.
change_description = ChangeDescription(description)
@ -5041,7 +5050,7 @@ def _RunClangFormatDiff(opts, clang_diff_files, top_dir, upstream_commit):
cmd.append('-i')
diff_cmd = BuildGitDiffCmd('-U0', upstream_commit, clang_diff_files)
diff_output = RunGit(diff_cmd)
diff_output = RunGit(diff_cmd).encode('utf-8')
stdout = RunCommand(cmd, stdin=diff_output, cwd=top_dir, env=env)
if opts.diff:
@ -5327,7 +5336,7 @@ def CMDcheckout(parser, args):
print('Multiple branches match issue %s:' % target_issue)
for i in range(len(branches)):
print('%d: %s' % (i, branches[i]))
which = raw_input('Choose by index: ')
which = ask_for_data('Choose by index: ')
try:
RunGit(['checkout', branches[int(which)]])
except (IndexError, ValueError):

62
tests/git_cl_test.py
Unescape Escape View File

@ -48,13 +48,6 @@ import subprocess2
def callError(code=1, cmd='', cwd='', stdout=b'', stderr=b''):
return subprocess2.CalledProcessError(code, cmd, cwd, stdout, stderr)
def _constantFn(return_value):
def f(*args, **kwargs):
return return_value
return f
CERR1 = callError(1)
@ -662,9 +655,6 @@ class TestGitCl(unittest.TestCase):
mock.patch(
'git_cl.SaveDescriptionBackup',
lambda _: self._mocked_call('SaveDescriptionBackup')).start()
mock.patch(
'git_cl.ask_for_data',
lambda prompt: self._mocked_call('ask_for_data', prompt)).start()
mock.patch(
'git_cl.write_json',
lambda *a: self._mocked_call('write_json', *a)).start()
@ -769,12 +759,13 @@ class TestGitCl(unittest.TestCase):
result = result.encode('utf-8')
return result
@mock.patch('sys.stdin', StringIO('blah\nye\n'))
@mock.patch('sys.stdout', StringIO())
def test_ask_for_explicit_yes_true(self):
self.calls = [
(('ask_for_data', 'prompt [Yes/No]: '), 'blah'),
(('ask_for_data', 'Please, type yes or no: '), 'ye'),
]
self.assertTrue(git_cl.ask_for_explicit_yes('prompt'))
self.assertEqual(
'prompt [Yes/No]: Please, type yes or no: ',
sys.stdout.getvalue())
def test_LoadCodereviewSettingsFromFile_gerrit(self):
codereview_file = StringIO('GERRIT_HOST: true')
@ -1193,6 +1184,9 @@ class TestGitCl(unittest.TestCase):
mock.patch('git_cl.Changelist.GitSanityChecks', return_value=True).start()
mock.patch(
'git_cl.Changelist.GetLocalDescription', return_value='foo').start()
mock.patch(
'git_cl.ask_for_data',
lambda prompt: self._mocked_call('ask_for_data', prompt)).start()
self.mockGit.config['gerrit.host'] = 'true'
self.mockGit.config['branch.master.gerritissue'] = (
@ -1473,7 +1467,8 @@ class TestGitCl(unittest.TestCase):
@mock.patch('git_cl.RunGit')
@mock.patch('git_cl.CMDupload')
@mock.patch('git_cl.ask_for_data')
@mock.patch('sys.stdin', StringIO('\n'))
@mock.patch('sys.stdout', StringIO())
def test_upload_branch_deps(self, *_mocks):
def mock_run_git(*args, **_kwargs):
if args[0] == ['for-each-ref',
@ -1511,10 +1506,11 @@ class TestGitCl(unittest.TestCase):
ret = git_cl.upload_branch_deps(MockChangelist(), [])
# CMDupload should have been called 5 times because of 5 dependent branches.
self.assertEqual(5, len(git_cl.CMDupload.mock_calls))
git_cl.ask_for_data.assert_called_once_with(
self.assertIn(
'This command will checkout all dependent branches '
'and run "git cl upload". Press Enter to continue, '
'or Ctrl+C to abort')
'or Ctrl+C to abort',
sys.stdout.getvalue())
self.assertEqual(0, ret)
def test_gerrit_change_id(self):
@ -1843,6 +1839,9 @@ class TestGitCl(unittest.TestCase):
self.assertEqual(1, git_cl.main(['checkout', '99999']))
def _test_gerrit_ensure_authenticated_common(self, auth):
mock.patch(
'git_cl.ask_for_data',
lambda prompt: self._mocked_call('ask_for_data', prompt)).start()
mock.patch('git_cl.gerrit_util.CookiesAuthenticator',
CookiesAuthenticatorMockFactory(hosts_with_creds=auth)).start()
self.mockGit.config['remote.origin.url'] = (
@ -2236,6 +2235,9 @@ class TestGitCl(unittest.TestCase):
mock.patch(
'git_cl.gclient_utils.rm_file_or_tree',
lambda path: self._mocked_call(['rm_file_or_tree', path])).start()
mock.patch(
'git_cl.ask_for_data',
lambda prompt: self._mocked_call('ask_for_data', prompt)).start()
return git_cl.Changelist(issue=123)
def test_GerritCommitMsgHookCheck_custom_hook(self):
@ -2385,6 +2387,9 @@ class TestGitCl(unittest.TestCase):
return self._mocked_call('os.path.exists', '%s/%s' % (dirname, base))
# git cl also checks for existence other files not relevant to this test.
return None
mock.patch(
'git_cl.ask_for_data',
lambda prompt: self._mocked_call('ask_for_data', prompt)).start()
mock.patch('os.path.exists', exists_mock).start()
def test_creds_check_gitcookies_not_configured(self):
@ -2852,7 +2857,7 @@ class ChangelistTest(unittest.TestCase):
'--description_file', '/tmp/fake-temp1',
])
gclient_utils.FileWrite.assert_called_once_with(
'/tmp/fake-temp1', b'description', mode='wb')
'/tmp/fake-temp1', 'description')
metrics.collector.add_repeated('sub_commands', {
'command': 'presubmit',
'execution_time': 100,
@ -2896,7 +2901,7 @@ class ChangelistTest(unittest.TestCase):
'--description_file', '/tmp/fake-temp1',
])
gclient_utils.FileWrite.assert_called_once_with(
'/tmp/fake-temp1', b'description', mode='wb')
'/tmp/fake-temp1', 'description')
class CMDTestCaseBase(unittest.TestCase):
@ -3463,6 +3468,13 @@ class CMDFormatTestCase(unittest.TestCase):
self.assertEqual(expected, git_cl._FilterYapfIgnoredFiles(
files, git_cl._GetYapfIgnorePatterns(self._top_dir)))
def _run_command_mock(self, return_value):
def f(*args, **kwargs):
if 'stdin' in kwargs:
self.assertIsInstance(kwargs['stdin'], bytes)
return return_value
return f
def testClangFormatDiffFull(self):
self._make_temp_file('test.cc', ['// test'])
git_cl.settings.GetFormatFullByDefault.return_value = False
@ -3470,13 +3482,13 @@ class CMDFormatTestCase(unittest.TestCase):
mock_opts = mock.Mock(full=True, dry_run=True, diff=False)
# Diff
git_cl.RunCommand.return_value = ' // test'
git_cl.RunCommand.side_effect = self._run_command_mock(' // test')
return_value = git_cl._RunClangFormatDiff(mock_opts, diff_file,
self._top_dir, 'HEAD')
self.assertEqual(2, return_value)
# No diff
git_cl.RunCommand.return_value = '// test'
git_cl.RunCommand.side_effect = self._run_command_mock('// test')
return_value = git_cl._RunClangFormatDiff(mock_opts, diff_file,
self._top_dir, 'HEAD')
self.assertEqual(0, return_value)
@ -3486,13 +3498,13 @@ class CMDFormatTestCase(unittest.TestCase):
mock_opts = mock.Mock(full=False, dry_run=True, diff=False)
# Diff
git_cl.RunCommand.return_value = 'error'
return_value = git_cl._RunClangFormatDiff(mock_opts, ['.'], self._top_dir,
'HEAD')
git_cl.RunCommand.side_effect = self._run_command_mock('error')
return_value = git_cl._RunClangFormatDiff(
mock_opts, ['.'], self._top_dir, 'HEAD')
self.assertEqual(2, return_value)
# No diff
git_cl.RunCommand.return_value = ''
git_cl.RunCommand.side_effect = self._run_command_mock('')
return_value = git_cl._RunClangFormatDiff(mock_opts, ['.'], self._top_dir,
'HEAD')
self.assertEqual(0, return_value)

Write Preview
Loading…
Cancel
Save