diff --git a/git_cl.py b/git_cl.py index feabbad2f..fcb0d5436 100755 --- a/git_cl.py +++ b/git_cl.py @@ -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): diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 87dcb3b55..aa56a3a7e 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -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)