Run safebrowsing_service_test through the net testserver code.

chrome/browser/safe_browsing/safe_browsing_test.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.
 //
 // This test uses the safebrowsing test server published at
 // http://code.google.com/p/google-safe-browsing/ to test the safebrowsing
 // protocol implemetation. Details of the safebrowsing testing flow is
 // documented at
 // http://code.google.com/p/google-safe-browsing/wiki/ProtocolTesting
 //
@@ skipping 25 matching lines @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/test/test_browser_thread.h"
 #include "net/base/host_resolver.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_log.h"
 #include "net/test/python_utils.h"
+#include "chrome/browser/safe_browsing/local_safebrowsing_test_server.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "net/url_request/url_request_status.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using content::BrowserThread;
 
 namespace {
 
 const FilePath::CharType kDataFile[] =
     FILE_PATH_LITERAL("testing_input_nomac.dat");
 const char kUrlVerifyPath[] = "/safebrowsing/verify_urls";
 const char kDBVerifyPath[] = "/safebrowsing/verify_database";
-const char kDBResetPath[] = "/reset";
 const char kTestCompletePath[] = "/test_complete";
 
 struct PhishingUrl {
   std::string url;
   std::string list_name;
   bool is_phishing;
 };
 
 // Parses server response for verify_urls. The expected format is:
 //
@@ skipping 30 matching lines @@
                  << ": " << record_parts[2];
       return false;
     }
     phishing_urls->push_back(phishing_url);
   }
   return true;
 }
 
 }  // namespace
 
-class SafeBrowsingTestServer {
- public:
-  explicit SafeBrowsingTestServer(const FilePath& datafile)
-      : datafile_(datafile),
-        server_handle_(base::kNullProcessHandle) {
-  }
-
-  ~SafeBrowsingTestServer() {
-    EXPECT_EQ(base::kNullProcessHandle, server_handle_);
-  }
-
-  // Start the python server test suite.
-  bool Start() {
-    // Get path to python server script
-    FilePath testserver_path;
-    if (!PathService::Get(base::DIR_SOURCE_ROOT, &testserver_path)) {
-      LOG(ERROR) << "Failed to get DIR_SOURCE_ROOT";
-      return false;
-    }
-    testserver_path = testserver_path
-        .Append(FILE_PATH_LITERAL("third_party"))
-        .Append(FILE_PATH_LITERAL("safe_browsing"))
-        .Append(FILE_PATH_LITERAL("testing"));
-    AppendToPythonPath(testserver_path);
-    FilePath testserver = testserver_path.Append(
-        FILE_PATH_LITERAL("safebrowsing_test_server.py"));
-
-    FilePath pyproto_code_dir;
-    if (!GetPyProtoPath(&pyproto_code_dir)) {
-      LOG(ERROR) << "Failed to get generated python protobuf dir";
-      return false;
-    }
-    AppendToPythonPath(pyproto_code_dir);
-    pyproto_code_dir = pyproto_code_dir.Append(FILE_PATH_LITERAL("google"));
-    AppendToPythonPath(pyproto_code_dir);
-
-    FilePath python_runtime;
-    EXPECT_TRUE(GetPythonRunTime(&python_runtime));
-    CommandLine cmd_line(python_runtime);
-    // Make python stdout and stderr unbuffered, to prevent incomplete stderr on
-    // win bots, and also fix mixed up ordering of stdout and stderr.
-    cmd_line.AppendSwitch("-u");
-    FilePath datafile = testserver_path.Append(datafile_);
-    cmd_line.AppendArgPath(testserver);
-    cmd_line.AppendArg(base::StringPrintf("--port=%d", kPort_));
-    cmd_line.AppendArgNative(FILE_PATH_LITERAL("--datafile=") +
-                             datafile.value());
-
-    base::LaunchOptions options;
-#if defined(OS_WIN)
-    options.start_hidden = true;
-#endif
-    if (!base::LaunchProcess(cmd_line, options, &server_handle_)) {
-      LOG(ERROR) << "Failed to launch server: "
-                 << cmd_line.GetCommandLineString();
-      return false;
-    }
-    return true;
-  }
-
-  // Stop the python server test suite.
-  bool Stop() {
-    if (server_handle_ == base::kNullProcessHandle)
-      return true;
-
-    // First check if the process has already terminated.
-    if (!base::WaitForSingleProcess(server_handle_, base::TimeDelta()) &&
-        !base::KillProcess(server_handle_, 1, true)) {
-      VLOG(1) << "Kill failed?";
-      return false;
-    }
-
-    base::CloseProcessHandle(server_handle_);
-    server_handle_ = base::kNullProcessHandle;
-    VLOG(1) << "Stopped.";
-    return true;
-  }
-
-  static const char* Host() {
-    return kHost_;
-  }
-
-  static int Port() {
-    return kPort_;
-  }
-
- private:
-  static const char kHost_[];
-  static const int kPort_;
-  FilePath datafile_;
-  base::ProcessHandle server_handle_;
-  DISALLOW_COPY_AND_ASSIGN(SafeBrowsingTestServer);
-};
-
-const char SafeBrowsingTestServer::kHost_[] = "localhost";
-const int SafeBrowsingTestServer::kPort_ = 40102;
-
 // This starts the browser and keeps status of states related to SafeBrowsing.
 class SafeBrowsingServiceTest : public InProcessBrowserTest {
  public:
   SafeBrowsingServiceTest()
     : safe_browsing_service_(NULL),
       is_database_ready_(true),
       is_update_scheduled_(false),
       is_checked_url_in_db_(false),
       is_checked_url_safe_(false) {
   }
 
   virtual ~SafeBrowsingServiceTest() {
   }
 
+  virtual void SetUp() OVERRIDE {
+    FilePath datafile_path;
+    ASSERT_TRUE(PathService::Get(base::DIR_SOURCE_ROOT, &datafile_path));
+
+    datafile_path = datafile_path.Append(FILE_PATH_LITERAL("third_party"))
+          .Append(FILE_PATH_LITERAL("safe_browsing"))
+          .Append(FILE_PATH_LITERAL("testing"))
+          .Append(kDataFile);
+    test_server_.reset(new LocalSafeBrowsingTestServer(datafile_path));
+    ASSERT_TRUE(test_server_->Start());
+
+    LOG(INFO) << "server is " << server_host() << ":" << server_port();
+    InProcessBrowserTest::SetUp();

[Ryan Sleevi, 2012/08/08 06:15:40]

Running InProcBrowsertest::SetUp() after, rather than before [and conversely, in
TearDown, before rather than after] seems to be contrary to how one would think
of it naturally (ctors = base before derived, dtors = derived before base)

It wasn't clear why this needed to be in SetUp/TearDown as opposed to simply
heap allocating in the SafeBrowsingServiceTest constructor (and letting the dtor
stop)

[mattm, 2012/08/30 21:50:47]

It can't do in the constructor, since you can't ASSERT_foo in the constructor..
though it could just attempt to do it in the constructor and then later assert
whether it worked or not..

However I guess it could just do this in SetUpCommandLine instead.  done.

+  }
+
+  virtual void TearDown() OVERRIDE {
+    InProcessBrowserTest::TearDown();
+    test_server_->Stop();
+  }
+
   void UpdateSafeBrowsingStatus() {
     ASSERT_TRUE(safe_browsing_service_);
     base::AutoLock lock(update_status_mutex_);
     last_update_ = safe_browsing_service_->protocol_manager_->last_update();
     is_update_scheduled_ =
         safe_browsing_service_->protocol_manager_->update_timer_.IsRunning();
   }
 
   void ForceUpdate() {
     ASSERT_TRUE(safe_browsing_service_);
@@ skipping 47 matching lines @@
 
   bool is_update_scheduled() {
     base::AutoLock l(update_status_mutex_);
     return is_update_scheduled_;
   }
 
   MessageLoop* SafeBrowsingMessageLoop() {
     return safe_browsing_service_->safe_browsing_thread_->message_loop();
   }
 
+  const std::string& server_host() const {
+    return test_server_->host_port_pair().host();
+  }
+
+  int server_port() const {
+    return test_server_->host_port_pair().port();
+  }
+
  protected:
   bool InitSafeBrowsingService() {
     safe_browsing_service_ = g_browser_process->safe_browsing_service();
     return safe_browsing_service_ != NULL;
   }
 
-  virtual void SetUpCommandLine(CommandLine* command_line) {
+  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
     // Makes sure the auto update is not triggered. This test will force the
     // update when needed.
     command_line->AppendSwitch(switches::kSbDisableAutoUpdate);
 
     // This test uses loopback. No need to use IPv6 especially it makes
     // local requests slow on Windows trybot when ipv6 local address [::1]
     // is not setup.
     command_line->AppendSwitch(switches::kDisableIPv6);
 
     // TODO(lzheng): The test server does not understand download related
     // requests. We need to fix the server.
     command_line->AppendSwitch(switches::kSbDisableDownloadProtection);
 
     // TODO(gcasto): Generate new testing data that includes the
     // client-side phishing whitelist.
     command_line->AppendSwitch(
         switches::kDisableClientSidePhishingDetection);
 
     // Point to the testing server for all SafeBrowsing requests.
     std::string url_prefix =
         base::StringPrintf("http://%s:%d/safebrowsing",
-                           SafeBrowsingTestServer::Host(),
-                           SafeBrowsingTestServer::Port());
+                           server_host().c_str(),
+                           server_port());
     command_line->AppendSwitchASCII(switches::kSbURLPrefix, url_prefix);
   }
 
   void SetTestStep(int step) {
     std::string test_step = base::StringPrintf("test_step=%d", step);
     safe_browsing_service_->protocol_manager_->set_additional_query(test_step);
   }
 
  private:
   SafeBrowsingService* safe_browsing_service_;
 
+  scoped_ptr<net::TestServer> test_server_;
+
   // Protects all variables below since they are read on UI thread
   // but updated on IO thread or safebrowsing thread.
   base::Lock update_status_mutex_;
 
   // States associated with safebrowsing service updates.
   bool is_database_ready_;
   base::Time last_update_;
   bool is_update_scheduled_;
   // Indicates if there is a match between a URL's prefix and safebrowsing
   // database (thus potentially it is a phishing URL).
@@ skipping 110 matching lines @@
     BrowserThread::PostDelayedTask(
         BrowserThread::IO,
         FROM_HERE,
         base::Bind(&SafeBrowsingServiceTestHelper::CheckStatusOnIOThread,
                    this),
         wait_time);
     // Will continue after OnWaitForStatusUpdateDone().
     content::RunMessageLoop();
   }
 
-  void WaitTillServerReady(const char* host, int port) {
-    response_status_ = net::URLRequestStatus::FAILED;
-    GURL url(base::StringPrintf("http://%s:%d%s?test_step=0",
-                                host, port, kDBResetPath));
-    // TODO(lzheng): We should have a way to reliably tell when a server is
-    // ready so we could get rid of the Sleep and retry loop.
-    while (true) {
-      if (FetchUrl(url) == net::URLRequestStatus::SUCCESS)
-        break;
-      // Wait and try again if last fetch was failed. The loop will hit the
-      // timeout in OutOfProcTestRunner if the fetch can not get success
-      // response.
-      base::PlatformThread::Sleep(TestTimeouts::tiny_timeout());
-    }
-  }
-
   // Calls test server to fetch database for verification.
-  net::URLRequestStatus::Status FetchDBToVerify(const char* host, int port,
-                                                int test_step) {
+  net::URLRequestStatus::Status FetchDBToVerify(
+      const std::string& host, int port, int test_step) {
     // TODO(lzheng): Remove chunk_type=add once it is not needed by the server.
     GURL url(base::StringPrintf(
         "http://%s:%d%s?"
         "client=chromium&appver=1.0&pver=2.2&test_step=%d&"
         "chunk_type=add",
-        host, port, kDBVerifyPath, test_step));
+        host.c_str(), port, kDBVerifyPath, test_step));
     return FetchUrl(url);
   }
 
   // Calls test server to fetch URLs for verification.
-  net::URLRequestStatus::Status FetchUrlsToVerify(const char* host, int port,
-                                                  int test_step) {
+  net::URLRequestStatus::Status FetchUrlsToVerify(
+      const std::string& host, int port, int test_step) {
     GURL url(base::StringPrintf(
         "http://%s:%d%s?"
         "client=chromium&appver=1.0&pver=2.2&test_step=%d",
-        host, port, kUrlVerifyPath, test_step));
+        host.c_str(), port, kUrlVerifyPath, test_step));
     return FetchUrl(url);
   }
 
   // Calls test server to check if test data is done. E.g.: if there is a
   // bad URL that server expects test to fetch full hash but the test didn't,
   // this verification will fail.
-  net::URLRequestStatus::Status VerifyTestComplete(const char* host, int port,
-                                                   int test_step) {
+  net::URLRequestStatus::Status VerifyTestComplete(
+      const std::string& host, int port, int test_step) {
     GURL url(StringPrintf("http://%s:%d%s?test_step=%d",
-                          host, port, kTestCompletePath, test_step));
+                          host.c_str(), port, kTestCompletePath, test_step));
     return FetchUrl(url);
   }
 
   // Callback for URLFetcher.
   virtual void OnURLFetchComplete(const net::URLFetcher* source) {
     source->GetResponseAsString(&response_data_);
     response_status_ = source->GetStatus().status();
     StopUILoop();
   }
 
@@ skipping 27 matching lines @@
   SafeBrowsingServiceTest* safe_browsing_test_;
   scoped_ptr<net::URLFetcher> url_fetcher_;
   std::string response_data_;
   net::URLRequestStatus::Status response_status_;
   net::URLRequestContextGetter* request_context_;
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingServiceTestHelper);
 };
 
 // See http://crbug.com/96459
 IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest,
-                       DISABLED_SafeBrowsingSystemTest) {
+                       SafeBrowsingSystemTest) {
   LOG(INFO) << "Start test";
-  const char* server_host = SafeBrowsingTestServer::Host();
-  int server_port = SafeBrowsingTestServer::Port();
   ASSERT_TRUE(InitSafeBrowsingService());
 
   net::URLRequestContextGetter* request_context =
       GetBrowserContext()->GetRequestContext();
   scoped_refptr<SafeBrowsingServiceTestHelper> safe_browsing_helper(
       new SafeBrowsingServiceTestHelper(this, request_context));
   int last_step = 0;
-  FilePath datafile_path = FilePath(kDataFile);
-  SafeBrowsingTestServer test_server(datafile_path);
-  ASSERT_TRUE(test_server.Start());
-
-  // Make sure the server is running.
-  safe_browsing_helper->WaitTillServerReady(server_host, server_port);
 
   // Waits and makes sure safebrowsing update is not happening.
   // The wait will stop once OnWaitForStatusUpdateDone in
   // safe_browsing_helper is called and status from safe_browsing_service_
   // is checked.
   safe_browsing_helper->WaitForStatusUpdate(base::TimeDelta());
   EXPECT_TRUE(is_database_ready());
   EXPECT_FALSE(is_update_scheduled());
   EXPECT_TRUE(last_update().is_null());
   // Starts updates. After each update, the test will fetch a list of URLs with
@@ skipping 21 matching lines @@
     } while (is_update_scheduled() || !is_database_ready());
 
 
     if (last_update() < now) {
       // This means no data available anymore.
       break;
     }
 
     // Fetches URLs to verify and waits till server responses with data.
     EXPECT_EQ(net::URLRequestStatus::SUCCESS,
-              safe_browsing_helper->FetchUrlsToVerify(server_host,
-                                                      server_port,
+              safe_browsing_helper->FetchUrlsToVerify(server_host(),
+                                                      server_port(),
                                                       step));
 
     std::vector<PhishingUrl> phishing_urls;
     EXPECT_TRUE(ParsePhishingUrls(safe_browsing_helper->response_data(),
                                   &phishing_urls));
     EXPECT_GT(phishing_urls.size(), 0U);
     for (size_t j = 0; j < phishing_urls.size(); ++j) {
       // Verifes with server if a URL is a phishing URL and waits till server
       // responses.
       safe_browsing_helper->CheckUrl(GURL(phishing_urls[j].url));
       if (phishing_urls[j].is_phishing) {
         EXPECT_TRUE(is_checked_url_in_db())
             << phishing_urls[j].url
             << " is_phishing: " << phishing_urls[j].is_phishing
             << " test step: " << step;
         EXPECT_FALSE(is_checked_url_safe())
             << phishing_urls[j].url
             << " is_phishing: " << phishing_urls[j].is_phishing
             << " test step: " << step;
       } else {
         EXPECT_TRUE(is_checked_url_safe())
             << phishing_urls[j].url
             << " is_phishing: " << phishing_urls[j].is_phishing
             << " test step: " << step;
       }
     }
     // TODO(lzheng): We should verify the fetched database with local
     // database to make sure they match.
     EXPECT_EQ(net::URLRequestStatus::SUCCESS,
-              safe_browsing_helper->FetchDBToVerify(server_host,
-                                                    server_port,
+              safe_browsing_helper->FetchDBToVerify(server_host(),
+                                                    server_port(),
                                                     step));
     EXPECT_GT(safe_browsing_helper->response_data().size(), 0U);
     last_step = step;
   }
 
   // Verifies with server if test is done and waits till server responses.
   EXPECT_EQ(net::URLRequestStatus::SUCCESS,
-            safe_browsing_helper->VerifyTestComplete(server_host,
-                                                     server_port,
+            safe_browsing_helper->VerifyTestComplete(server_host(),
+                                                     server_port(),
                                                      last_step));
   EXPECT_EQ("yes", safe_browsing_helper->response_data());
-  test_server.Stop();
 }