Change RunPythonUnitTests() to run the unit tests in a separate process.

The unit tests could modify global state in hard-to-revert ways and would make the PRESUBMIT.py check flaky.
Having the test running out of process alleviate any potential issue at the cost of speed (more noticeable on Windows).

TEST=unit tests
BUG=none

Review URL: http://codereview.chromium.org/147035

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@19248 0039d316-1c4b-4281-b951-d872f2087c98

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 []

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
 

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(