GTTF: convert some tests in chrome to use EmbeddedTestServer patch nr 1

chrome/browser/extensions/extension_apitest.cc

Index: chrome/browser/extensions/extension_apitest.cc
diff --git a/chrome/browser/extensions/extension_apitest.cc b/chrome/browser/extensions/extension_apitest.cc
index c14182b8a5697b715107a58d4cf0c52a24977fc2..621028fbab1de816692ccc03b443b2b827523b99 100644
--- a/chrome/browser/extensions/extension_apitest.cc
+++ b/chrome/browser/extensions/extension_apitest.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/extensions/extension_apitest.h"
 
+#include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/extensions/api/test/test_api.h"
@@ -18,7 +19,11 @@
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_service.h"
+#include "net/base/escape.h"
 #include "net/base/net_util.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/http_response.h"
+#include "net/test/embedded_test_server/http_request.h"
 #include "net/test/spawned_test_server/spawned_test_server.h"
 
 namespace {
@@ -28,9 +33,108 @@ const char kTestServerPort[] = "testServer.port";
 const char kTestDataDirectory[] = "testDataDirectory";
 const char kTestWebSocketPort[] = "testWebSocketPort";
 
+scoped_ptr<net::test_server::HttpResponse> HandleServerRedirectRequest(
+    const net::test_server::HttpRequest& request) {
+  if (!StartsWithASCII(request.relative_url, "/server-redirect?", true))
+    return scoped_ptr<net::test_server::HttpResponse>(NULL);
+
+  size_t query_string_pos = request.relative_url.find('?');
+  std::string redirect_target =
+      request.relative_url.substr(query_string_pos + 1);
+
+  scoped_ptr<net::test_server::BasicHttpResponse> http_response(
+      new net::test_server::BasicHttpResponse);
+  http_response->set_code(net::HTTP_MOVED_PERMANENTLY);
+  http_response->AddCustomHeader("Location", redirect_target);
+  return http_response.PassAs<net::test_server::HttpResponse>();
+}
+
+scoped_ptr<net::test_server::HttpResponse> HandleEchoHeaderRequest(
+    const net::test_server::HttpRequest& request) {
+  if (!StartsWithASCII(request.relative_url, "/echoheader?", true))
+    return scoped_ptr<net::test_server::HttpResponse>(NULL);
+
+  size_t query_string_pos = request.relative_url.find('?');
+  std::string header_name =
+      request.relative_url.substr(query_string_pos + 1);
+
+  std::string header_value;
+  std::map<std::string, std::string>::const_iterator it = request.headers.find(
+      header_name);
+  if (it != request.headers.end())
+    header_value = it->second;
+
+  scoped_ptr<net::test_server::BasicHttpResponse> http_response(
+      new net::test_server::BasicHttpResponse);
+  http_response->set_code(net::HTTP_OK);
+  http_response->set_content(header_value);
+  return http_response.PassAs<net::test_server::HttpResponse>();
+}
+
+scoped_ptr<net::test_server::HttpResponse> HandleSetCookieRequest(
+    const net::test_server::HttpRequest& request) {
+  if (!StartsWithASCII(request.relative_url, "/set-cookie?", true))
+    return scoped_ptr<net::test_server::HttpResponse>(NULL);
+
+  scoped_ptr<net::test_server::BasicHttpResponse> http_response(
+      new net::test_server::BasicHttpResponse);
+  http_response->set_code(net::HTTP_OK);
+
+  size_t query_string_pos = request.relative_url.find('?');
+  std::string cookie_value =
+      request.relative_url.substr(query_string_pos + 1);
+
+  std::vector<std::string> cookies;
+  base::SplitString(cookie_value, '&', &cookies);
+
+  for (size_t i = 0; i < cookies.size(); i++)
+    http_response->AddCustomHeader("Set-Cookie", cookies[i]);
+
+  return http_response.PassAs<net::test_server::HttpResponse>();
+}
+
+scoped_ptr<net::test_server::HttpResponse> HandleSetHeaderRequest(
+    const net::test_server::HttpRequest& request) {
+  if (!StartsWithASCII(request.relative_url, "/set-header?", true))
+    return scoped_ptr<net::test_server::HttpResponse>(NULL);
+
+  size_t query_string_pos = request.relative_url.find('?');
+  std::string escaped_header =
+      request.relative_url.substr(query_string_pos + 1);
+
+  std::string header =
+      net::UnescapeURLComponent(escaped_header,
+                                net::UnescapeRule::NORMAL |
+                                net::UnescapeRule::SPACES |
+                                net::UnescapeRule::URL_SPECIAL_CHARS);
+
+  size_t colon_pos = header.find(':');
+  if (colon_pos == std::string::npos)
+    return scoped_ptr<net::test_server::HttpResponse>(NULL);
+
+  std::string header_name = header.substr(0, colon_pos);
+  // Skip space after colon.
+  std::string header_value = header.substr(colon_pos + 2);
+
+  scoped_ptr<net::test_server::BasicHttpResponse> http_response(
+      new net::test_server::BasicHttpResponse);
+  http_response->set_code(net::HTTP_OK);
+  http_response->AddCustomHeader(header_name, header_value);
+  return http_response.PassAs<net::test_server::HttpResponse>();
+}
+

[asargent_no_longer_on_chrome, 2013/06/21 23:08:14]

observation: This whole block of new functions seems generally useful and not
particularly specific to ExtensionApiTest. Could it live in a more common place?

[Paweł Hajdan Jr., 2013/06/22 00:16:02]

I was thinking about it. For now I prefer it to be limited, so that other new
tests don't use it.

If there is a need, it will be moved, yes.

 };  // namespace
 
-ExtensionApiTest::ExtensionApiTest() {}
+ExtensionApiTest::ExtensionApiTest() {
+  embedded_test_server()->RegisterRequestHandler(
+      base::Bind(&HandleServerRedirectRequest));
+  embedded_test_server()->RegisterRequestHandler(
+      base::Bind(&HandleEchoHeaderRequest));
+  embedded_test_server()->RegisterRequestHandler(
+      base::Bind(&HandleSetCookieRequest));
+  embedded_test_server()->RegisterRequestHandler(
+      base::Bind(&HandleSetHeaderRequest));
+}
 
 ExtensionApiTest::~ExtensionApiTest() {}
 
@@ -306,14 +410,14 @@ const extensions::Extension* ExtensionApiTest::GetSingleLoadedExtension() {
 }
 
 bool ExtensionApiTest::StartTestServer() {
-  if (!test_server()->Start())

[asargent_no_longer_on_chrome, 2013/06/21 20:02:11]

It seems kind of strange to have a method called StartTestServer that no longer
starts the test server, leading to code in consumers like the following:

ASSERT_TRUE(StartTestServer());
ASSERT_TRUE(test_server()->Start());

which reads very awkwardly. 

If I'm understanding your change correctly, you're making this method now do
something like "initialize" the test server, but not start it. Would it make
sense to rename this method to InitializeTestServer, and then add a new
StartTestServer method which calls InitializeTestServer and then does the
test_server()->Start() call?

[Paweł Hajdan Jr., 2013/06/21 20:37:31]

There are two test servers here: test_server() - old, out of process, and
embedded_test_server() - new, in process one.

This change in code makes StartTestServer() start the embedded test server.
Tests that need the test_server() for any reason still call
test_server()->Start(), but I think it's an exception rather than rule.

[asargent_no_longer_on_chrome, 2013/06/21 23:08:14]

Ok, I see. Can we rename one of the two then? I don't care so much which one,
but right now it is confusing that the names are so similar. Perhaps something
like one of these two:

ASSERT_TRUE(StartEmbeddedTestServer());
ASSERT_TRUE(test_server()->Start());

or

ASSERT_TRUE(StartTestServer());
ASSERT_TRUE(external_test_server()->Start());

?

[Paweł Hajdan Jr., 2013/06/22 00:16:02]

Done.

+  if (!embedded_test_server()->InitializeAndWaitUntilReady())
     return false;
 
   // Build a dictionary of values that tests can use to build URLs that
   // access the test server and local file system.  Tests can see these values
   // using the extension API function chrome.test.getConfig().
   test_config_->SetInteger(kTestServerPort,
-                           test_server()->host_port_pair().port());
+                           embedded_test_server()->port());
 
   return true;
 }