Use check_call() instead of Popen() for canned checks.

This removes a significant amount of code.

R=dpranke@chromium.org
BUG=
TEST=

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

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

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):

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(