Update test server spawner to run multiple test servers in parallel

Previously test server spawner didn't allow running more than one test
server. With this change it's now possible to run multiple tests that
depend on test_server in parallel.
 1. Updated chrome_test_launch_spawner.py to support more than one
    test server. /kill now requires a get parameter to specify which
    server should be killed. Number of concurrent test servers is
    specified by test runner which instantiates the spawner.
 2. Updated test runners for Android and Fuchsia to pass the new
    max_servers parameter for the spawner to match test parallelism
    on these platforms (1 on Android, 4 on Fuchsia).

Bug: 731302
Change-Id: I2e0e0c79dddb0b9f3745a444aae28e9235c3f126
Reviewed-on: https://chromium-review.googlesource.com/627339
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#497967}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: d15b34fd6a7c4775c6488fef1a4b7abd7f1c46e5

android/pylib/local/local_test_server_spawner.py

@@ -13,6 +13,11 @@ from pylib.constants import host_paths
 with host_paths.SysPath(host_paths.BUILD_COMMON_PATH):
   import chrome_test_server_spawner
 
+
+# The tests should not need more than one test server instance.
+MAX_TEST_SERVER_INSTANCES = 1
+
+
 def _WaitUntil(predicate, max_attempts=5):
   """Blocks until the provided predicate (function) is true.
 
@@ -58,7 +63,7 @@ class LocalTestServerSpawner(test_server.TestServer):
     super(LocalTestServerSpawner, self).__init__()
     self._device = device
     self._spawning_server = chrome_test_server_spawner.SpawningServer(
-        port, PortForwarderAndroid(device, tool))
+        port, PortForwarderAndroid(device, tool), MAX_TEST_SERVER_INSTANCES)
     self._tool = tool
 
   @property

fuchsia/runner_common.py

@@ -169,12 +169,6 @@ def BuildBootfs(output_directory, runtime_deps, bin_name, child_args,
   autorun_file = open(bin_name + '.bootfs_autorun', 'w')
   autorun_file.write('#!/bin/sh\n')
   if _IsRunningOnBot():
-    # We drop to -smp 1 to avoid counterintuitive observations on the realtime
-    # clock, but keep the concurrency at the default of 4. Insert at the
-    # beginning of the list so that if the real command line provides a specific
-    # value later, it will be used.
-    child_args.insert(0, '--test-launcher-jobs=4')
-
     # TODO(scottmg): Passed through for https://crbug.com/755282.
     autorun_file.write('export CHROME_HEADLESS=1\n')
 

fuchsia/test_runner.py

@@ -24,6 +24,10 @@ DIR_SOURCE_ROOT = os.path.abspath(
 sys.path.append(os.path.join(DIR_SOURCE_ROOT, 'build', 'util', 'lib', 'common'))
 import chrome_test_server_spawner
 
+# RunFuchsia() may run qemu with 1 or 4 CPUs. In both cases keep test
+# concurrency set to 4.
+DEFAULT_TEST_CONCURRENCY = 4
+
 
 def IsLocalPortAvailable(port):
   s = socket.socket()
@@ -137,9 +141,11 @@ def main():
   if args.test_launcher_batch_limit:
     child_args.append('--test-launcher-batch-limit=%d' %
                        args.test_launcher_batch_limit)
-  if args.test_launcher_jobs:
-    child_args.append('--test-launcher-jobs=%d' %
-                       args.test_launcher_jobs)
+
+  test_concurrency = args.test_launcher_jobs \
+      if args.test_launcher_jobs else DEFAULT_TEST_CONCURRENCY
+  child_args.append('--test-launcher-jobs=%d' % test_concurrency)
+
   if args.gtest_filter:
     child_args.append('--gtest_filter=' + args.gtest_filter)
   if args.gtest_repeat:
@@ -154,7 +160,7 @@ def main():
   # Start test server spawner for tests that need it.
   if args.enable_test_server:
     spawning_server = chrome_test_server_spawner.SpawningServer(
-        0, PortForwarderNoop())
+        0, PortForwarderNoop(), test_concurrency)
     spawning_server.Start()
 
     # Generate test server config.

util/lib/common/chrome_test_server_spawner.py

@@ -157,11 +157,13 @@ class TestServerThread(threading.Thread):
       return False
     logging.info('Got port json data: %s', port_json)
     port_json = json.loads(port_json)
-    if port_json.has_key('port') and isinstance(port_json['port'], int):
-      self.host_port = port_json['port']
-      return self.port_forwarder.WaitPortNotAvailable(self.host_port)
-    logging.error('Failed to get port information from the server data.')
-    return False
+
+    if not port_json.has_key('port') or not isinstance(port_json['port'], int):
+      logging.error('Failed to get port information from the server data.')
+      return False
+
+    self.host_port = port_json['port']
+    return self.port_forwarder.WaitPortNotAvailable(self.host_port)
 
   def _GenerateCommandLineArguments(self):
     """Generates the command line to run the test server.
@@ -315,43 +317,54 @@ class SpawningServerRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
     logging.info(content_length)
     test_server_argument_json = self.rfile.read(content_length)
     logging.info(test_server_argument_json)
-    # There should only be one test server instance at a time. However it may
-    # be possible that a previous instance was not cleaned up properly
-    # (crbug.com/665686)
-    if self.server.test_server_instance:
-      port = self.server.test_server_instance.host_port
-      logging.info('Killing lingering test server instance on port: %d', port)
-      self.server.test_server_instance.Stop()
-      self.server.test_server_instance = None
+
+    if len(self.server.test_servers) >= self.server.max_instances:
+      self._SendResponse(400, 'Invalid request', {},
+                         'Too many test servers running')
+      return
+
     ready_event = threading.Event()
-    self.server.test_server_instance = TestServerThread(
-        ready_event,
-        json.loads(test_server_argument_json),
-        self.server.port_forwarder)
-    self.server.test_server_instance.setDaemon(True)
-    self.server.test_server_instance.start()
+    new_server = TestServerThread(ready_event,
+                                  json.loads(test_server_argument_json),
+                                  self.server.port_forwarder)
+    new_server.setDaemon(True)
+    new_server.start()
     ready_event.wait()
-    if self.server.test_server_instance.is_ready:
+    if new_server.is_ready:
       self._SendResponse(200, 'OK', {}, json.dumps(
-          {'port': self.server.test_server_instance.forwarder_device_port,
+          {'port': new_server.forwarder_device_port,
            'message': 'started'}))
-      logging.info('Test server is running on port: %d.',
-                   self.server.test_server_instance.host_port)
+      port = new_server.host_port
+      logging.info('Test server is running on port: %d.' % port)
+      assert not self.server.test_servers.has_key(port)
+      self.server.test_servers[port] = new_server
     else:
-      self.server.test_server_instance.Stop()
-      self.server.test_server_instance = None
+      new_server.Stop()
       self._SendResponse(500, 'Test Server Error.', {}, '')
       logging.info('Encounter problem during starting a test server.')
 
-  def _KillTestServer(self):
+  def _KillTestServer(self, params):
     """Stops the test server instance."""
     # There should only ever be one test server at a time. This may do the
     # wrong thing if we try and start multiple test servers.
-    if not self.server.test_server_instance:
+    try:
+      port = int(params['port'][0])
+    except ValueError, KeyError:
+      port = None
+    if port == None or port <= 0:
+      self._SendResponse(400, 'Invalid request.', {}, 'port must be specified')
       return
-    port = self.server.test_server_instance.host_port
+
+    if not self.server.test_servers.has_key(port):
+      self._SendResponse(400, 'Invalid request.', {},
+                         "testserver isn't running on port %d" % port)
+      return
+
+    server = self.server.test_servers.pop(port)
+
     logging.info('Handling request to kill a test server on port: %d.', port)
-    self.server.test_server_instance.Stop()
+    server.Stop()
+
     # Make sure the status of test server is correct before sending response.
     if self.server.port_forwarder.WaitHostPortAvailable(port):
       self._SendResponse(200, 'OK', {}, 'killed')
@@ -359,7 +372,6 @@ class SpawningServerRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
     else:
       self._SendResponse(500, 'Test Server Error.', {}, '')
       logging.info('Encounter problem during killing a test server.')
-    self.server.test_server_instance = None
 
   def do_POST(self):
     parsed_path = urlparse.urlparse(self.path)
@@ -379,7 +391,7 @@ class SpawningServerRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
     for param in params:
       logging.info('%s=%s', param, params[param][0])
     if action == '/kill':
-      self._KillTestServer()
+      self._KillTestServer(params)
     elif action == '/ping':
       # The ping handler is used to check whether the spawner server is ready
       # to serve the requests. We don't need to test the status of the test
@@ -394,17 +406,18 @@ class SpawningServerRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
 class SpawningServer(object):
   """The class used to start/stop a http server."""
 
-  def __init__(self, test_server_spawner_port, port_forwarder):
+  def __init__(self, test_server_spawner_port, port_forwarder, max_instances):
     self.server = BaseHTTPServer.HTTPServer(('', test_server_spawner_port),
                                             SpawningServerRequestHandler)
     self.server_port = self.server.server_port
     logging.info('Started test server spawner on port: %d.', self.server_port)
 
     self.server.port_forwarder = port_forwarder
-    self.server.test_server_instance = None
+    self.server.test_servers = {}
+    self.server.max_instances = max_instances
 
   def _Listen(self):
-    logging.info('Starting test server spawner')
+    logging.info('Starting test server spawner.')
     self.server.serve_forever()
 
   def Start(self):
@@ -427,6 +440,9 @@ class SpawningServer(object):
     This should be called if the test server spawner is reused,
     to avoid sharing the test server instance.
     """
-    if self.server.test_server_instance:
-      self.server.test_server_instance.Stop()
-      self.server.test_server_instance = None
+    if self.server.test_servers:
+      logging.warning('Not all test servers were stopped.')
+      for port in self.server.test_servers:
+        logging.warning('Stopping test server on port %d' % port)
+        self.server.test_servers[port].Stop()
+      self.server.test_servers = []