google_apis: Add GetURL() to test_server::HttpServer

chrome/browser/google_apis/test_server/http_server_unittest.cc

Index: chrome/browser/google_apis/test_server/http_server_unittest.cc
diff --git a/chrome/browser/google_apis/test_server/http_server_unittest.cc b/chrome/browser/google_apis/test_server/http_server_unittest.cc
index 2fd1f51ff5ee6abe813b2b090166ec937f2f0cb3..9e5492ae416b3625999429d82ea765236a3b0ae8 100644
--- a/chrome/browser/google_apis/test_server/http_server_unittest.cc
+++ b/chrome/browser/google_apis/test_server/http_server_unittest.cc
@@ -4,9 +4,12 @@
 
 #include "chrome/browser/google_apis/test_server/http_server.h"
 
+#include "base/stringprintf.h"
 #include "base/threading/thread.h"
+#include "chrome/browser/google_apis/test_util.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_browser_thread.h"
+#include "net/http/http_response_headers.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "net/url_request/url_request_test_util.h"
@@ -17,23 +20,35 @@ namespace test_server {
 
 namespace {
 
-// Helper function to receive content of a response returned after invoking
-// |fetcher|.
-std::string GetFetcherResponseContent(const net::URLFetcher* fetcher) {
+// Gets the content from the given URLFetcher.
+std::string GetContentFromFetcher(const net::URLFetcher& fetcher) {
   std::string result;
-  const bool success = fetcher->GetResponseAsString(&result);
+  const bool success = fetcher.GetResponseAsString(&result);
   EXPECT_TRUE(success);
   return result;
 }
 
+// Gets the content type from the given URLFetcher.
+std::string GetContentTypeFromFetcher(const net::URLFetcher& fetcher) {
+  const net::HttpResponseHeaders* headers = fetcher.GetResponseHeaders();
+  if (headers) {
+    std::string content_type;
+    if (headers->GetMimeType(&content_type))
+      return content_type;
+  }
+  return "";
+}
+
 }  // namespace
 
 class HttpServerTest : public testing::Test,
                        public net::URLFetcherDelegate {
  public:
   HttpServerTest()
-    : ui_thread_(content::BrowserThread::UI, &message_loop_),
-      io_thread_(content::BrowserThread::IO) {
+      : num_responses_received_(0),
+        num_responses_expected_(0),
+        ui_thread_(content::BrowserThread::UI, &message_loop_),
+        io_thread_(content::BrowserThread::IO) {
   }
 
   virtual void SetUp() OVERRIDE {
@@ -43,102 +58,179 @@ class HttpServerTest : public testing::Test,
         content::BrowserThread::GetMessageLoopProxyForThread(
             content::BrowserThread::IO));
 
-    server_.reset(new HttpServer());
-    ASSERT_TRUE(server_->InitializeAndWaitUntilReady());
+    ASSERT_TRUE(server_.InitializeAndWaitUntilReady());
   }
 
   virtual void TearDown() OVERRIDE {
-    server_->ShutdownAndWaitUntilComplete();
+    server_.ShutdownAndWaitUntilComplete();
   }
 
   // net::URLFetcherDelegate override.
   virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE {
-    MessageLoop::current()->Quit();
+    ++num_responses_received_;
+    if (num_responses_received_ == num_responses_expected_)
+      message_loop_.Quit();
+  }
+
+  // Waits until the specified number of responses are received.
+  void WaitForResponses(int num_responses) {
+    num_responses_received_ = 0;
+    num_responses_expected_ = num_responses;
+    message_loop_.Run();  // Will be terminated in OnURLFetchComplete().
   }
 
  protected:
+  int num_responses_received_;
+  int num_responses_expected_;
   MessageLoopForUI message_loop_;
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread io_thread_;
   scoped_refptr<net::TestURLRequestContextGetter> request_context_getter_;
-  scoped_ptr<HttpServer> server_;
+  HttpServer server_;
 };
 
-TEST_F(HttpServerTest, TextRequest) {
-  // The simplest text response with an auto generated url.
-  GURL url1 = server_->RegisterTextResponse("test1",
-                                            "Raspberry chocolate",
-                                            "text/html",
-                                            SUCCESS);
-  ASSERT_NE("", url1.spec());
-
-  GURL url2 = server_->RegisterTextResponse("test2",
-                                            "Vanilla chocolate",
-                                            "text/html",
-                                            SUCCESS);
-  ASSERT_NE("", url2.spec());
-
-  // Response with a specified url and response code.
-  GURL url3 = server_->RegisterTextResponse(
-      "chocolate/bar.html",  // URL
-      "No chocolates",  // Dummy response text.
-      "text/plain",  // Content type.
-      NOT_FOUND);  // Response code (404 here).
-  ASSERT_NE("", url3.spec());
-
-  // Set up fetchers.
+TEST_F(HttpServerTest, GetBaseURL) {
+  EXPECT_EQ(base::StringPrintf("http://127.0.0.1:%d/", server_.port()),
+                               server_.GetBaseURL().spec());
+}
+
+TEST_F(HttpServerTest, GetURL) {
+  EXPECT_EQ(base::StringPrintf("http://127.0.0.1:%d/path?query=foo",
+                               server_.port()),
+            server_.GetURL("/path?query=foo").spec());
+}
+
+TEST_F(HttpServerTest, RegisterDefaultResponse) {
+  HttpResponse http_response;
+  http_response.set_code(MOVED);

[hashimoto, 2012/11/16 05:38:10]

Please add a brief description about why you chose 301 as the expected response
code for this test.

[satorux1, 2012/11/16 05:56:44]

Done.

+  http_response.set_content("<b>Moved!</b>");

[hashimoto, 2012/11/16 05:38:10]

How about making this data a constant?
The same goes for other tests.

[satorux1, 2012/11/16 05:56:44]

It's rather repetitive, but I think just using string literals is easier to
read.

+  http_response.set_content_type("text/html");

[hashimoto, 2012/11/16 05:38:10]

ditto.

+  http_response.AddCustomHeader("Server", "test server");

[hashimoto, 2012/11/16 05:38:10]

ditto.

+  server_.RegisterDefaultResponse("/test", http_response);

[hashimoto, 2012/11/16 05:38:10]

ditto.

+
+  scoped_ptr<net::URLFetcher> fetcher(
+      net::URLFetcher::Create(server_.GetURL("/test"),
+                              net::URLFetcher::GET,
+                              this));
+  fetcher->SetRequestContext(request_context_getter_.get());
+  fetcher->Start();
+  WaitForResponses(1);
+
+  EXPECT_EQ(net::URLRequestStatus::SUCCESS, fetcher->GetStatus().status());
+  EXPECT_EQ(301, fetcher->GetResponseCode());

[hashimoto, 2012/11/16 05:38:10]

Why are you using MOVED above and 301 here?
The same goes for other tests.

[satorux1, 2012/11/16 05:56:44]

Done.

+  EXPECT_EQ("<b>Moved!</b>", GetContentFromFetcher(*fetcher));
+  EXPECT_EQ("text/html", GetContentTypeFromFetcher(*fetcher));
+  const net::HttpResponseHeaders* headers = fetcher->GetResponseHeaders();
+  ASSERT_TRUE(headers);
+  ASSERT_TRUE(headers->HasHeaderValue("Server", "test server"));
+

[hashimoto, 2012/11/16 05:38:10]

nit: No need have a blank line here.

[satorux1, 2012/11/16 05:56:44]

Done.

+}
+
+TEST_F(HttpServerTest, RegisterTextResponse) {
+  server_.RegisterTextResponse("/test",
+                               "Raspberry chocolate",
+                               "text/plain",
+                               SUCCESS);
+
+  scoped_ptr<net::URLFetcher> fetcher(
+      net::URLFetcher::Create(server_.GetURL("/test"),
+                              net::URLFetcher::GET,
+                              this));
+  fetcher->SetRequestContext(request_context_getter_.get());
+  fetcher->Start();
+  WaitForResponses(1);
+
+  EXPECT_EQ(net::URLRequestStatus::SUCCESS, fetcher->GetStatus().status());
+  EXPECT_EQ(200, fetcher->GetResponseCode());
+  EXPECT_EQ("Raspberry chocolate", GetContentFromFetcher(*fetcher));
+  EXPECT_EQ("text/plain", GetContentTypeFromFetcher(*fetcher));
+}
+
+TEST_F(HttpServerTest, RegisterFileResponse) {
+  server_.RegisterFileResponse(
+      "/test",
+      test_util::GetTestFilePath("gdata/testfile.txt"),
+      "text/plain",
+      SUCCESS);
+
+  scoped_ptr<net::URLFetcher> fetcher(
+      net::URLFetcher::Create(server_.GetURL("/test"),
+                              net::URLFetcher::GET,
+                              this));
+  fetcher->SetRequestContext(request_context_getter_.get());
+  fetcher->Start();
+  WaitForResponses(1);
+
+  EXPECT_EQ(net::URLRequestStatus::SUCCESS, fetcher->GetStatus().status());
+  EXPECT_EQ(200, fetcher->GetResponseCode());
+  EXPECT_EQ("test file\n", GetContentFromFetcher(*fetcher));
+  EXPECT_EQ("text/plain", GetContentTypeFromFetcher(*fetcher));
+}
+
+TEST_F(HttpServerTest, DefaultNotFoundResponse) {
+  scoped_ptr<net::URLFetcher> fetcher(
+      net::URLFetcher::Create(server_.GetBaseURL(),

[hashimoto, 2012/11/16 05:38:10]

Instead of GetBaseURL, how about using something like GetURL("/non-existent")?
I think this would make the purpose of this test clearer.

[satorux1, 2012/11/16 05:56:44]

Done.

+                              net::URLFetcher::GET,
+                              this));
+  fetcher->SetRequestContext(request_context_getter_.get());
+
+  fetcher->Start();
+  WaitForResponses(1);
+  EXPECT_EQ(net::URLRequestStatus::SUCCESS, fetcher->GetStatus().status());
+  EXPECT_EQ(404, fetcher->GetResponseCode());
+}
+
+TEST_F(HttpServerTest, ConcurrentFetches) {
+  server_.RegisterTextResponse("/test1",
+                               "Raspberry chocolate",
+                               "text/html",
+                               SUCCESS);
+  server_.RegisterTextResponse("/test2",
+                               "Vanilla chocolate",
+                               "text/html",
+                               SUCCESS);
+  server_.RegisterTextResponse("/test3",
+                               "No chocolates",
+                               "text/plain",
+                               NOT_FOUND);
+
   scoped_ptr<net::URLFetcher> fetcher1 = scoped_ptr<net::URLFetcher>(
-      net::URLFetcher::Create(url1,
+      net::URLFetcher::Create(server_.GetURL("/test1"),
                               net::URLFetcher::GET,
                               this));
   fetcher1->SetRequestContext(request_context_getter_.get());
   scoped_ptr<net::URLFetcher> fetcher2 = scoped_ptr<net::URLFetcher>(
-      net::URLFetcher::Create(url2,
+      net::URLFetcher::Create(server_.GetURL("/test2"),
                               net::URLFetcher::GET,
                               this));
   fetcher2->SetRequestContext(request_context_getter_.get());
   scoped_ptr<net::URLFetcher> fetcher3 = scoped_ptr<net::URLFetcher>(
-      net::URLFetcher::Create(url3,
+      net::URLFetcher::Create(server_.GetURL("/test3"),
                               net::URLFetcher::GET,
                               this));
   fetcher3->SetRequestContext(request_context_getter_.get());
 
-  // Test.
+  // Fetch the three URLs concurrently.
   fetcher1->Start();
-  MessageLoop::current()->Run();
+  fetcher2->Start();
+  fetcher3->Start();
+  WaitForResponses(3);
+
   EXPECT_EQ(net::URLRequestStatus::SUCCESS, fetcher1->GetStatus().status());
   EXPECT_EQ(200, fetcher1->GetResponseCode());
-  EXPECT_EQ("Raspberry chocolate", GetFetcherResponseContent(fetcher1.get()));
+  EXPECT_EQ("Raspberry chocolate", GetContentFromFetcher(*fetcher1));
+  EXPECT_EQ("text/html", GetContentTypeFromFetcher(*fetcher1));
 
-  fetcher2->Start();
-  MessageLoop::current()->Run();
   EXPECT_EQ(net::URLRequestStatus::SUCCESS, fetcher1->GetStatus().status());

[hashimoto, 2012/11/16 05:38:10]

fetcher2?

[satorux1, 2012/11/16 05:56:44]

Good catch! Done.

   EXPECT_EQ(200, fetcher2->GetResponseCode());
-  EXPECT_EQ("Vanilla chocolate", GetFetcherResponseContent(fetcher2.get()));
+  EXPECT_EQ("Vanilla chocolate", GetContentFromFetcher(*fetcher2));
+  EXPECT_EQ("text/html", GetContentTypeFromFetcher(*fetcher2));
 
-  fetcher3->Start();
-  MessageLoop::current()->Run();
   EXPECT_EQ(net::URLRequestStatus::SUCCESS, fetcher1->GetStatus().status());

[hashimoto, 2012/11/16 05:38:10]

fetcher3?

[satorux1, 2012/11/16 05:56:44]

Done.

   EXPECT_EQ(404, fetcher3->GetResponseCode());
-  EXPECT_EQ("No chocolates", GetFetcherResponseContent(fetcher3.get()));
-}
-
-TEST_F(HttpServerTest, DefaultNotFoundResponse) {
-  ASSERT_NE("", server_->GetBaseURL().spec());
-
-  scoped_ptr<net::URLFetcher> fetcher = scoped_ptr<net::URLFetcher>(
-      net::URLFetcher::Create(server_->GetBaseURL(),
-                              net::URLFetcher::GET,
-                              this));
-  fetcher->SetRequestContext(request_context_getter_.get());
-
-  fetcher->Start();
-  MessageLoop::current()->Run();
-  EXPECT_EQ(net::URLRequestStatus::SUCCESS, fetcher->GetStatus().status());
-  EXPECT_EQ(404, fetcher->GetResponseCode());
+  EXPECT_EQ("No chocolates", GetContentFromFetcher(*fetcher3));
+  EXPECT_EQ("text/plain", GetContentTypeFromFetcher(*fetcher3));
 }
 
-// TODO(mtomasz): Write a test for a file response.
-
 }  // namespace test_server
 }  // namespace google_apis