[sync] Add --port and --xmpp-port parameters to run_testserver.cc

net/tools/testserver/run_testserver.cc

 // Copyright (c) 2012 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.
 
 #include <stdio.h>
 
 #include "base/at_exit.h"
 #include "base/command_line.h"
 #include "base/file_path.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/process_util.h"
 #include "base/test/test_timeouts.h"
+#include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "net/test/python_utils.h"
 #include "net/test/test_server.h"
+#include "net/test/local_sync_test_server.h"
 
 static void PrintUsage() {
   printf("run_testserver --doc-root=relpath [--http|--https|--ftp|--sync]\n"
-         "               [--https-cert=ok|mismatched-name|expired]\n");
-  printf("(NOTE: relpath should be relative to the 'src' directory)\n");
+         "               [--https-cert=ok|mismatched-name|expired]\n"
+         "               [--port=<port>] [--xmpp-port=<xmpp_port>]\n");
+  printf("(NOTE: relpath should be relative to the 'src' directory.\n");
+  printf("       --port and --xmpp-port only work with the --sync flag.)\n");
 }
 
 // Launches the chromiumsync_test script, testing the --sync functionality.
 static bool RunSyncTest() {
  if (!net::TestServer::SetPythonPath()) {
     LOG(ERROR) << "Error trying to set python path. Exiting.";
     return false;
   }
 
   FilePath sync_test_path;
@@ skipping 48 matching lines @@
   if (command_line->HasSwitch("https")) {
     server_type = net::TestServer::TYPE_HTTPS;
   } else if (command_line->HasSwitch("ftp")) {
     server_type = net::TestServer::TYPE_FTP;
   } else if (command_line->HasSwitch("sync")) {
     server_type = net::TestServer::TYPE_SYNC;
   } else if (command_line->HasSwitch("sync-test")) {
     return RunSyncTest() ? 0 : -1;
   }
 
+  std::string port_str = "0";

[akalin, 2012/05/23 05:29:21]

move port_str inside the if statement and don't initialize it to some value. 
Also maybe do something if the conversion fails? (log + use default value?) 
Something like:

int port = 0;
if (command_line->HasSwitch("port")) {
  const std::string& port_str = command_line->GetSwitchValueASCII("port");
  if (!base::StringToInt(port_str, &port)) {
    port = 0;
    LOG(WARNING) << "...";
  }
}

you may want to decomp it into a helper function since you use it twice, also.

[Raghu Simha, 2012/05/23 06:02:59]

Done.

+  int port_int = 0;
+  if (command_line->HasSwitch("port")) {
+    port_str = command_line->GetSwitchValueASCII("port");
+    base::StringToInt(port_str, &port_int);
+  }
+
+  std::string xmpp_port_str = "0";
+  int xmpp_port_int = 0;
+  if (command_line->HasSwitch("xmpp-port")) {
+    xmpp_port_str = command_line->GetSwitchValueASCII("xmpp-port");
+    base::StringToInt(xmpp_port_str, &xmpp_port_int);
+  }
+
   net::TestServer::HTTPSOptions https_options;
   if (command_line->HasSwitch("https-cert")) {
     server_type = net::TestServer::TYPE_HTTPS;
     std::string cert_option = command_line->GetSwitchValueASCII("https-cert");
     if (cert_option == "ok") {
       https_options.server_certificate = net::TestServer::HTTPSOptions::CERT_OK;
     } else if (cert_option == "mismatched-name") {
       https_options.server_certificate =
           net::TestServer::HTTPSOptions::CERT_MISMATCHED_NAME;
     } else if (cert_option == "expired") {
       https_options.server_certificate =
           net::TestServer::HTTPSOptions::CERT_EXPIRED;
     } else {
       printf("Error: --https-cert has invalid value %s\n", cert_option.c_str());
       PrintUsage();
       return -1;
     }
   }
 
   FilePath doc_root = command_line->GetSwitchValuePath("doc-root");
   if ((server_type != net::TestServer::TYPE_SYNC) && doc_root.empty()) {
     printf("Error: --doc-root must be specified\n");
     PrintUsage();
     return -1;
   }
 
   scoped_ptr<net::TestServer> test_server;
-  if (server_type == net::TestServer::TYPE_HTTPS)
-    test_server.reset(new net::TestServer(https_options, doc_root));
-  else
-    test_server.reset(new net::TestServer(server_type,
-                                          net::TestServer::kLocalhost,
-                                          doc_root));
+  switch (server_type) {
+    case net::TestServer::TYPE_HTTPS:
+      test_server.reset(new net::TestServer(https_options, doc_root));
+      break;
+    case net::TestServer::TYPE_SYNC:
+      test_server.reset(new net::LocalSyncTestServer((uint16) port_int,

[akalin, 2012/05/23 05:29:21]

prefer static_cast<uint16>()

[akalin, 2012/05/23 05:29:21]

in fact, you can probably stick the int parsing code here since it's only needed
for sync stuff.  you may need to put braces around the case block (since you're
defining new variables).  Or decomp it out to a separate function.

[Raghu Simha, 2012/05/23 06:02:59]

Done.

[Raghu Simha, 2012/05/23 06:02:59]

Done. Moved to a separate function. Added braces in any case.

+                                                     (uint16) xmpp_port_int));
+      break;
+    default:
+      test_server.reset(new net::TestServer(server_type,
+                                            net::TestServer::kLocalhost,
+                                            doc_root));
+      break;
+  }
 
   if (!test_server->Start()) {
     printf("Error: failed to start test server. Exiting.\n");
     return -1;
   }
 
   if (!file_util::DirectoryExists(test_server->document_root())) {
     printf("Error: invalid doc root: \"%s\" does not exist!\n",
         UTF16ToUTF8(test_server->document_root().LossyDisplayName()).c_str());
     return -1;
   }
 
   printf("testserver running at %s (type ctrl+c to exit)\n",
          test_server->host_port_pair().ToString().c_str());
 
   message_loop.Run();
   return 0;
 }