diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 24ae89f57..3aaecf86b 100755 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -256,35 +256,53 @@ def CheckTreeIsOpen(input_api, output_api, url, closed): return [] -def _RunPythonUnitTests_LoadTests(input_api, module_name): - """Meant to be stubbed out during unit testing.""" - module = __import__(module_name) - for part in module_name.split('.')[1:]: - module = getattr(module, part) - return input_api.unittest.TestLoader().loadTestsFromModule(module)._tests - - def RunPythonUnitTests(input_api, output_api, unit_tests): - """Imports the unit_tests modules and run them.""" + """Run the unit tests out of process, capture the output and use the result + code to determine success. + """ # We don't want to hinder users from uploading incomplete patches. if input_api.is_committing: message_type = output_api.PresubmitError else: message_type = output_api.PresubmitNotifyResult - tests_suite = [] outputs = [] for unit_test in unit_tests: - try: - tests_suite.extend(_RunPythonUnitTests_LoadTests(input_api, unit_test)) - except ImportError: - outputs.append(message_type("Failed to load %s" % unit_test, - long_text=input_api.traceback.format_exc())) - - buffer = input_api.cStringIO.StringIO() - results = input_api.unittest.TextTestRunner(stream=buffer, verbosity=0).run( - input_api.unittest.TestSuite(tests_suite)) - if not results.wasSuccessful(): - outputs.append(message_type("%d unit tests failed." % - (len(results.failures) + len(results.errors)), - long_text=buffer.getvalue())) - return outputs + # 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 + # get subtle bugs. + cwd = None + env = None + unit_test_name = unit_test + # "python -m test.unit_test" doesn't work. We need to change to the right + # directory instead. + if '.' in unit_test: + # Tests imported in submodules (subdirectories) assume that the current + # directory is in the PYTHONPATH. Manually fix that. + unit_test = unit_test.replace('.', '/') + cwd = input_api.os_path.dirname(unit_test) + unit_test = input_api.os_path.basename(unit_test) + env = input_api.environ.copy() + backpath = [';'.join(['..'] * (cwd.count('/') + 1))] + if env.get('PYTHONPATH'): + backpath.append(env.get('PYTHONPATH')) + env['PYTHONPATH'] = ';'.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 [] diff --git a/presubmit_support.py b/presubmit_support.py index 88a535fde..8586a4871 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -197,6 +197,10 @@ class InputApi(object): self.unittest = unittest self.urllib2 = urllib2 + # To easily fork python. + self.python_executable = sys.executable + self.environ = os.environ + # InputApi.platform is the platform you're currently running on. self.platform = sys.platform diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 9a6ffc199..bf9237cac 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -494,8 +494,9 @@ class InputApiUnittest(PresubmitTestsBase): 'DepotToLocalPath', 'FilterSourceFile', 'LocalPaths', 'LocalToDepotPath', 'PresubmitLocalPath', 'ReadFile', 'RightHandSideLines', 'ServerPaths', - 'basename', 'cPickle', 'cStringIO', 'canned_checks', 'change', + 'basename', 'cPickle', 'cStringIO', 'canned_checks', 'change', 'environ', 'is_committing', 'marshal', 'os_path', 'pickle', 'platform', + 'python_executable', 're', 'subprocess', 'tempfile', 'traceback', 'unittest', 'urllib2', 'version', ] @@ -975,8 +976,6 @@ class CannedChecksUnittest(PresubmitTestsBase): def setUp(self): PresubmitTestsBase.setUp(self) - self.mox.StubOutWithMock(presubmit_canned_checks, - '_RunPythonUnitTests_LoadTests') def MockInputApi(self, change, committing): input_api = self.mox.CreateMock(presubmit.InputApi) @@ -985,8 +984,11 @@ class CannedChecksUnittest(PresubmitTestsBase): input_api.traceback = presubmit.traceback input_api.urllib2 = self.mox.CreateMock(presubmit.urllib2) input_api.unittest = unittest + input_api.subprocess = self.mox.CreateMock(presubmit.subprocess) + input_api.change = change input_api.is_committing = committing + input_api.python_executable = 'pyyyyython' return input_api def testMembersChanged(self): @@ -1271,9 +1273,14 @@ class CannedChecksUnittest(PresubmitTestsBase): def testRunPythonUnitTestsNonExistentUpload(self): input_api = self.MockInputApi(None, False) - presubmit_canned_checks._RunPythonUnitTests_LoadTests( - input_api, '_non_existent_module').AndRaise( - exceptions.ImportError('Blehh')) + 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')) self.mox.ReplayAll() results = presubmit_canned_checks.RunPythonUnitTests( @@ -1284,57 +1291,31 @@ class CannedChecksUnittest(PresubmitTestsBase): def testRunPythonUnitTestsNonExistentCommitting(self): input_api = self.MockInputApi(None, True) - presubmit_canned_checks._RunPythonUnitTests_LoadTests( - input_api, '_non_existent_module').AndRaise( - exceptions.ImportError('Blehh')) + 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')) self.mox.ReplayAll() results = presubmit_canned_checks.RunPythonUnitTests( input_api, presubmit.OutputApi, ['_non_existent_module']) self.assertEquals(len(results), 1) self.assertEquals(results[0].__class__, presubmit.OutputApi.PresubmitError) - def testRunPythonUnitTestsEmptyUpload(self): - input_api = self.MockInputApi(None, False) - test_module = self.mox.CreateMockAnything() - presubmit_canned_checks._RunPythonUnitTests_LoadTests( - input_api, 'test_module').AndReturn([]) - self.mox.ReplayAll() - - results = presubmit_canned_checks.RunPythonUnitTests( - input_api, presubmit.OutputApi, ['test_module']) - self.assertEquals(results, []) - - def testRunPythonUnitTestsEmptyCommitting(self): - input_api = self.MockInputApi(None, True) - test_module = self.mox.CreateMockAnything() - presubmit_canned_checks._RunPythonUnitTests_LoadTests( - input_api, 'test_module').AndReturn([]) - self.mox.ReplayAll() - - results = presubmit_canned_checks.RunPythonUnitTests( - input_api, presubmit.OutputApi, ['test_module']) - self.assertEquals(results, []) - def testRunPythonUnitTestsFailureUpload(self): input_api = self.MockInputApi(None, False) input_api.unittest = self.mox.CreateMock(unittest) input_api.cStringIO = self.mox.CreateMock(presubmit.cStringIO) - test = self.mox.CreateMockAnything() - presubmit_canned_checks._RunPythonUnitTests_LoadTests( - input_api, 'test_module').AndReturn([test]) - runner = self.mox.CreateMockAnything() - buffer = self.mox.CreateMockAnything() - input_api.cStringIO.StringIO().AndReturn(buffer) - buffer.getvalue().AndReturn('BOO HOO!') - input_api.unittest.TextTestRunner(stream=buffer, verbosity=0 - ).AndReturn(runner) - suite = self.mox.CreateMockAnything() - input_api.unittest.TestSuite([test]).AndReturn(suite) - test_result = self.mox.CreateMockAnything() - runner.run(suite).AndReturn(test_result) - test_result.wasSuccessful().AndReturn(False) - test_result.failures = [None, None] - test_result.errors = [None, None, None] + 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!', '')) self.mox.ReplayAll() results = presubmit_canned_checks.RunPythonUnitTests( @@ -1342,55 +1323,38 @@ class CannedChecksUnittest(PresubmitTestsBase): self.assertEquals(len(results), 1) self.assertEquals(results[0].__class__, presubmit.OutputApi.PresubmitNotifyResult) - self.assertEquals(results[0]._long_text, 'BOO HOO!') + self.assertEquals(results[0]._long_text, + "Test 'test_module' failed with code -1\nBOO HOO!") def testRunPythonUnitTestsFailureCommitting(self): input_api = self.MockInputApi(None, True) - input_api.unittest = self.mox.CreateMock(unittest) - input_api.cStringIO = self.mox.CreateMock(presubmit.cStringIO) - test = self.mox.CreateMockAnything() - presubmit_canned_checks._RunPythonUnitTests_LoadTests( - input_api, 'test_module').AndReturn([test]) - runner = self.mox.CreateMockAnything() - buffer = self.mox.CreateMockAnything() - input_api.cStringIO.StringIO().AndReturn(buffer) - buffer.getvalue().AndReturn('BOO HOO!') - input_api.unittest.TextTestRunner(stream=buffer, verbosity=0 - ).AndReturn(runner) - suite = self.mox.CreateMockAnything() - input_api.unittest.TestSuite([test]).AndReturn(suite) - test_result = self.mox.CreateMockAnything() - runner.run(suite).AndReturn(test_result) - test_result.wasSuccessful().AndReturn(False) - test_result.failures = [None, None] - test_result.errors = [None, None, None] + 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!', '')) 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, 'BOO HOO!') + self.assertEquals(results[0]._long_text, + "Test 'test_module' failed with code 1\nBOO HOO!") def testRunPythonUnitTestsSuccess(self): input_api = self.MockInputApi(None, False) input_api.cStringIO = self.mox.CreateMock(presubmit.cStringIO) input_api.unittest = self.mox.CreateMock(unittest) - test = self.mox.CreateMockAnything() - presubmit_canned_checks._RunPythonUnitTests_LoadTests( - input_api, 'test_module').AndReturn([test]) - runner = self.mox.CreateMockAnything() - buffer = self.mox.CreateMockAnything() - input_api.cStringIO.StringIO().AndReturn(buffer) - input_api.unittest.TextTestRunner(stream=buffer, verbosity=0 - ).AndReturn(runner) - suite = self.mox.CreateMockAnything() - input_api.unittest.TestSuite([test]).AndReturn(suite) - test_result = self.mox.CreateMockAnything() - runner.run(suite).AndReturn(test_result) - test_result.wasSuccessful().AndReturn(True) - test_result.failures = 0 - test_result.errors = 0 + 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(('', '')) self.mox.ReplayAll() results = presubmit_canned_checks.RunPythonUnitTests(