Don't duplicate test server arguments in Android test setup.

build/android/pylib/chrome_test_server_spawner.py

Index: build/android/pylib/chrome_test_server_spawner.py
diff --git a/build/android/pylib/chrome_test_server_spawner.py b/build/android/pylib/chrome_test_server_spawner.py
index 08bfb4506a11d433ab172d45c1ffd4179d00ec20..9fb3c925e602014938295586743505cfb958a7ec 100644
--- a/build/android/pylib/chrome_test_server_spawner.py
+++ b/build/android/pylib/chrome_test_server_spawner.py
@@ -179,47 +179,36 @@ class TestServerThread(threading.Thread):
     """
     if self.command_line:
       return
-    # The following arguments must exist.
-    type_cmd = _GetServerTypeCommandLine(self.arguments['server-type'])
+
+    args_copy = dict(self.arguments)
+
+    # Translate the server type.
+    type_cmd = _GetServerTypeCommandLine(args_copy.pop('server-type'))
     if type_cmd:
       self.command_line.append(type_cmd)
-    self.command_line.append('--port=%d' % self.host_port)
+
     # Use a pipe to get the port given by the instance of Python test server
     # if the test does not specify the port.
+    assert self.host_port == args_copy['port']
     if self.host_port == 0:
       (self.pipe_in, self.pipe_out) = os.pipe()
       self.command_line.append('--startup-pipe=%d' % self.pipe_out)
-    self.command_line.append('--host=%s' % self.arguments['host'])
-    data_dir = self.arguments['data-dir'] or 'chrome/test/data'
+
+    # --data-dir gets mangled.
+    data_dir = args_copy.pop('data-dir') or 'chrome/test/data'
     if not os.path.isabs(data_dir):
       data_dir = os.path.join(constants.DIR_SOURCE_ROOT, data_dir)
     self.command_line.append('--data-dir=%s' % data_dir)
-    # The following arguments are optional depending on the individual test.
-    if self.arguments.has_key('log-to-console'):
-      self.command_line.append('--log-to-console')
-    if self.arguments.has_key('auth-token'):
-      self.command_line.append('--auth-token=%s' % self.arguments['auth-token'])
-    if self.arguments.has_key('https'):
-      self.command_line.append('--https')
-      if self.arguments.has_key('cert-and-key-file'):

[bulach, 2014/04/22 09:09:48]

nit: just want to point out: these arguments would previously only be added if
it had --https, with the new implementation they'd always be included. I think
this won't be problem :) but just wanted to highlight..

[davidben, 2014/04/22 16:23:13]

The C++ side contains that logic anyway. It shouldn't be a problem. :-)

Though, I did notice that I missed some of the other filename munging, so I've
updated that. The missing data-dir case is no longer handled, but
BaseTestServer::GenerateArguments always includes it anyway.

-        self.command_line.append('--cert-and-key-file=%s' % os.path.join(
-            constants.DIR_SOURCE_ROOT, self.arguments['cert-and-key-file']))
-      if self.arguments.has_key('ocsp'):
-        self.command_line.append('--ocsp=%s' % self.arguments['ocsp'])
-      if self.arguments.has_key('https-record-resume'):
-        self.command_line.append('--https-record-resume')
-      if self.arguments.has_key('ssl-client-auth'):
-        self.command_line.append('--ssl-client-auth')
-      if self.arguments.has_key('tls-intolerant'):
-        self.command_line.append('--tls-intolerant=%s' %
-                                 self.arguments['tls-intolerant'])
-      if self.arguments.has_key('ssl-client-ca'):
-        for ca in self.arguments['ssl-client-ca']:
-          self.command_line.append('--ssl-client-ca=%s' %
-                                   os.path.join(constants.DIR_SOURCE_ROOT, ca))
-      if self.arguments.has_key('ssl-bulk-cipher'):
-        for bulk_cipher in self.arguments['ssl-bulk-cipher']:
-          self.command_line.append('--ssl-bulk-cipher=%s' % bulk_cipher)
+
+    # Pass the remaining arguments as-is.
+    for key, values in args_copy.iteritems():
+      if not isinstance(values, list):
+        values = [values]
+      for value in values:
+        if value is None:

[bulach, 2014/04/22 09:09:48]

nit: no need for "is None":
if value:
  self.command_line.append('--%s=%s' % (key, value))
else:
  self.command_line.append('--%s' % key)

[davidben, 2014/04/22 16:23:13]

That would probably cause troubles if value is 0.

+          self.command_line.append('--%s' % key)
+        else:
+          self.command_line.append('--%s=%s' % (key, value))
 
   def _CloseUnnecessaryFDsForTestServerProcess(self):
     # This is required to avoid subtle deadlocks that could be caused by the