diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index a1144b864..e421bd750 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -448,11 +448,6 @@ def RunUnitTests(input_api, output_api, unit_tests, verbose=False): else: message_type = output_api.PresubmitPromptWarning - if verbose: - pipe = None - else: - pipe = input_api.subprocess.PIPE - results = [] for unit_test in unit_tests: cmd = [] @@ -463,15 +458,13 @@ def RunUnitTests(input_api, output_api, unit_tests, verbose=False): if verbose: print('Running %s' % unit_test) try: - proc = input_api.subprocess.Popen( - cmd, cwd=input_api.PresubmitLocalPath(), stdout=pipe, stderr=pipe) - out = '\n'.join(filter(None, proc.communicate())) - if proc.returncode: - results.append(message_type( - '%s failed with return code %d\n%s' % ( - unit_test, proc.returncode, out))) - except (OSError, input_api.subprocess.CalledProcessError): - results.append(message_type('%s failed' % unit_test)) + if verbose: + input_api.subprocess.check_call(cmd, cwd=input_api.PresubmitLocalPath()) + else: + input_api.subprocess.check_output( + cmd, cwd=input_api.PresubmitLocalPath()) + except (OSError, input_api.subprocess.CalledProcessError), e: + results.append(message_type('%s failed!\n%s' % (unit_test, e))) return results @@ -486,7 +479,7 @@ def RunPythonUnitTests(input_api, output_api, unit_tests): message_type = output_api.PresubmitError else: message_type = output_api.PresubmitNotifyResult - outputs = [] + results = [] for unit_test in unit_tests: # Run the unit tests out of process. This is because some unit tests # stub out base libraries and don't clean up their mess. It's too easy to @@ -510,26 +503,12 @@ def RunPythonUnitTests(input_api, output_api, unit_tests): if env.get('PYTHONPATH'): backpath.append(env.get('PYTHONPATH')) env['PYTHONPATH'] = input_api.os_path.pathsep.join((backpath)) - subproc = input_api.subprocess.Popen( - [ - input_api.python_executable, - '-m', - '%s' % unit_test - ], - cwd=cwd, - env=env, - stdin=input_api.subprocess.PIPE, - stdout=input_api.subprocess.PIPE, - stderr=input_api.subprocess.PIPE) - stdoutdata, stderrdata = subproc.communicate() - # Discard the output if returncode == 0 - if subproc.returncode: - outputs.append('Test \'%s\' failed with code %d\n%s\n%s\n' % ( - unit_test_name, subproc.returncode, stdoutdata, stderrdata)) - if outputs: - return [message_type('%d unit tests failed.' % len(outputs), - long_text='\n'.join(outputs))] - return [] + cmd = [input_api.python_executable, '-m', '%s' % unit_test] + try: + input_api.subprocess.check_output(cmd, cwd=cwd, env=env) + except (OSError, input_api.subprocess.CalledProcessError), e: + results.append(message_type('%s failed!\n%s' % (unit_test_name, e))) + return results def _FetchAllFiles(input_api, white_list, black_list): diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 65984ec9c..8b6a68c9e 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -6,7 +6,7 @@ """Unit tests for presubmit_support.py and presubmit_canned_checks.py.""" # pylint is too confused. -# pylint: disable=E1101,E1103,W0212,W0403 +# pylint: disable=E1101,E1103,R0201,W0212,W0403 import StringIO import sys @@ -1246,6 +1246,10 @@ class CannedChecksUnittest(PresubmitTestsBase): input_api.urllib2 = self.mox.CreateMock(presubmit.urllib2) input_api.unittest = unittest input_api.subprocess = self.mox.CreateMock(presubmit.subprocess) + class fake_CalledProcessError(Exception): + def __str__(self): + return 'foo' + input_api.subprocess.CalledProcessError = fake_CalledProcessError input_api.change = change input_api.host_url = 'http://localhost' @@ -1731,14 +1735,10 @@ class CannedChecksUnittest(PresubmitTestsBase): def testRunPythonUnitTestsNonExistentUpload(self): input_api = self.MockInputApi(None, False) - process = self.mox.CreateMockAnything() - process.returncode = 2 - input_api.subprocess.Popen( - ['pyyyyython', '-m', '_non_existent_module'], cwd=None, env=None, - stderr=presubmit.subprocess.PIPE, stdin=presubmit.subprocess.PIPE, - stdout=presubmit.subprocess.PIPE).AndReturn(process) - process.communicate().AndReturn( - ('', 'pyyython: module _non_existent_module not found')) + input_api.subprocess.check_output( + ['pyyyyython', '-m', '_non_existent_module'], cwd=None, env=None + ).AndRaise( + input_api.subprocess.CalledProcessError()) self.mox.ReplayAll() results = presubmit_canned_checks.RunPythonUnitTests( @@ -1749,15 +1749,12 @@ class CannedChecksUnittest(PresubmitTestsBase): def testRunPythonUnitTestsNonExistentCommitting(self): input_api = self.MockInputApi(None, True) - process = self.mox.CreateMockAnything() - process.returncode = 2 - input_api.subprocess.Popen( - ['pyyyyython', '-m', '_non_existent_module'], cwd=None, env=None, - stderr=presubmit.subprocess.PIPE, stdin=presubmit.subprocess.PIPE, - stdout=presubmit.subprocess.PIPE).AndReturn(process) - process.communicate().AndReturn( - ('', 'pyyython: module _non_existent_module not found')) + input_api.subprocess.check_output( + ['pyyyyython', '-m', '_non_existent_module'], cwd=None, env=None + ).AndRaise( + input_api.subprocess.CalledProcessError()) self.mox.ReplayAll() + results = presubmit_canned_checks.RunPythonUnitTests( input_api, presubmit.OutputApi, ['_non_existent_module']) self.assertEquals(len(results), 1) @@ -1767,13 +1764,9 @@ class CannedChecksUnittest(PresubmitTestsBase): input_api = self.MockInputApi(None, False) input_api.unittest = self.mox.CreateMock(unittest) input_api.cStringIO = self.mox.CreateMock(presubmit.cStringIO) - process = self.mox.CreateMockAnything() - process.returncode = -1 - input_api.subprocess.Popen( - ['pyyyyython', '-m', 'test_module'], cwd=None, env=None, - stderr=presubmit.subprocess.PIPE, stdin=presubmit.subprocess.PIPE, - stdout=presubmit.subprocess.PIPE).AndReturn(process) - process.communicate().AndReturn(('BOO HOO!', '')) + input_api.subprocess.check_output( + ['pyyyyython', '-m', 'test_module'], cwd=None, env=None).AndRaise( + input_api.subprocess.CalledProcessError()) self.mox.ReplayAll() results = presubmit_canned_checks.RunPythonUnitTests( @@ -1781,38 +1774,27 @@ class CannedChecksUnittest(PresubmitTestsBase): self.assertEquals(len(results), 1) self.assertEquals(results[0].__class__, presubmit.OutputApi.PresubmitNotifyResult) - self.assertEquals(results[0]._long_text, - "Test 'test_module' failed with code -1\nBOO HOO!") + self.assertEquals('test_module failed!\nfoo', results[0]._message) def testRunPythonUnitTestsFailureCommitting(self): input_api = self.MockInputApi(None, True) - process = self.mox.CreateMockAnything() - process.returncode = 1 - input_api.subprocess.Popen( - ['pyyyyython', '-m', 'test_module'], cwd=None, env=None, - stderr=presubmit.subprocess.PIPE, stdin=presubmit.subprocess.PIPE, - stdout=presubmit.subprocess.PIPE).AndReturn(process) - process.communicate().AndReturn(('BOO HOO!', '')) + input_api.subprocess.check_output( + ['pyyyyython', '-m', 'test_module'], cwd=None, env=None).AndRaise( + input_api.subprocess.CalledProcessError()) self.mox.ReplayAll() results = presubmit_canned_checks.RunPythonUnitTests( input_api, presubmit.OutputApi, ['test_module']) self.assertEquals(len(results), 1) self.assertEquals(results[0].__class__, presubmit.OutputApi.PresubmitError) - self.assertEquals(results[0]._long_text, - "Test 'test_module' failed with code 1\nBOO HOO!") + self.assertEquals('test_module failed!\nfoo', results[0]._message) def testRunPythonUnitTestsSuccess(self): input_api = self.MockInputApi(None, False) input_api.cStringIO = self.mox.CreateMock(presubmit.cStringIO) input_api.unittest = self.mox.CreateMock(unittest) - process = self.mox.CreateMockAnything() - process.returncode = 0 - input_api.subprocess.Popen( - ['pyyyyython', '-m', 'test_module'], cwd=None, env=None, - stderr=presubmit.subprocess.PIPE, stdin=presubmit.subprocess.PIPE, - stdout=presubmit.subprocess.PIPE).AndReturn(process) - process.communicate().AndReturn(('', '')) + input_api.subprocess.check_output( + ['pyyyyython', '-m', 'test_module'], cwd=None, env=None) self.mox.ReplayAll() results = presubmit_canned_checks.RunPythonUnitTests( @@ -2022,16 +2004,11 @@ mac|success|blew unit_tests = ['allo', 'bar.py'] input_api.PresubmitLocalPath().AndReturn(self.fake_root_dir) input_api.PresubmitLocalPath().AndReturn(self.fake_root_dir) - proc1 = self.mox.CreateMockAnything() - input_api.subprocess.Popen(['allo'], cwd=self.fake_root_dir, - stderr=None, stdout=None).AndReturn(proc1) - proc1.returncode = 0 - proc1.communicate().AndReturn(['baz', None]) - proc2 = self.mox.CreateMockAnything() - input_api.subprocess.Popen(['bar.py'], cwd=self.fake_root_dir, - stderr=None, stdout=None).AndReturn(proc2) - proc2.returncode = 1 - proc2.communicate().AndReturn(['bouz', None]) + input_api.subprocess.check_call( + ['allo'], cwd=self.fake_root_dir) + input_api.subprocess.check_call( + ['bar.py'], cwd=self.fake_root_dir).AndRaise( + input_api.subprocess.CalledProcessError()) self.mox.ReplayAll() results = presubmit_canned_checks.RunUnitTests( @@ -2053,11 +2030,8 @@ mac|success|blew path = presubmit.os.path.join(self.fake_root_dir, 'random_directory') input_api.os_listdir(path).AndReturn(['.', '..', 'a', 'b', 'c']) input_api.os_path.isfile = lambda x: not x.endswith('.') - proc1 = self.mox.CreateMockAnything() - input_api.subprocess.Popen(['random_directory/b'], cwd=self.fake_root_dir, - stderr=None, stdout=None).AndReturn(proc1) - proc1.returncode = 0 - proc1.communicate().AndReturn(['baz', None]) + input_api.subprocess.check_call( + ['random_directory/b'], cwd=self.fake_root_dir) self.mox.ReplayAll() results = presubmit_canned_checks.RunUnitTestsInDirectory(