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

presubmit_canned_checks.py

 # Copyright (c) 2010 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Generic presubmit checks that can be reused by other presubmit checks."""
 
 
 ### Description checks
 
 def CheckChangeHasTestField(input_api, output_api):
@@ skipping 430 matching lines @@
   """Runs all unit tests in a directory.
 
   On Windows, sys.executable is used for unit tests ending with ".py".
   """
   # 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.PresubmitPromptWarning
 
-  if verbose:
-    pipe = None
-  else:
-    pipe = input_api.subprocess.PIPE
-
   results = []
   for unit_test in unit_tests:
     cmd = []
     if input_api.platform == 'win32' and unit_test.endswith('.py'):
       # Windows needs some help.
       cmd = [input_api.python_executable]
     cmd.append(unit_test)
     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
 
 
 def RunPythonUnitTests(input_api, output_api, unit_tests):
   """Run the unit tests out of process, capture the output and use the result
   code to determine success.
 
   DEPRECATED.
   """
   # 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
-  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
     # 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()
       # At least on Windows, it seems '.' must explicitly be in PYTHONPATH
       backpath = [
           '.', input_api.os_path.pathsep.join(['..'] * (cwd.count('/') + 1))
         ]
       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):
   """Hack to fetch all files."""
   # We cannot use AffectedFiles here because we want to test every python
   # file on each single python change. It's because a change in a python file
   # can break another unmodified file.
   # Use code similar to InputApi.FilterSourceFile()
   def Find(filepath, filters):
     for item in filters:
@@ skipping 357 matching lines @@
       input_api, output_api, source_file_filter=text_files))
   results.extend(input_api.canned_checks.CheckSvnForCommonMimeTypes(
       input_api, output_api))
   results.extend(input_api.canned_checks.CheckLicense(
       input_api, output_api, license_header, source_file_filter=sources))
   results.extend(_CheckConstNSObject(
       input_api, output_api, source_file_filter=sources))
   results.extend(_CheckSingletonInHeaders(
       input_api, output_api, source_file_filter=sources))
   return results