From 286adf5ad68b88df9028ab8239ae55c5b2eab81c Mon Sep 17 00:00:00 2001 From: "cbentzel@chromium.org" Date: Wed, 27 Oct 2010 15:11:46 +0000 Subject: Revert 64065 - testserver.py listens on ephemeral ports by default. I'm not convinced that this caused some of the test failures, but I needed to leave unexpectedly and didn't have time to investigate. If --port is specified on the command line, testserver.py will listen on that port, otherwise it will listen on an ephemeral port. If --startup_pipe is specified, the port number is written to the pipe as a 2 byte unsigned int in host order. TestServer by default spawns testserver.py to listen on an ephemeral port and reads the port value from the pipe. If necessary, the port can still be fixed using TestServer::ForcePort, but that will hopefully get deprecated quickly. BUG=56814 TEST=try bots pass Review URL: http://codereview.chromium.org/3549003 TBR=cbentzel@chromium.org Review URL: http://codereview.chromium.org/4165004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@64070 0039d316-1c4b-4281-b951-d872f2087c98 --- net/test/test_server.cc | 53 ++++++++++++++++++++++++++++++++++--------- net/test/test_server.h | 5 +--- net/test/test_server_posix.cc | 39 +++++++++++-------------------- net/test/test_server_win.cc | 21 ++++------------- 4 files changed, 60 insertions(+), 58 deletions(-) (limited to 'net/test') diff --git a/net/test/test_server.cc b/net/test/test_server.cc index 99cb4a6..0b1cd085 100644 --- a/net/test/test_server.cc +++ b/net/test/test_server.cc @@ -38,6 +38,46 @@ const int kServerConnectionAttempts = 10; // Connection timeout in milliseconds for tests. const int kServerConnectionTimeoutMs = 1000; +const char kTestServerShardFlag[] = "test-server-shard"; + +int GetPortBase(net::TestServer::Type type) { + switch (type) { + case net::TestServer::TYPE_FTP: + return 3117; + case net::TestServer::TYPE_HTTP: + return 1337; + case net::TestServer::TYPE_HTTPS: + return 9443; + case net::TestServer::TYPE_HTTPS_CLIENT_AUTH: + return 9543; + case net::TestServer::TYPE_HTTPS_EXPIRED_CERTIFICATE: + // TODO(phajdan.jr): Some tests rely on this hardcoded value. + // Some uses of this are actually in .html/.js files. + return 9666; + case net::TestServer::TYPE_HTTPS_MISMATCHED_HOSTNAME: + return 9643; + default: + NOTREACHED(); + } + return -1; +} + +int GetPort(net::TestServer::Type type) { + int port = GetPortBase(type); + if (CommandLine::ForCurrentProcess()->HasSwitch(kTestServerShardFlag)) { + std::string shard_str(CommandLine::ForCurrentProcess()->GetSwitchValueASCII( + kTestServerShardFlag)); + int shard = -1; + if (base::StringToInt(shard_str, &shard)) { + port += shard; + } else { + LOG(FATAL) << "Got invalid " << kTestServerShardFlag << " flag value. " + << "An integer is expected."; + } + } + return port; +} + std::string GetHostname(net::TestServer::Type type) { if (type == net::TestServer::TYPE_HTTPS_MISMATCHED_HOSTNAME) { // Return a different hostname string that resolves to the same hostname. @@ -56,10 +96,9 @@ void SetMacTestCertificate(X509Certificate* cert); #endif TestServer::TestServer(Type type, const FilePath& document_root) - : host_port_pair_(GetHostname(type), 0), + : host_port_pair_(GetHostname(type), GetPort(type)), process_handle_(base::kNullProcessHandle), - type_(type), - started_(false) { + type_(type) { FilePath src_dir; PathService::Get(base::DIR_SOURCE_ROOT, &src_dir); @@ -109,7 +148,6 @@ bool TestServer::Start() { return false; } - started_ = true; return true; } @@ -117,8 +155,6 @@ bool TestServer::Stop() { if (!process_handle_) return true; - started_ = false; - // First check if the process has already terminated. bool ret = base::WaitForSingleProcess(process_handle_, 0); if (!ret) @@ -134,11 +170,6 @@ bool TestServer::Stop() { return ret; } -const HostPortPair& TestServer::host_port_pair() const { - DCHECK(started_); - return host_port_pair_; -} - std::string TestServer::GetScheme() const { switch (type_) { case TYPE_FTP: diff --git a/net/test/test_server.h b/net/test/test_server.h index 2ee801e..4e68fd9 100644 --- a/net/test/test_server.h +++ b/net/test/test_server.h @@ -53,7 +53,7 @@ class TestServer { bool Stop(); const FilePath& document_root() const { return document_root_; } - const HostPortPair& host_port_pair() const; + const HostPortPair& host_port_pair() const { return host_port_pair_; } std::string GetScheme() const; bool GetAddressList(AddressList* address_list) const WARN_UNUSED_RESULT; @@ -121,9 +121,6 @@ class TestServer { Type type_; - // Has the server been started? - bool started_; - DISALLOW_COPY_AND_ASSIGN(TestServer); }; diff --git a/net/test/test_server_posix.cc b/net/test/test_server_posix.cc index 17b2631..1456ac8 100644 --- a/net/test/test_server_posix.cc +++ b/net/test/test_server_posix.cc @@ -110,37 +110,24 @@ bool TestServer::LaunchPython(const FilePath& testserver_path) { } bool TestServer::WaitToStart() { - uint16 port; - uint8* buffer = reinterpret_cast(&port); - ssize_t bytes_read = 0; - ssize_t bytes_max = sizeof(port); - while (bytes_read < bytes_max) { - struct pollfd poll_fds[1]; - - poll_fds[0].fd = child_fd_; - poll_fds[0].events = POLLIN | POLLPRI; - poll_fds[0].revents = 0; - - int rv = HANDLE_EINTR(poll(poll_fds, 1, - TestTimeouts::action_max_timeout_ms())); - if (rv != 1) { - LOG(ERROR) << "Failed to poll for the child file descriptor."; - return false; - } + struct pollfd poll_fds[1]; + + poll_fds[0].fd = child_fd_; + poll_fds[0].events = POLLIN | POLLPRI; + poll_fds[0].revents = 0; - ssize_t num_bytes = HANDLE_EINTR(read(child_fd_, buffer + bytes_read, - bytes_max - bytes_read)); - if (num_bytes <= 0) - break; - bytes_read += num_bytes; + int rv = HANDLE_EINTR(poll(poll_fds, 1, + TestTimeouts::action_max_timeout_ms())); + if (rv != 1) { + LOG(ERROR) << "Failed to poll for the child file descriptor."; + return false; } + char buf[8]; + ssize_t n = HANDLE_EINTR(read(child_fd_, buf, sizeof(buf))); // We don't need the FD anymore. child_fd_closer_.reset(NULL); - if (bytes_read < bytes_max) - return false; - host_port_pair_.set_port(port); - return true; + return n > 0; } bool TestServer::CheckCATrusted() { diff --git a/net/test/test_server_win.cc b/net/test/test_server_win.cc index cfc7178..a8b3678 100644 --- a/net/test/test_server_win.cc +++ b/net/test/test_server_win.cc @@ -149,24 +149,11 @@ bool TestServer::LaunchPython(const FilePath& testserver_path) { } bool TestServer::WaitToStart() { - uint16 port; - uint8* buffer = reinterpret_cast(&port); - DWORD bytes_read = 0; - DWORD bytes_max = sizeof(port); - while (bytes_read < bytes_max) { - DWORD num_bytes; - if (!ReadFile(child_fd_, buffer + bytes_read, bytes_max - bytes_read, - &num_bytes, NULL)) - break; - if (num_bytes <= 0) - break; - bytes_read += num_bytes; - } + char buf[8]; + DWORD bytes_read; + BOOL result = ReadFile(child_fd_, buf, sizeof(buf), &bytes_read, NULL); child_fd_.Close(); - if (bytes_read < bytes_max) - return false; - host_port_pair_.set_port(port); - return true; + return result && bytes_read > 0; } bool TestServer::CheckCATrusted() { -- cgit v1.1