Add subprocess.Popen().kill() to python 2.5.

Move code in tests/fake_repos.py to subprocess2.py.

R=dpranke@chromium.org
BUG=
TEST=

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

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

subprocess2.py

@@ -41,6 +41,47 @@ class CalledProcessError(subprocess.CalledProcessError):
     return '\n'.join(filter(None, (out, self.stdout, self.stderr)))
 
 
+## Utility functions
+
+
+def kill_pid(pid):
+  """Kills a process by its process id."""
+  try:
+    # Unable to import 'module'
+    # pylint: disable=F0401
+    import signal
+    return os.kill(pid, signal.SIGKILL)
+  except ImportError:
+    pass
+
+
+def kill_win(process):
+  """Kills a process with its windows handle.
+
+  Has no effect on other platforms.
+  """
+  try:
+    # Unable to import 'module'
+    # pylint: disable=F0401
+    import win32process
+    # Access to a protected member _handle of a client class
+    # pylint: disable=W0212
+    return win32process.TerminateProcess(process._handle, -1)
+  except ImportError:
+    pass
+
+
+def add_kill():
+  """Adds kill() method to subprocess.Popen for python <2.6"""
+  if hasattr(subprocess.Popen, 'kill'):
+    return
+
+  if sys.platform == 'win32':
+    subprocess.Popen.kill = kill_win
+  else:
+    subprocess.Popen.kill = lambda process: kill_pid(process.pid)
+
+
 def hack_subprocess():
   """subprocess functions may throw exceptions when used in multiple threads.
 
@@ -92,6 +133,7 @@ def Popen(args, **kwargs):
   """
   # Make sure we hack subprocess if necessary.
   hack_subprocess()
+  add_kill()
 
   env = get_english_env(kwargs.get('env'))
   if env:

tests/fake_repos.py

@@ -13,7 +13,6 @@ import os
 import pprint
 import re
 import socket
-import subprocess
 import sys
 import tempfile
 import time
@@ -22,48 +21,7 @@ import time
 from tests import trial_dir
 import gclient_utils
 import scm
-
-## Utility functions
-
-
-def kill_pid(pid):
-  """Kills a process by its process id."""
-  try:
-    # Unable to import 'module'
-    # pylint: disable=F0401
-    import signal
-    return os.kill(pid, signal.SIGKILL)
-  except ImportError:
-    pass
-
-
-def kill_win(process):
-  """Kills a process with its windows handle.
-
-  Has no effect on other platforms.
-  """
-  try:
-    # Unable to import 'module'
-    # pylint: disable=F0401
-    import win32process
-    # Access to a protected member _handle of a client class
-    # pylint: disable=W0212
-    return win32process.TerminateProcess(process._handle, -1)
-  except ImportError:
-    pass
-
-
-def add_kill():
-  """Adds kill() method to subprocess.Popen for python <2.6"""
-  if hasattr(subprocess.Popen, 'kill'):
-    return
-
-  if sys.platform == 'win32':
-    subprocess.Popen.kill = kill_win
-  else:
-    def kill_nix(process):
-      return kill_pid(process.pid)
-    subprocess.Popen.kill = kill_nix
+import subprocess2
 
 
 def write(path, content):
@@ -75,18 +33,6 @@ def write(path, content):
 join = os.path.join
 
 
-def check_call(*args, **kwargs):
-  logging.debug(args[0])
-  subprocess.check_call(*args, **kwargs)
-
-
-def Popen(*args, **kwargs):
-  kwargs.setdefault('stdout', subprocess.PIPE)
-  kwargs.setdefault('stderr', subprocess.STDOUT)
-  logging.debug(args[0])
-  return subprocess.Popen(*args, **kwargs)
-
-
 def read_tree(tree_root):
   """Returns a dict of all the files in a tree. Defaults to self.root_dir."""
   tree = {}
@@ -123,31 +69,32 @@ def commit_svn(repo, usr, pwd):
     elif status[0] == '!':
       to_remove.append(filepath)
   if to_add:
-    check_call(['svn', 'add', '--no-auto-props', '-q'] + to_add, cwd=repo)
+    subprocess2.check_output(
+        ['svn', 'add', '--no-auto-props', '-q'] + to_add, cwd=repo)
   if to_remove:
-    check_call(['svn', 'remove', '-q'] + to_remove, cwd=repo)
-  proc = Popen(
+    subprocess2.check_output(['svn', 'remove', '-q'] + to_remove, cwd=repo)
+
+  out = subprocess2.check_output(
       ['svn', 'commit', repo, '-m', 'foo', '--non-interactive',
         '--no-auth-cache',
         '--username', usr, '--password', pwd],
       cwd=repo)
-  out, err = proc.communicate()
   match = re.search(r'(\d+)', out)
   if not match:
-    raise Exception('Commit failed', out, err, proc.returncode)
+    raise Exception('Commit failed', out)
   rev = match.group(1)
-  st = Popen(['svn', 'status'], cwd=repo).communicate()[0]
-  assert len(st) == 0, st
+  status = subprocess2.check_output(['svn', 'status'], cwd=repo)
+  assert len(status) == 0, status
   logging.debug('At revision %s' % rev)
   return rev
 
 
 def commit_git(repo):
   """Commits the changes and returns the new hash."""
-  check_call(['git', 'add', '-A', '-f'], cwd=repo)
-  check_call(['git', 'commit', '-q', '--message', 'foo'], cwd=repo)
-  rev = Popen(['git', 'show-ref', '--head', 'HEAD'],
-              cwd=repo).communicate()[0].split(' ', 1)[0]
+  subprocess2.check_call(['git', 'add', '-A', '-f'], cwd=repo)
+  subprocess2.check_call(['git', 'commit', '-q', '--message', 'foo'], cwd=repo)
+  rev = subprocess2.check_output(
+      ['git', 'show-ref', '--head', 'HEAD'], cwd=repo).split(' ', 1)[0]
   logging.debug('At revision %s' % rev)
   return rev
 
@@ -271,7 +218,6 @@ class FakeReposBase(object):
     self.cleanup_dirt()
     if not self.root_dir:
       try:
-        add_kill()
         # self.root_dir is not set before this call.
         self.trial.set_up()
         self.git_root = join(self.root_dir, 'git')
@@ -328,7 +274,7 @@ class FakeReposBase(object):
         pid = int(self.git_pid_file.read())
         self.git_pid_file.close()
         logging.debug('Killing git daemon pid %s' % pid)
-        kill_pid(pid)
+        subprocess2.kill_pid(pid)
         self.git_pid_file = None
       wait_for_port_to_free(self.host, self.git_port)
       self.git_port = None
@@ -363,8 +309,8 @@ class FakeReposBase(object):
     if self.svnserve:
       return True
     try:
-      check_call(['svnadmin', 'create', self.svn_repo])
-    except OSError, e:
+      subprocess2.check_call(['svnadmin', 'create', self.svn_repo])
+    except subprocess2.CalledProcessError, e:
       logging.debug('Failed with : %s' % e)
       return False
     write(join(self.svn_repo, 'conf', 'svnserve.conf'),
@@ -390,7 +336,11 @@ class FakeReposBase(object):
     if self.host == '127.0.0.1':
       cmd.append('--listen-host=' + self.host)
     self.check_port_is_free(self.svn_port)
-    self.svnserve = Popen(cmd, cwd=self.svn_repo)
+    self.svnserve = subprocess2.Popen(
+        cmd,
+        cwd=self.svn_repo,
+        stdout=subprocess2.PIPE,
+        stderr=subprocess2.PIPE)
     wait_for_port_to_bind(self.host, self.svn_port, self.svnserve)
     self.svn_base = 'svn://%s:%d/svn/' % (self.host, self.svn_port)
     self.populateSvn()
@@ -406,7 +356,7 @@ class FakeReposBase(object):
       return False
     assert self.git_pid_file == None
     for repo in ['repo_%d' % r for r in range(1, self.NB_GIT_REPOS + 1)]:
-      check_call(['git', 'init', '-q', join(self.git_root, repo)])
+      subprocess2.check_call(['git', 'init', '-q', join(self.git_root, repo)])
       self.git_hashes[repo] = [None]
     self.git_port = find_free_port(self.host, 20000)
     self.git_base = 'git://%s:%d/git/' % (self.host, self.git_port)
@@ -421,7 +371,11 @@ class FakeReposBase(object):
     if self.host == '127.0.0.1':
       cmd.append('--listen=' + self.host)
     self.check_port_is_free(self.git_port)
-    self.gitdaemon = Popen(cmd, cwd=self.root_dir)
+    self.gitdaemon = subprocess2.Popen(
+        cmd,
+        cwd=self.root_dir,
+        stdout=subprocess2.PIPE,
+        stderr=subprocess2.PIPE)
     wait_for_port_to_bind(self.host, self.git_port, self.gitdaemon)
     self.populateGit()
     self.git_dirty = False
@@ -473,9 +427,10 @@ class FakeRepos(FakeReposBase):
   def populateSvn(self):
     """Creates a few revisions of changes including DEPS files."""
     # Repos
-    check_call(['svn', 'checkout', self.svn_base, self.svn_checkout,
-                '-q', '--non-interactive', '--no-auth-cache',
-                '--username', self.USERS[0][0], '--password', self.USERS[0][1]])
+    subprocess2.check_call(
+        ['svn', 'checkout', self.svn_base, self.svn_checkout,
+         '-q', '--non-interactive', '--no-auth-cache',
+         '--username', self.USERS[0][0], '--password', self.USERS[0][1]])
     assert os.path.isdir(join(self.svn_checkout, '.svn'))
     def file_system(rev, DEPS):
       fs = {

tests/gclient_smoketest.py

@@ -21,7 +21,8 @@ import unittest
 
 ROOT_DIR = os.path.dirname(os.path.abspath(__file__))
 sys.path.insert(0, os.path.dirname(ROOT_DIR))
-from tests.fake_repos import check_call, join, write, FakeReposTestBase
+from tests.fake_repos import join, write, FakeReposTestBase
+import subprocess2
 
 GCLIENT_PATH = os.path.join(os.path.dirname(ROOT_DIR), 'gclient')
 COVERAGE = False
@@ -1082,7 +1083,7 @@ class GClientSmokeFromCheckout(GClientSmokeBase):
     os.rmdir(self.root_dir)
     if self.enabled:
       usr, pwd = self.FAKE_REPOS.USERS[0]
-      check_call(
+      subprocess2.check_call(
           ['svn', 'checkout', self.svn_base + '/trunk/webkit',
            self.root_dir, '-q',
            '--non-interactive', '--no-auth-cache',

tests/local_rietveld.py

@@ -13,9 +13,12 @@ if necessary and starts the server on a free inbound TCP port.
 import optparse
 import os
 import socket
-import subprocess
+import sys
 import time
 
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..'))
+import subprocess2
+
 
 class Failure(Exception):
   pass
@@ -55,9 +58,6 @@ class LocalRietveld(object):
     self.rietveld = os.path.join(self.base_dir, 'tests', 'rietveld')
     self.test_server = None
     self.port = None
-    # Generate a friendly environment.
-    self.env = os.environ.copy()
-    self.env['LANGUAGE'] = 'en'
     self.out = None
     self.err = None
 
@@ -66,23 +66,21 @@ class LocalRietveld(object):
     if not os.path.isfile(self.dev_app):
       raise Failure('Install google_appengine sdk in %s' % self.sdk_path)
 
-    def call(*args, **kwargs):
-      kwargs['env'] = self.env
-      x = subprocess.Popen(*args, **kwargs)
-      x.communicate()
-      return x.returncode == 0
-
     # Second, checkout rietveld if not available.
     if not os.path.isdir(self.rietveld):
       print('Checking out rietveld...')
-      if not call(
-          ['svn', 'co', '-q',
-              'http://rietveld.googlecode.com/svn/trunk@681',
-              self.rietveld]):
+      try:
+        subprocess2.check_call(
+            ['svn', 'co', '-q', 'http://rietveld.googlecode.com/svn/trunk@681',
+             self.rietveld])
+      except subprocess2.CalledProcessError:
         raise Failure('Failed to checkout rietveld')
     else:
       print('Syncing rietveld...')
-      if not call(['svn', 'up', '-q', '-r', '681'], cwd=self.rietveld):
+      try:
+        subprocess2.check_call(
+            ['svn', 'up', '-q', '-r', '681'], cwd=self.rietveld)
+      except subprocess2.CalledProcessError:
         raise Failure('Failed to checkout rietveld')
 
   def start_server(self, verbose=False):
@@ -101,9 +99,8 @@ class LocalRietveld(object):
         '--port=%d' % self.port,
         '--datastore_path=' + os.path.join(self.rietveld, 'tmp.db'),
         '-c']
-    self.test_server = subprocess.Popen(
-        cmd, stdout=self.out, stderr=self.err, env=self.env,
-        cwd=self.rietveld)
+    self.test_server = subprocess2.Popen(
+        cmd, stdout=self.out, stderr=self.err, cwd=self.rietveld)
     # Loop until port 127.0.0.1:port opens or the process dies.
     while not test_port(self.port):
       self.test_server.poll()
@@ -111,11 +108,12 @@ class LocalRietveld(object):
         raise Failure(
             'Test rietveld instance failed early on port %s' %
             self.port)
-      time.sleep(0.001)
+      time.sleep(0.01)
 
   def stop_server(self):
     if self.test_server:
       self.test_server.kill()
+      self.test_server.wait()
       self.test_server = None
       self.port = None
     if self.out: