Get output from hung mojo apptests.

Depends on https://codereview.chromium.org/1239563003

Start Xvfb and Openbox separately from the test process.
This lets us get output from killed hung test processes.
(wrapping the commandline with xvfb.py prevents this)

Remove xvfb from Paths objects. Use more single quotes.

TODO: Use test_env.run_executable() or its env vars?

BUG=493535,504917
TEST=Apptests killed for timeouts yield output.
R=maruel@chromium.org
TBR=ben@chromium.org,sky@chromium.org

Review URL: https://codereview.chromium.org/1230683002

Cr-Commit-Position: refs/heads/master@{#338943}

11 files changed, 328 insertions, 303 deletions

diff --git a/mandoline/tools/android/install_mandoline.py b/mandoline/tools/android/install_mandoline.py
index 3cf465b..4fd7703 100755
--- a/mandoline/tools/android/install_mandoline.py
+++ b/mandoline/tools/android/install_mandoline.py
@@ -15,7 +15,7 @@ sys.path.insert(0, os.path.join(os.path.abspath(os.path.dirname(__file__)),
 from mopy.android import AndroidShell
 from mopy.config import Config
 
-USAGE = ("install_mandoline.py [<shell-and-app-args>]")
+USAGE = ('install_mandoline.py [<shell-and-app-args>]')
 
 def main():
   logging.basicConfig()
@@ -35,11 +35,11 @@ def main():
   config = Config(target_os=Config.OS_ANDROID,
                   target_cpu=runner_args.target_cpu,
                   is_debug=runner_args.debug,
-                  apk_name="Mandoline.apk")
+                  apk_name='Mandoline.apk')
   shell = AndroidShell(config)
   shell.InitShell(None, runner_args.device)
   return 0
 
 
-if __name__ == "__main__":
+if __name__ == '__main__':
   sys.exit(main())
diff --git a/mandoline/tools/android/run_mandoline.py b/mandoline/tools/android/run_mandoline.py
index 6156242..c06e008 100755
--- a/mandoline/tools/android/run_mandoline.py
+++ b/mandoline/tools/android/run_mandoline.py
@@ -22,10 +22,10 @@ sys.path.insert(0, os.path.join(os.path.abspath(os.path.dirname(__file__)),
 from mopy.android import AndroidShell
 from mopy.config import Config
 
-USAGE = ("run_mandoline.py [<shell-and-app-args>] [<start-page-url>]")
+USAGE = ('run_mandoline.py [<shell-and-app-args>] [<start-page-url>]')
 
 def _CreateSOLinks(dest_dir):
-  """Creates links from files (eg. *.mojo) to the real .so for gdb to find."""
+  '''Creates links from files (eg. *.mojo) to the real .so for gdb to find.'''
   # The files to create links for. The key is the name as seen on the device,
   # and the target an array of path elements as to where the .so lives (relative
   # to the output directory).
@@ -65,7 +65,7 @@ def main():
   config = Config(target_os=Config.OS_ANDROID,
                   target_cpu=runner_args.target_cpu,
                   is_debug=runner_args.debug,
-                  apk_name="Mandoline.apk")
+                  apk_name='Mandoline.apk')
   shell = AndroidShell(config)
   shell.InitShell(None, runner_args.device)
   p = shell.ShowLogs()
@@ -84,5 +84,5 @@ def main():
   return 0
 
 
-if __name__ == "__main__":
+if __name__ == '__main__':
   sys.exit(main())
diff --git a/mojo/tools/android_mojo_shell.py b/mojo/tools/android_mojo_shell.py
index 032557f..cf80f02 100755
--- a/mojo/tools/android_mojo_shell.py
+++ b/mojo/tools/android_mojo_shell.py
@@ -10,7 +10,7 @@ import sys
 from mopy.android import AndroidShell
 from mopy.config import Config
 
-USAGE = ("android_mojo_shell.py [<shell-and-app-args>] [<mojo-app>]")
+USAGE = ('android_mojo_shell.py [<shell-and-app-args>] [<mojo-app>]')
 
 def main():
   logging.basicConfig()
@@ -27,18 +27,18 @@ def main():
   parser.add_argument('--origin', help='Origin for mojo: URLs.',
                       default='localhost')
   parser.add_argument('--device', help='Serial number of the target device.')
-  parser.add_argument("--verbose", default=False, action='store_true')
+  parser.add_argument('--verbose', default=False, action='store_true')
   runner_args, args = parser.parse_known_args()
 
   logger = logging.getLogger()
-  logging.basicConfig(stream=sys.stdout, format="%(levelname)s:%(message)s")
+  logging.basicConfig(stream=sys.stdout, format='%(levelname)s:%(message)s')
   logger.setLevel(logging.DEBUG if runner_args.verbose else logging.WARNING)
-  logger.debug("Initialized logging: level=%s" % logger.level)
+  logger.debug('Initialized logging: level=%s' % logger.level)
 
   config = Config(target_os=Config.OS_ANDROID,
                   target_cpu=runner_args.target_cpu,
                   is_debug=runner_args.debug,
-                  apk_name="MojoRunner.apk")
+                  apk_name='MojoRunner.apk')
   shell = AndroidShell(config)
   shell.InitShell(runner_args.origin, runner_args.device)
   p = shell.ShowLogs()
@@ -46,5 +46,5 @@ def main():
   return 0
 
 
-if __name__ == "__main__":
+if __name__ == '__main__':
   sys.exit(main())
diff --git a/mojo/tools/apptest_runner.py b/mojo/tools/apptest_runner.py
index 6e0d2a3..2115578 100755
--- a/mojo/tools/apptest_runner.py
+++ b/mojo/tools/apptest_runner.py
@@ -3,7 +3,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-"""A test runner for gtest application tests."""
+'''A test runner for gtest application tests.'''
 
 import argparse
 import json
@@ -16,34 +16,35 @@ from mopy import gtest
 from mopy.config import Config
 
 
+APPTESTS = os.path.abspath(os.path.join(__file__, '..', 'data', 'apptests'))
+
+
 def main():
-  parser = argparse.ArgumentParser(description="An application test runner.")
-  parser.add_argument("build_dir", type=str, help="The build output directory.")
-  parser.add_argument("--verbose", default=False, action='store_true',
-                      help="Print additional logging information.")
+  parser = argparse.ArgumentParser(description='An application test runner.')
+  parser.add_argument('build_dir', type=str, help='The build output directory.')
+  parser.add_argument('--verbose', default=False, action='store_true',
+                      help='Print additional logging information.')
   parser.add_argument('--repeat-count', default=1, metavar='INT',
                       action='store', type=int,
-                      help="The number of times to repeat the set of tests.")
+                      help='The number of times to repeat the set of tests.')
   parser.add_argument('--write-full-results-to', metavar='FILENAME',
                       help='The path to write the JSON list of full results.')
-  parser.add_argument("--test-list-file", metavar='FILENAME', type=file,
-                      default=os.path.abspath(os.path.join(__file__, os.pardir,
-                                                           "data", "apptests")),
-                      help="The file listing apptests to run.")
+  parser.add_argument('--test-list-file', metavar='FILENAME', type=file,
+                      default=APPTESTS, help='The file listing tests to run.')
   args = parser.parse_args()
 
   gtest.set_color()
   logger = logging.getLogger()
-  logging.basicConfig(stream=sys.stdout, format="%(levelname)s:%(message)s")
+  logging.basicConfig(stream=sys.stdout, format='%(levelname)s:%(message)s')
   logger.setLevel(logging.DEBUG if args.verbose else logging.WARNING)
-  logger.debug("Initialized logging: level=%s" % logger.level)
+  logger.debug('Initialized logging: level=%s' % logger.level)
 
-  logger.debug("Test list file: %s", args.test_list_file)
+  logger.debug('Test list file: %s', args.test_list_file)
   config = Config(args.build_dir)
-  execution_globals = {"config": config}
+  execution_globals = {'config': config}
   exec args.test_list_file in execution_globals
-  test_list = execution_globals["tests"]
-  logger.debug("Test list: %s" % test_list)
+  test_list = execution_globals['tests']
+  logger.debug('Test list: %s' % test_list)
 
   shell = None
   if config.target_os == Config.OS_ANDROID:
@@ -58,39 +59,39 @@ def main():
   failed_suites = 0
   for _ in range(args.repeat_count):
     for test_dict in test_list:
-      test = test_dict["test"]
-      test_name = test_dict.get("name", test)
-      test_type = test_dict.get("type", "gtest")
-      test_args = test_dict.get("args", [])
+      test = test_dict['test']
+      test_name = test_dict.get('name', test)
+      test_type = test_dict.get('type', 'gtest')
+      test_args = test_dict.get('args', [])
 
-      print "Running %s...%s" % (test_name, ("\n" if args.verbose else "")),
+      print 'Running %s...%s' % (test_name, ('\n' if args.verbose else '')),
       sys.stdout.flush()
 
-      assert test_type in ("gtest", "gtest_isolated")
-      isolate = test_type == "gtest_isolated"
+      assert test_type in ('gtest', 'gtest_isolated')
+      isolate = test_type == 'gtest_isolated'
       (test, fail) = gtest.run_apptest(config, shell, test_args, test, isolate)
       tests.extend(test)
       failed.extend(fail)
       result = test and not fail
-      print "[  PASSED  ]" if result else "[  FAILED  ]",
-      print test_name if args.verbose or not result else ""
+      print '[  PASSED  ]' if result else '[  FAILED  ]',
+      print test_name if args.verbose or not result else ''
       # Abort when 3 apptest suites, or a tenth of all, have failed.
       # base::TestLauncher does this for timeouts and unknown results.
       failed_suites += 0 if result else 1
       if failed_suites >= max(3, len(test_list) / 10):
-        print "Too many failing suites (%d), exiting now." % failed_suites
-        failed.append("Test runner aborted for excessive failures.")
+        print 'Too many failing suites (%d), exiting now.' % failed_suites
+        failed.append('Test runner aborted for excessive failures.')
         break;
 
     if failed:
       break;
 
-  print "[==========] %d tests ran." % len(tests)
-  print "[  PASSED  ] %d tests." % (len(tests) - len(failed))
+  print '[==========] %d tests ran.' % len(tests)
+  print '[  PASSED  ] %d tests.' % (len(tests) - len(failed))
   if failed:
-    print "[  FAILED  ] %d tests, listed below:" % len(failed)
+    print '[  FAILED  ] %d tests, listed below:' % len(failed)
     for failure in failed:
-      print "[  FAILED  ] %s" % failure
+      print '[  FAILED  ] %s' % failure
 
   if args.write_full_results_to:
     _WriteJSONResults(tests, failed, args.write_full_results_to)
@@ -99,12 +100,12 @@ def main():
 
 
 def _WriteJSONResults(tests, failed, write_full_results_to):
-  """Write the apptest results in the Chromium JSON test results format.
+  '''Write the apptest results in the Chromium JSON test results format.
      See <http://www.chromium.org/developers/the-json-test-results-format>
      TODO(msw): Use Chromium and TYP testing infrastructure.
      TODO(msw): Use GTest Suite.Fixture names, not the apptest names.
      Adapted from chrome/test/mini_installer/test_installer.py
-  """
+  '''
   results = {
     'interrupted': False,
     'path_delimiter': '.',
diff --git a/mojo/tools/data/apptests b/mojo/tools/data/apptests
index 140ee6a..f656322 100644
--- a/mojo/tools/data/apptests
+++ b/mojo/tools/data/apptests
@@ -7,38 +7,38 @@
 # The entries in |tests| are dictionaries of the following form:
 #   {
 #     # Required URL for apptest.
-#     "test": "mojo:test_app_url",
-#     # Optional display name (otherwise the entry for "test" above is used).
-#     "name": "mojo:test_app_url (more details)",
+#     'test': 'mojo:test_app_url',
+#     # Optional display name (otherwise the entry for 'test' above is used).
+#     'name': 'mojo:test_app_url (more details)',
 #     # Optional test type. Valid values:
-#     #   * "gtest": (default)
-#     #   * "gtest_isolated": like "gtest", but run with fixture isolation.
+#     #   * 'gtest': (default)
+#     #   * 'gtest_isolated': like 'gtest', but run with fixture isolation.
 #     #                       i.e., each test in a fresh mojo_shell
-#     "type": "gtest",
+#     'type': 'gtest',
 #     # Optional arguments for the shell or test.
-#     "args": ["--some-flag-for-the-shell", "--some-flag-for-the-test"],
+#     'args': ['--some-flag-for-the-shell', '--some-flag-for-the-test'],
 #   }
 #
 # TODO(vtl|msw): Add a way of specifying data dependencies.
 
 tests = [
   {
-    "test": "mojo:clipboard_apptests",
-    "type": "gtest_isolated",
+    'test': 'mojo:clipboard_apptests',
+    'type': 'gtest_isolated',
   },
   {
-    "test": "mojo:network_service_apptests",
-    "type": "gtest_isolated",
+    'test': 'mojo:network_service_apptests',
+    'type': 'gtest_isolated',
   },
   # TODO(msw|jam): Fix and enable the runner_apptests: http://crbug.com/479316
   #{
-  #  "test": "mojo:runner_apptests",
-  #  "type": "gtest_isolated",
+  #  'test': 'mojo:runner_apptests',
+  #  'type': 'gtest_isolated',
   #},
   {
-    "test": "mojo:view_manager_apptests",
-    "type": "gtest_isolated",
-    "args": ["--use-headless-config"]
+    'test': 'mojo:view_manager_apptests',
+    'type': 'gtest_isolated',
+    'args': ['--use-headless-config']
   },
 ]
 
@@ -46,42 +46,42 @@ tests = [
 if config.target_os != config.OS_ANDROID:
   tests += [
     {
-      "test": "mojo:filesystem_apptests",
-      "type": "gtest_isolated",
+      'test': 'mojo:filesystem_apptests',
+      'type': 'gtest_isolated',
     },
     {
-      "test": "mojo:html_viewer_apptests",
-      "type": "gtest_isolated",
-      "args": ["--use-test-config"]
+      'test': 'mojo:html_viewer_apptests',
+      'type': 'gtest_isolated',
+      'args': ['--use-test-config']
     },
     {
-      "test": "mojo:html_viewer_apptests",
-      "type": "gtest_isolated",
-      "args": ["--use-test-config", "--oopifs",
-               "--enable-html-viewer-test-interface"]
+      'test': 'mojo:html_viewer_apptests',
+      'type': 'gtest_isolated',
+      'args': ['--use-test-config', '--oopifs',
+               '--enable-html-viewer-test-interface']
     },
     {
-      "test": "mojo:mandoline_browser_apptests",
-      "type": "gtest_isolated",
-      "args": ["--use-headless-config"]
+      'test': 'mojo:mandoline_browser_apptests',
+      'type': 'gtest_isolated',
+      'args': ['--use-headless-config']
     },
     {
-      "test": "mojo:mandoline_frame_apptests",
-      "type": "gtest_isolated",
-      "args": ["--use-headless-config"]
+      'test': 'mojo:mandoline_frame_apptests',
+      'type': 'gtest_isolated',
+      'args': ['--use-headless-config']
     },
     # TODO(xhwang): Fix and enable mojo:media_pipeline_integration_apptests.
     # http://crbug.com/501417
     {
-      "test": "mojo:media_apptests",
-      "type": "gtest_isolated",
+      'test': 'mojo:media_apptests',
+      'type': 'gtest_isolated',
     },
     {
-      "test": "mojo:resource_provider_apptests",
-      "type": "gtest_isolated",
+      'test': 'mojo:resource_provider_apptests',
+      'type': 'gtest_isolated',
     },
     {
-      "test": "mojo:sql_apptests",
-      "type": "gtest_isolated",
+      'test': 'mojo:sql_apptests',
+      'type': 'gtest_isolated',
     },
   ]
diff --git a/mojo/tools/generate_mojo_shell_assets_list.py b/mojo/tools/generate_mojo_shell_assets_list.py
index 1a55af4..1018e94 100755
--- a/mojo/tools/generate_mojo_shell_assets_list.py
+++ b/mojo/tools/generate_mojo_shell_assets_list.py
@@ -4,14 +4,14 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-"""Generates the assets_list file from a directory."""
+'''Generates the assets_list file from a directory.'''
 
 import argparse
 import os
 import sys
 
 def main():
-  parser = argparse.ArgumentParser(usage="--dir <directory>")
+  parser = argparse.ArgumentParser(usage='--dir <directory>')
 
   parser.add_argument('--dir', help='Directory read files from.', required=True)
 
diff --git a/mojo/tools/mopy/android.py b/mojo/tools/mopy/android.py
index ec275db..3b09310 100644
--- a/mojo/tools/mopy/android.py
+++ b/mojo/tools/mopy/android.py
@@ -15,8 +15,8 @@ import urlparse
 
 from .paths import Paths
 
-sys.path.append(os.path.join(os.path.dirname(__file__), os.pardir, os.pardir,
-                             os.pardir, 'build', 'android'))
+sys.path.append(os.path.join(os.path.abspath(os.path.dirname(__file__)),
+                             '..', '..', '..', 'build', 'android'))
 from pylib import constants
 from pylib.base import base_test_runner
 from pylib.device import device_errors
@@ -39,16 +39,16 @@ MAPPING_PREFIX = '--map-origin='
 
 
 def _ExitIfNeeded(process):
-  """Exits |process| if it is still alive."""
+  '''Exits |process| if it is still alive.'''
   if process.poll() is None:
     process.kill()
 
 
 class AndroidShell(object):
-  """
+  '''
   Used to set up and run a given mojo shell binary on an Android device.
   |config| is the mopy.config.Config for the build.
-  """
+  '''
   def __init__(self, config):
     self.adb_path = constants.GetAdbPath()
     self.config = config
@@ -64,16 +64,16 @@ class AndroidShell(object):
   def _CreateADBCommand(self, args):
     adb_command = [self.adb_path, '-s', self.device.adb.GetDeviceSerial()]
     adb_command.extend(args)
-    logging.getLogger().debug("Command: %s", " ".join(adb_command))
+    logging.getLogger().debug('Command: %s', ' '.join(adb_command))
     return adb_command
 
   def _ReadFifo(self, path, pipe, on_fifo_closed, max_attempts=5):
-    """
+    '''
     Reads the fifo at |path| on the device and write the contents to |pipe|.
     Calls |on_fifo_closed| when the fifo is closed. This method will try to find
     the path up to |max_attempts|, waiting 1 second between each attempt. If it
     cannot find |path|, a exception will be raised.
-    """
+    '''
     def Run():
       def _WaitForFifo():
         for _ in xrange(max_attempts):
@@ -81,7 +81,7 @@ class AndroidShell(object):
             return
           time.sleep(1)
         on_fifo_closed()
-        raise Exception("Unable to find fifo: %s" % path)
+        raise Exception('Unable to find fifo: %s' % path)
       _WaitForFifo()
       stdout_cat = subprocess.Popen(self._CreateADBCommand([
                                       'shell',
@@ -92,7 +92,7 @@ class AndroidShell(object):
       stdout_cat.wait()
       on_fifo_closed()
 
-    thread = threading.Thread(target=Run, name="StdoutRedirector")
+    thread = threading.Thread(target=Run, name='StdoutRedirector')
     thread.start()
 
   def _StartHttpServerForDirectory(self, path):
@@ -103,11 +103,11 @@ class AndroidShell(object):
     return 'http://127.0.0.1:%d/' % ports[0]
 
   def _StartHttpServerForOriginMapping(self, mapping):
-    """
+    '''
     If |mapping| points at a local file starts an http server to serve files
     from the directory and returns the new mapping. This is intended to be
     called for every --map-origin value.
-    """
+    '''
     parts = mapping.split('=')
     if len(parts) != 2:
       return mapping
@@ -121,7 +121,7 @@ class AndroidShell(object):
     return parts[0] + '=' + localUrl
 
   def _StartHttpServerForOriginMappings(self, map_parameters):
-    """Calls _StartHttpServerForOriginMapping for every --map-origin arg."""
+    '''Calls _StartHttpServerForOriginMapping for every --map-origin arg.'''
     if not map_parameters:
       return []
 
@@ -134,13 +134,13 @@ class AndroidShell(object):
     return [MAPPING_PREFIX + ','.join(result)]
 
   def InitShell(self, origin='localhost', device=None):
-    """
+    '''
     Runs adb as root, starts an origin server, and installs the apk as needed.
     |origin| is the origin for mojo: URLs; if its value is 'localhost', a local
     http server will be set up to serve files from the build directory.
     |device| is the target device to run on, if multiple devices are connected.
     Returns 0 on success or a non-zero exit code on a terminal failure.
-    """
+    '''
     try:
       devices = device_utils.DeviceUtils.HealthyDevices()
       if device:
@@ -152,26 +152,26 @@ class AndroidShell(object):
       else:
         raise device_errors.NoDevicesError()
 
-      logging.getLogger().debug("Using device: %s", self.device)
+      logging.getLogger().debug('Using device: %s', self.device)
       # Clean the logs on the device to avoid displaying prior activity.
       subprocess.check_call(self._CreateADBCommand(['logcat', '-c']))
       self.device.EnableRoot()
       self.device.Install(self.paths.apk_path)
     except base_error.BaseError as e:
-      # Report "device not found" as infra failures. See http://crbug.com/493900
-      print "Exception in AndroidShell.InitShell:\n%s" % str(e)
-      if e.is_infra_error or "error: device not found" in str(e):
+      # Report 'device not found' as infra failures. See http://crbug.com/493900
+      print 'Exception in AndroidShell.InitShell:\n%s' % str(e)
+      if e.is_infra_error or 'error: device not found' in str(e):
         return constants.INFRA_EXIT_CODE
       return constants.ERROR_EXIT_CODE
 
     if origin is 'localhost':
       origin = self._StartHttpServerForDirectory(self.paths.build_dir)
     if origin:
-      self.shell_args.append("--origin=" + origin)
+      self.shell_args.append('--origin=' + origin)
     return 0
 
   def _GetProcessId(self, process):
-    """Returns the process id of the process on the remote device."""
+    '''Returns the process id of the process on the remote device.'''
     while True:
       line = process.stdout.readline()
       pid_command = 'launcher waiting for GDB. pid: '
@@ -181,28 +181,28 @@ class AndroidShell(object):
     return 0
 
   def _GetLocalGdbPath(self):
-    """Returns the path to the android gdb."""
-    if self.config.target_cpu == "arm":
-      return os.path.join(constants.ANDROID_NDK_ROOT, "toolchains",
-                          "arm-linux-androideabi-4.9", "prebuilt",
-                          "linux-x86_64", "bin", "arm-linux-androideabi-gdb")
-    elif self.config.target_cpu == "x86":
-      return os.path.join(constants.ANDROID_NDK_ROOT, "toolchains",
-                          "x86-4.9", "prebuilt", "linux-x86_64", "bin",
-                          "i686-linux-android-gdb")
-    elif self.config.target_cpu == "x64":
-      return os.path.join(constants.ANDROID_NDK_ROOT, "toolchains",
-                          "x86_64-4.9", "prebuilt", "linux-x86_64", "bin",
-                          "x86_64-linux-android-gdb")
+    '''Returns the path to the android gdb.'''
+    if self.config.target_cpu == 'arm':
+      return os.path.join(constants.ANDROID_NDK_ROOT, 'toolchains',
+                          'arm-linux-androideabi-4.9', 'prebuilt',
+                          'linux-x86_64', 'bin', 'arm-linux-androideabi-gdb')
+    elif self.config.target_cpu == 'x86':
+      return os.path.join(constants.ANDROID_NDK_ROOT, 'toolchains',
+                          'x86-4.9', 'prebuilt', 'linux-x86_64', 'bin',
+                          'i686-linux-android-gdb')
+    elif self.config.target_cpu == 'x64':
+      return os.path.join(constants.ANDROID_NDK_ROOT, 'toolchains',
+                          'x86_64-4.9', 'prebuilt', 'linux-x86_64', 'bin',
+                          'x86_64-linux-android-gdb')
     else:
-      raise Exception("Unknown target_cpu: %s" % self.config.target_cpu)
+      raise Exception('Unknown target_cpu: %s' % self.config.target_cpu)
 
   def _WaitForProcessIdAndStartGdb(self, process):
-    """
+    '''
     Waits until we see the process id from the remote device, starts up
     gdbserver on the remote device, and gdb on the local device.
-    """
-    # Wait until we see "PID"
+    '''
+    # Wait until we see 'PID'
     pid = self._GetProcessId(process)
     assert pid != 0
     # No longer need the logcat process.
@@ -226,7 +226,7 @@ class AndroidShell(object):
     time.sleep(1)
 
     local_gdb_process = subprocess.Popen([self._GetLocalGdbPath(),
-                                          "-x",
+                                          '-x',
                                           gdbinit_path],
                                          cwd=self.temp_gdb_dir)
     atexit.register(_ExitIfNeeded, local_gdb_process)
@@ -239,12 +239,12 @@ class AndroidShell(object):
                     stdout,
                     on_fifo_closed,
                     temp_gdb_dir=None):
-    """
+    '''
     Starts the shell with the given |arguments|, directing output to |stdout|.
     |on_fifo_closed| will be run if the FIFO can't be found or when it's closed.
     |temp_gdb_dir| is set to a location with appropriate symlinks for gdb to
     find when attached to the device's remote process on startup.
-    """
+    '''
     assert self.device
     arguments += self.shell_args
 
@@ -266,7 +266,7 @@ class AndroidShell(object):
       self.device.adb.Forward('tcp:5039', 'tcp:5039')
       logcat_process = self.ShowLogs(stdout=subprocess.PIPE)
 
-    fifo_path = "/data/data/%s/stdout.fifo" % self.target_package
+    fifo_path = '/data/data/%s/stdout.fifo' % self.target_package
     subprocess.check_call(self._CreateADBCommand(
         ['shell', 'rm', '-f', fifo_path]))
     arguments.append('--fifo-path=%s' % fifo_path)
@@ -288,11 +288,11 @@ class AndroidShell(object):
       cmd_process.wait()
 
   def kill(self):
-    """Stops the mojo shell; matches the Popen.kill method signature."""
+    '''Stops the mojo shell; matches the Popen.kill method signature.'''
     self.device.ForceStop(self.target_package)
 
   def ShowLogs(self, stdout=sys.stdout):
-    """Displays the mojo shell logs and returns the process reading the logs."""
+    '''Displays the mojo shell logs and returns the process reading the logs.'''
     logcat = subprocess.Popen(self._CreateADBCommand([
                                'logcat',
                                '-s',
@@ -303,14 +303,14 @@ class AndroidShell(object):
 
 
 def _CreateGdbInit(tmp_dir, gdb_init_path, build_dir):
-  """
+  '''
   Creates the gdbinit file.
 
   Args:
     tmp_dir: the directory where the gdbinit and other files lives.
     gdb_init_path: path to gdbinit
     build_dir: path where build files are located.
-  """
+  '''
   gdbinit = ('target remote localhost:5039\n'
              'def reload-symbols\n'
              '  set solib-search-path %s:%s\n'
diff --git a/mojo/tools/mopy/config.py b/mojo/tools/mopy/config.py
index 8496d03..cac46d4 100644
--- a/mojo/tools/mopy/config.py
+++ b/mojo/tools/mopy/config.py
@@ -11,23 +11,23 @@ import sys
 
 
 class Config(object):
-  """A Config contains a dictionary that species a build configuration."""
+  '''A Config contains a dictionary that species a build configuration.'''
 
   # Valid values for target_os:
-  OS_ANDROID = "android"
-  OS_CHROMEOS = "chromeos"
-  OS_LINUX = "linux"
-  OS_MAC = "mac"
-  OS_WINDOWS = "windows"
+  OS_ANDROID = 'android'
+  OS_CHROMEOS = 'chromeos'
+  OS_LINUX = 'linux'
+  OS_MAC = 'mac'
+  OS_WINDOWS = 'windows'
 
   # Valid values for target_cpu:
-  ARCH_X86 = "x86"
-  ARCH_X64 = "x64"
-  ARCH_ARM = "arm"
+  ARCH_X86 = 'x86'
+  ARCH_X64 = 'x64'
+  ARCH_ARM = 'arm'
 
   def __init__(self, build_dir=None, target_os=None, target_cpu=None,
-               is_debug=None, apk_name="MojoRunner.apk"):
-    """Function arguments take precedence over GN args and default values."""
+               is_debug=None, apk_name='MojoRunner.apk'):
+    '''Function arguments take precedence over GN args and default values.'''
     assert target_os in (None, Config.OS_ANDROID, Config.OS_CHROMEOS,
                          Config.OS_LINUX, Config.OS_MAC, Config.OS_WINDOWS)
     assert target_cpu in (None, Config.ARCH_X86, Config.ARCH_X64,
@@ -35,59 +35,59 @@ class Config(object):
     assert is_debug in (None, True, False)
 
     self.values = {
-      "build_dir": build_dir,
-      "target_os": self.GetHostOS(),
-      "target_cpu": self.GetHostCPU(),
-      "is_debug": True,
-      "dcheck_always_on": False,
-      "is_asan": False,
-      "apk_name": apk_name,
+      'build_dir': build_dir,
+      'target_os': self.GetHostOS(),
+      'target_cpu': self.GetHostCPU(),
+      'is_debug': True,
+      'dcheck_always_on': False,
+      'is_asan': False,
+      'apk_name': apk_name,
     }
 
     self._ParseGNArgs()
     if target_os is not None:
-      self.values["target_os"] = target_os
+      self.values['target_os'] = target_os
     if target_cpu is not None:
-      self.values["target_cpu"] = target_cpu
+      self.values['target_cpu'] = target_cpu
     if is_debug is not None:
-      self.values["is_debug"] = is_debug
+      self.values['is_debug'] = is_debug
 
   @staticmethod
   def GetHostOS():
-    if sys.platform == "linux2":
+    if sys.platform == 'linux2':
       return Config.OS_LINUX
-    if sys.platform == "darwin":
+    if sys.platform == 'darwin':
       return Config.OS_MAC
-    if sys.platform == "win32":
+    if sys.platform == 'win32':
       return Config.OS_WINDOWS
-    raise NotImplementedError("Unsupported host OS")
+    raise NotImplementedError('Unsupported host OS')
 
   @staticmethod
   def GetHostCPU():
     # Derived from //native_client/pynacl/platform.py
     machine = platform.machine()
-    if machine in ("x86", "x86-32", "x86_32", "x8632", "i386", "i686", "ia32",
-                   "32"):
+    if machine in ('x86', 'x86-32', 'x86_32', 'x8632', 'i386', 'i686', 'ia32',
+                   '32'):
       return Config.ARCH_X86
-    if machine in ("x86-64", "amd64", "AMD64", "x86_64", "x8664", "64"):
+    if machine in ('x86-64', 'amd64', 'AMD64', 'x86_64', 'x8664', '64'):
       return Config.ARCH_X64
-    if machine.startswith("arm"):
+    if machine.startswith('arm'):
       return Config.ARCH_ARM
-    raise Exception("Cannot identify CPU arch: %s" % machine)
+    raise Exception('Cannot identify CPU arch: %s' % machine)
 
   def _ParseGNArgs(self):
-    """Parse the gn config file from the build directory, if it exists."""
-    TRANSLATIONS = { "true": "True", "false": "False", }
-    if self.values["build_dir"] is None:
+    '''Parse the gn config file from the build directory, if it exists.'''
+    TRANSLATIONS = { 'true': 'True', 'false': 'False', }
+    if self.values['build_dir'] is None:
       return
-    gn_file = os.path.join(self.values["build_dir"], "args.gn")
+    gn_file = os.path.join(self.values['build_dir'], 'args.gn')
     if not os.path.isfile(gn_file):
       return
 
-    with open(gn_file, "r") as f:
+    with open(gn_file, 'r') as f:
       for line in f:
-        line = re.sub("\s*#.*", "", line)
-        result = re.match("^\s*(\w+)\s*=\s*(.*)\s*$", line)
+        line = re.sub('\s*#.*', '', line)
+        result = re.match('^\s*(\w+)\s*=\s*(.*)\s*$', line)
         if result:
           key = result.group(1)
           value = result.group(2)
@@ -97,35 +97,35 @@ class Config(object):
 
   @property
   def build_dir(self):
-    """Build directory path."""
-    return self.values["build_dir"]
+    '''Build directory path.'''
+    return self.values['build_dir']
 
   @property
   def target_os(self):
-    """OS of the build/test target."""
-    return self.values["target_os"]
+    '''OS of the build/test target.'''
+    return self.values['target_os']
 
   @property
   def target_cpu(self):
-    """CPU arch of the build/test target."""
-    return self.values["target_cpu"]
+    '''CPU arch of the build/test target.'''
+    return self.values['target_cpu']
 
   @property
   def is_debug(self):
-    """Is Debug build?"""
-    return self.values["is_debug"]
+    '''Is Debug build?'''
+    return self.values['is_debug']
 
   @property
   def dcheck_always_on(self):
-    """DCHECK and MOJO_DCHECK are fatal even in release builds"""
-    return self.values["dcheck_always_on"]
+    '''DCHECK and MOJO_DCHECK are fatal even in release builds'''
+    return self.values['dcheck_always_on']
 
   @property
   def is_asan(self):
-    """Is ASAN build?"""
-    return self.values["is_asan"]
+    '''Is ASAN build?'''
+    return self.values['is_asan']
 
   @property
   def apk_name(self):
-    """Name of the APK file to run"""
-    return self.values["apk_name"]
+    '''Name of the APK file to run'''
+    return self.values['apk_name']
diff --git a/mojo/tools/mopy/gtest.py b/mojo/tools/mopy/gtest.py
index 189ae06..887ae07 100644
--- a/mojo/tools/mopy/gtest.py
+++ b/mojo/tools/mopy/gtest.py
@@ -14,16 +14,20 @@ import time
 from mopy.config import Config
 from mopy.paths import Paths
 
+sys.path.append(os.path.join(os.path.abspath(os.path.dirname(__file__)),
+                             '..', '..', '..', 'testing'))
+import xvfb
+
 
 def set_color():
-  """Run gtests with color on TTY, unless its environment variable is set."""
-  if sys.stdout.isatty() and "GTEST_COLOR" not in os.environ:
-    logging.getLogger().debug("Setting GTEST_COLOR=yes")
-    os.environ["GTEST_COLOR"] = "yes"
+  '''Run gtests with color on TTY, unless its environment variable is set.'''
+  if sys.stdout.isatty() and 'GTEST_COLOR' not in os.environ:
+    logging.getLogger().debug('Setting GTEST_COLOR=yes')
+    os.environ['GTEST_COLOR'] = 'yes'
 
 
 def run_apptest(config, shell, args, apptest, isolate):
-  """Run the apptest; optionally isolating fixtures across shell invocations.
+  '''Run the apptest; optionally isolating fixtures across shell invocations.
 
   Returns the list of tests run and the list of failures.
 
@@ -33,7 +37,7 @@ def run_apptest(config, shell, args, apptest, isolate):
     args: The arguments for the shell or apptest.
     apptest: The application test URL.
     isolate: True if the test fixtures should be run in isolation.
-  """
+  '''
   tests = [apptest]
   failed = []
   if not isolate:
@@ -44,69 +48,69 @@ def run_apptest(config, shell, args, apptest, isolate):
   else:
     tests = _get_fixtures(config, shell, args, apptest)
     for fixture in tests:
-      arguments = args + ["--gtest_filter=%s" % fixture]
+      arguments = args + ['--gtest_filter=%s' % fixture]
       if not _run_apptest_with_retry(config, shell, arguments, apptest):
         failed.append(fixture)
       # Abort when 20 fixtures, or a tenth of the apptest fixtures, have failed.
       # base::TestLauncher does this for timeouts and unknown results.
       if len(failed) >= max(20, len(tests) / 10):
-        print "Too many failing fixtures (%d), exiting now." % len(failed)
-        return (tests, failed + [apptest + " aborted for excessive failures."])
+        print 'Too many failing fixtures (%d), exiting now.' % len(failed)
+        return (tests, failed + [apptest + ' aborted for excessive failures.'])
   return (tests, failed)
 
 
 # TODO(msw): Determine proper test retry counts; allow configuration.
 def _run_apptest_with_retry(config, shell, args, apptest, try_count=3):
-  """Runs an apptest, retrying on failure; returns True if any run passed."""
+  '''Runs an apptest, retrying on failure; returns True if any run passed.'''
   for try_number in range(try_count):
     if _run_apptest(config, shell, args, apptest):
       return True
-    print "Failed %s/%s test run attempts." % (try_number + 1, try_count)
+    print 'Failed %s/%s test run attempts.' % (try_number + 1, try_count)
   return False
 
 
 def _run_apptest(config, shell, args, apptest):
-  """Runs an apptest and checks the output for signs of gtest failure."""
+  '''Runs an apptest and checks the output for signs of gtest failure.'''
   command = _build_command_line(config, args, apptest)
-  logging.getLogger().debug("Command: %s" % " ".join(command))
+  logging.getLogger().debug('Command: %s' % ' '.join(command))
   start_time = time.time()
 
   try:
-    output = _run_test_with_timeout(config, shell, args, apptest)
+    output = _run_test_with_xvfb(config, shell, args, apptest)
   except Exception as e:
     _print_exception(command, e)
     return False
 
-  # Fail on output with gtest's "[  FAILED  ]" or a lack of "[  PASSED  ]".
+  # Fail on output with gtest's '[  FAILED  ]' or a lack of '[  PASSED  ]'.
   # The latter condition ensures failure on broken command lines or output.
   # Check output instead of exit codes because mojo shell always exits with 0.
-  if output.find("[  FAILED  ]") != -1 or output.find("[  PASSED  ]") == -1:
+  if output.find('[  FAILED  ]') != -1 or output.find('[  PASSED  ]') == -1:
     _print_exception(command, output)
     return False
 
   ms = int(round(1000 * (time.time() - start_time)))
-  logging.getLogger().debug("Passed with output (%d ms):\n%s" % (ms, output))
+  logging.getLogger().debug('Passed with output (%d ms):\n%s' % (ms, output))
   return True
 
 
 def _get_fixtures(config, shell, args, apptest):
-  """Returns an apptest's "Suite.Fixture" list via --gtest_list_tests output."""
-  arguments = args + ["--gtest_list_tests"]
+  '''Returns an apptest's 'Suite.Fixture' list via --gtest_list_tests output.'''
+  arguments = args + ['--gtest_list_tests']
   command = _build_command_line(config, arguments, apptest)
-  logging.getLogger().debug("Command: %s" % " ".join(command))
+  logging.getLogger().debug('Command: %s' % ' '.join(command))
   try:
-    tests = _run_test_with_timeout(config, shell, arguments, apptest)
-    logging.getLogger().debug("Tests for %s:\n%s" % (apptest, tests))
+    tests = _run_test_with_xvfb(config, shell, arguments, apptest)
+    logging.getLogger().debug('Tests for %s:\n%s' % (apptest, tests))
     # Remove log lines from the output and ensure it matches known formatting.
-    tests = re.sub("^(\[|WARNING: linker:).*\n", "", tests, flags=re.MULTILINE)
-    if not re.match("^(\w*\.\r?\n(  \w*\r?\n)+)+", tests):
-      raise Exception("Unrecognized --gtest_list_tests output:\n%s" % tests)
-    tests = tests.split("\n")
+    tests = re.sub('^(\[|WARNING: linker:).*\n', '', tests, flags=re.MULTILINE)
+    if not re.match('^(\w*\.\r?\n(  \w*\r?\n)+)+', tests):
+      raise Exception('Unrecognized --gtest_list_tests output:\n%s' % tests)
+    tests = tests.split('\n')
     test_list = []
     for line in tests:
       if not line:
         continue
-      if line[0] != " ":
+      if line[0] != ' ':
         suite = line.strip()
         continue
       test_list.append(suite + line.strip())
@@ -117,80 +121,100 @@ def _get_fixtures(config, shell, args, apptest):
 
 
 def _print_exception(command_line, exception):
-  """Print a formatted exception raised from a failed command execution."""
-  exit_code = ""
+  '''Print a formatted exception raised from a failed command execution.'''
+  exit_code = ''
   if hasattr(exception, 'returncode'):
-    exit_code = " (exit code %d)" % exception.returncode
-  print "\n[  FAILED  ] Command%s: %s" % (exit_code, " ".join(command_line))
-  print 72 * "-"
+    exit_code = ' (exit code %d)' % exception.returncode
+  print '\n[  FAILED  ] Command%s: %s' % (exit_code, ' '.join(command_line))
+  print 72 * '-'
   if hasattr(exception, 'output'):
     print exception.output
   print str(exception)
-  print 72 * "-"
+  print 72 * '-'
 
 
 def _build_command_line(config, args, apptest):
-  """Build the apptest command line. This value isn't executed on Android."""
-  paths = Paths(config)
-  # On Linux, always run tests with xvfb, but not for --gtest_list_tests.
-  not_list_tests = not "--gtest_list_tests" in args
-  use_xvfb = config.target_os == Config.OS_LINUX and not_list_tests
-  xvfb_prefix = [paths.xvfb, paths.build_dir] if use_xvfb else []
-  data_dir = ["--use-temporary-user-data-dir"] if not_list_tests else []
-  return xvfb_prefix + [paths.mojo_runner] + data_dir + args + [apptest]
+  '''Build the apptest command line. This value isn't executed on Android.'''
+  not_list_tests = not '--gtest_list_tests' in args
+  data_dir = ['--use-temporary-user-data-dir'] if not_list_tests else []
+  return [Paths(config).mojo_runner] + data_dir + args + [apptest]
+
+
+def _run_test_with_xvfb(config, shell, args, apptest):
+  '''Run the test with xvfb; return the output or raise an exception.'''
+  # Prepending testing/xvfb.py to the command line precludes direct control of
+  # the test subprocess, and prevents easily getting output when tests timeout.
+  xvfb_proc = None
+  openbox_proc = None
+  env = os.environ.copy()
+  if '--gtest_list_tests' not in args and xvfb.should_start_xvfb(env):
+    (xvfb_proc, openbox_proc) = xvfb.start_xvfb(env, Paths(config).build_dir)
+    if not xvfb_proc or not xvfb_proc.pid:
+      raise Exception('Xvfb failed to start; aborting test run.')
+    if not openbox_proc or not openbox_proc.pid:
+      raise Exception('Openbox failed to start; aborting test run.')
+    logging.getLogger().debug('Running with Xvfb and Openbox.')
+
+  output = _run_test_with_timeout(config, shell, args, apptest, env)
+  xvfb.kill(xvfb_proc)
+  xvfb.kill(openbox_proc)
+  return output
 
 
 # TODO(msw): Determine proper test timeout durations (starting small).
-def _run_test_with_timeout(config, shell, args, apptest, timeout_in_seconds=10):
-  """Run the test with a timeout; return the output or raise an exception."""
+def _run_test_with_timeout(config, shell, args, apptest, env, seconds=10):
+  '''Run the test with a timeout; return the output or raise an exception.'''
   result = Queue.Queue()
   thread = threading.Thread(target=_run_test,
-                            args=(config, shell, args, apptest, result))
+                            args=(config, shell, args, apptest, env, result))
   thread.start()
   process_or_shell = result.get()
-  thread.join(timeout_in_seconds)
+  thread.join(seconds)
+  timeout_exception = ''
 
   if thread.is_alive():
+    timeout_exception = '\nError: Test timeout after %s seconds' % seconds
+    logging.getLogger().debug('Killing the runner or shell for timeout.')
     try:
       process_or_shell.kill()
     except OSError:
       pass  # The process may have ended after checking |is_alive|.
-    return "Error: Test timeout after %s seconds" % timeout_in_seconds
-
-  if not result.empty():
-    (output, exception) = result.get()
-    if exception:
-      raise Exception("%s%s%s" % (output, "\n" if output else "", exception))
-    return output
-
-  return "Error: Test exited with no output."
 
-
-def _run_test(config, shell, args, apptest, result):
-  """Run the test and put the output and any exception in |result|."""
-  output = ""
-  exception = ""
+  thread.join(seconds)
+  if thread.is_alive():
+    raise Exception('Error: Test hung and could not be killed!')
+  if result.empty():
+    raise Exception('Error: Test exited with no output.')
+  (output, exception) = result.get()
+  exception += timeout_exception
+  if exception:
+    raise Exception('%s%s%s' % (output, '\n' if output else '', exception))
+  return output
+
+
+def _run_test(config, shell, args, apptest, env, result):
+  '''Run the test; put the shell/proc, output and any exception in |result|.'''
+  output = ''
+  exception = ''
   try:
     if (config.target_os != Config.OS_ANDROID):
       command = _build_command_line(config, args, apptest)
       process = subprocess.Popen(command, stdout=subprocess.PIPE,
-                                 stderr=subprocess.PIPE)
+                                 stderr=subprocess.PIPE, env=env)
       result.put(process)
-      process.wait()
-      if not process.poll():
-        output = str(process.stdout.read())
-      else:
-        exception = "Error: Test exited with code: %d" % process.returncode
+      output = process.communicate()[0]
+      if process.returncode:
+        exception = 'Error: Test exited with code: %d' % process.returncode
     else:
       assert shell
       result.put(shell)
       (r, w) = os.pipe()
-      with os.fdopen(r, "r") as rf:
-        with os.fdopen(w, "w") as wf:
+      with os.fdopen(r, 'r') as rf:
+        with os.fdopen(w, 'w') as wf:
           arguments = args + [apptest]
           shell.StartActivity('MojoShellActivity', arguments, wf, wf.close)
           output = rf.read()
   except Exception as e:
-    output = e.output if hasattr(e, 'output') else ""
-    exception = str(e)
+    output += (e.output + '\n') if hasattr(e, 'output') else ''
+    exception += str(e)
   result.put((output, exception))
diff --git a/mojo/tools/mopy/paths.py b/mojo/tools/mopy/paths.py
index 7677329..ed254f4 100644
--- a/mojo/tools/mopy/paths.py
+++ b/mojo/tools/mopy/paths.py
@@ -7,44 +7,44 @@ import os
 from .config import Config
 
 
+SRC_ROOT = os.path.abspath(os.path.join(__file__, '..', '..', '..', '..'))
+
 class Paths(object):
-  """Provides commonly used paths"""
+  '''Provides commonly used paths'''
 
   def __init__(self, config):
-    """Generate paths to binary artifacts from a Config object."""
-    self.src_root = os.path.abspath(os.path.join(__file__,
-      os.pardir, os.pardir, os.pardir, os.pardir))
-    self.mojo_dir = os.path.join(self.src_root, "mojo")
+    '''Generate paths to binary artifacts from a Config object.'''
+    self.src_root = SRC_ROOT
+    self.mojo_dir = os.path.join(self.src_root, 'mojo')
 
     self.build_dir = config.build_dir
     if self.build_dir is None:
-      subdir = ""
+      subdir = ''
       if config.target_os == Config.OS_ANDROID:
-        subdir += "android_"
+        subdir += 'android_'
         if config.target_cpu != Config.ARCH_ARM:
-          subdir += config.target_cpu + "_"
+          subdir += config.target_cpu + '_'
       elif config.target_os == Config.OS_CHROMEOS:
-        subdir += "chromeos_"
-      subdir += "Debug" if config.is_debug else "Release"
+        subdir += 'chromeos_'
+      subdir += 'Debug' if config.is_debug else 'Release'
       if config.is_asan:
-        subdir += "_asan"
+        subdir += '_asan'
       if not(config.is_debug) and config.dcheck_always_on:
-        subdir += "_dcheck"
-      self.build_dir = os.path.join(self.src_root, "out", subdir)
+        subdir += '_dcheck'
+      self.build_dir = os.path.join(self.src_root, 'out', subdir)
 
-    self.mojo_runner = os.path.join(self.build_dir, "mojo_runner")
+    self.mojo_runner = os.path.join(self.build_dir, 'mojo_runner')
     if config.target_os == Config.OS_WINDOWS:
-      self.mojo_runner += ".exe"
+      self.mojo_runner += '.exe'
     if config.target_os == Config.OS_ANDROID:
-      self.apk_path = os.path.join(self.build_dir, "apks", config.apk_name)
-      self.mojo_runner = os.path.join(self.src_root, "mojo", "tools",
-                                      "android_mojo_shell.py")
-    self.xvfb = os.path.join(self.src_root, "testing", "xvfb.py")
+      self.apk_path = os.path.join(self.build_dir, 'apks', config.apk_name)
+      self.mojo_runner = os.path.join(self.src_root, 'mojo', 'tools',
+                                      'android_mojo_shell.py')
 
   def RelPath(self, path):
-    """Returns the given path, relative to the current directory."""
+    '''Returns the given path, relative to the current directory.'''
     return os.path.relpath(path)
 
   def SrcRelPath(self, path):
-    """Returns the given path, relative to self.src_root."""
+    '''Returns the given path, relative to self.src_root.'''
     return os.path.relpath(path, self.src_root)
diff --git a/mojo/tools/rev_sdk.py b/mojo/tools/rev_sdk.py
index db81034..d3d14c7 100755
--- a/mojo/tools/rev_sdk.py
+++ b/mojo/tools/rev_sdk.py
@@ -3,9 +3,9 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-"""Tool to roll Mojo into Chromium. See:
+'''Tool to roll Mojo into Chromium. See:
 https://github.com/domokit/mojo/wiki/Rolling-code-between-chromium-and-mojo#mojo---chromium-updates-sdk--edk
-"""
+'''
 
 import os
 import sys
@@ -13,28 +13,28 @@ from utils import commit
 from utils import chromium_root_dir
 from utils import system
 
-sdk_prefix_in_chromium = "third_party/mojo/src"
+sdk_prefix_in_chromium = 'third_party/mojo/src'
 sdk_dirs_to_clone = [
-  "mojo/edk",
-  "mojo/public",
-  "nacl_bindings",
+  'mojo/edk',
+  'mojo/public',
+  'nacl_bindings',
 ]
 
 sdk_dirs_to_not_clone = [
-  "mojo/public/cpp/application",
-  "mojo/public/interfaces/application",
-  "mojo/public/interfaces/network",
-  "mojo/public/java/application",
+  'mojo/public/cpp/application',
+  'mojo/public/interfaces/application',
+  'mojo/public/interfaces/network',
+  'mojo/public/java/application',
 ]
 
 # Individual files to preserve within the target repository during roll. These
 # are relative to |sdk_prefix_in_chromium| but are not maintained in the mojo
 # repository.
 preserved_chromium_files = [
-  "mojo/edk/DEPS",
-  "mojo/public/DEPS",
-  "mojo/public/platform/nacl/DEPS",
-  "nacl_bindings/DEPS",
+  'mojo/edk/DEPS',
+  'mojo/public/DEPS',
+  'mojo/public/platform/nacl/DEPS',
+  'nacl_bindings/DEPS',
 ]
 
 # A dictionary mapping dirs to clone to their destination locations in Chromium.
@@ -45,19 +45,19 @@ for sdk_dir in sdk_dirs_to_clone:
   dirs_to_clone[sdk_dir] = sdk_dir_in_chromium
 
 def rev(source_dir, chromium_dir, mojo_revision):
-  src_commit = system(["git", "rev-parse", mojo_revision],
+  src_commit = system(['git', 'rev-parse', mojo_revision],
                       cwd=source_dir).strip()
 
   for input_dir, dest_dir in dirs_to_clone.iteritems():
     if os.path.exists(os.path.join(chromium_dir, dest_dir)):
-      print "removing directory %s" % dest_dir
-      system(["git", "rm", "-r", dest_dir], cwd=chromium_dir)
-    print "cloning directory %s into %s" % (input_dir, dest_dir)
-    files = system(["git", "ls-files", input_dir], cwd=source_dir)
+      print 'removing directory %s' % dest_dir
+      system(['git', 'rm', '-r', dest_dir], cwd=chromium_dir)
+    print 'cloning directory %s into %s' % (input_dir, dest_dir)
+    files = system(['git', 'ls-files', input_dir], cwd=source_dir)
     for f in files.splitlines():
       # Don't copy presubmit files over since the code is read-only on the
       # chromium side.
-      if os.path.basename(f) == "PRESUBMIT.py":
+      if os.path.basename(f) == 'PRESUBMIT.py':
         continue
 
       exclude = False
@@ -72,27 +72,27 @@ def rev(source_dir, chromium_dir, mojo_revision):
       # |input_dir|.
       f_relpath = os.path.relpath(f, input_dir)
       dest_path = os.path.join(chromium_dir, dest_dir, f_relpath)
-      system(["mkdir", "-p", os.path.dirname(dest_path)])
-      system(["cp", os.path.join(source_dir, f), dest_path])
+      system(['mkdir', '-p', os.path.dirname(dest_path)])
+      system(['cp', os.path.join(source_dir, f), dest_path])
     os.chdir(chromium_dir)
-    system(["git", "add", dest_dir], cwd=chromium_dir)
+    system(['git', 'add', dest_dir], cwd=chromium_dir)
 
-  mojo_public_dest_dir = os.path.join(sdk_prefix_in_chromium, "mojo/public")
-  version_filename = os.path.join(mojo_public_dest_dir, "VERSION")
-  with open(version_filename, "w") as version_file:
+  mojo_public_dest_dir = os.path.join(sdk_prefix_in_chromium, 'mojo/public')
+  version_filename = os.path.join(mojo_public_dest_dir, 'VERSION')
+  with open(version_filename, 'w') as version_file:
     version_file.write(src_commit)
-  system(["git", "add", version_filename], cwd=chromium_dir)
+  system(['git', 'add', version_filename], cwd=chromium_dir)
 
   # Reset preserved files that were blown away.
   for rel_path in preserved_chromium_files:
     preserved_path = os.path.join(sdk_prefix_in_chromium, rel_path)
-    system(["git", "reset", "--", preserved_path])
-    system(["git", "checkout", preserved_path])
+    system(['git', 'reset', '--', preserved_path])
+    system(['git', 'checkout', preserved_path])
 
-  commit("Update mojo sdk to rev " + src_commit, cwd=chromium_dir)
+  commit('Update mojo sdk to rev ' + src_commit, cwd=chromium_dir)
 
 if len(sys.argv) < 2:
-  print "usage: rev_sdk.py <mojo source dir> [<mojo revision>]"
+  print 'usage: rev_sdk.py <mojo source dir> [<mojo revision>]'
   sys.exit(1)
 
 # Allow override of the roll revision.