Display DNS probe results.

chrome/browser/net/dns_probe_service_unittest.cc

Index: chrome/browser/net/dns_probe_service_unittest.cc
diff --git a/chrome/browser/net/dns_probe_service_unittest.cc b/chrome/browser/net/dns_probe_service_unittest.cc
index 108332a3949daa95c7a4d0826e9e1ba19bf46da5..73f89a3610567f808ca9c4c5f0f79488b5accc51 100644
--- a/chrome/browser/net/dns_probe_service_unittest.cc
+++ b/chrome/browser/net/dns_probe_service_unittest.cc
@@ -9,111 +9,26 @@
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop.h"
 #include "base/run_loop.h"
-#include "chrome/browser/net/dns_probe_job.h"
+#include "chrome/browser/net/dns_probe_runner.h"
+#include "chrome/browser/net/dns_probe_test_util.h"
 #include "chrome/common/net/net_error_info.h"
+#include "net/dns/dns_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using chrome_common_net::DnsProbeResult;
+using base::MessageLoopForIO;
+using base::RunLoop;
+using chrome_common_net::DnsProbeStatus;
+using net::MockDnsClientRule;
 
 namespace chrome_browser_net {
 
 namespace {
 
-class MockDnsProbeJob : public DnsProbeJob {
- public:
-  MockDnsProbeJob(const CallbackType& callback,
-                  DnsProbeJob::Result result)
-      : weak_factory_(this) {
-    base::MessageLoop::current()->PostTask(
-        FROM_HERE,
-        base::Bind(&MockDnsProbeJob::CallCallback,
-                   weak_factory_.GetWeakPtr(),
-                   callback,
-                   result));
-  }
-
-  virtual ~MockDnsProbeJob() { }
-
- private:
-  void CallCallback(const CallbackType& callback, Result result) {
-    callback.Run(this, result);
-  }
-
-  base::WeakPtrFactory<MockDnsProbeJob> weak_factory_;
-};
-
-class TestDnsProbeService : public DnsProbeService {
- public:
-  TestDnsProbeService()
-      : DnsProbeService(),
-        system_job_created_(false),
-        public_job_created_(false),
-        mock_system_result_(DnsProbeJob::SERVERS_UNKNOWN),
-        mock_public_result_(DnsProbeJob::SERVERS_UNKNOWN),
-        mock_system_fail_(false) {
-  }
-
-  virtual ~TestDnsProbeService() { }
-
-  void set_mock_results(
-      DnsProbeJob::Result mock_system_result,
-      DnsProbeJob::Result mock_public_result) {
-    mock_system_result_ = mock_system_result;
-    mock_public_result_ = mock_public_result;
-  }
-
-  void set_mock_system_fail(bool mock_system_fail) {
-    mock_system_fail_ = mock_system_fail;
-  }
-
-  bool jobs_created(void) {
-    return system_job_created_ && public_job_created_;
-  }
-
-  void ResetJobsCreated() {
-    system_job_created_ = false;
-    public_job_created_ = false;
-  }
-
-  void MockExpireResults() {
-    ExpireResults();
-  }
-
-  bool system_job_created_;
-  bool public_job_created_;
-
- private:
-  // Override methods in DnsProbeService to return mock jobs:
-
-  virtual scoped_ptr<DnsProbeJob> CreateSystemProbeJob(
-      const DnsProbeJob::CallbackType& job_callback) OVERRIDE {
-    if (mock_system_fail_)
-      return scoped_ptr<DnsProbeJob>(NULL);
-
-    system_job_created_ = true;
-    return scoped_ptr<DnsProbeJob>(
-        new MockDnsProbeJob(job_callback,
-                            mock_system_result_));
-  }
-
-  virtual scoped_ptr<DnsProbeJob> CreatePublicProbeJob(
-      const DnsProbeJob::CallbackType& job_callback) OVERRIDE {
-    public_job_created_ = true;
-    return scoped_ptr<DnsProbeJob>(
-        new MockDnsProbeJob(job_callback,
-                            mock_public_result_));
-  }
-
-  DnsProbeJob::Result mock_system_result_;
-  DnsProbeJob::Result mock_public_result_;
-  bool mock_system_fail_;
-};
-
 class DnsProbeServiceTest : public testing::Test {
  public:
   DnsProbeServiceTest()
       : callback_called_(false),
-        callback_result_(chrome_common_net::DNS_PROBE_UNKNOWN) {
+        callback_result_(chrome_common_net::DNS_PROBE_MAX) {
   }
 
   void Probe() {
@@ -122,97 +37,87 @@ class DnsProbeServiceTest : public testing::Test {
   }
 
   void RunUntilIdle() {
-    base::RunLoop run_loop;
+    RunLoop run_loop;
     run_loop.RunUntilIdle();

[mmenke, 2013/06/12 19:17:12]

Per earlier comment, suggest inlining this everywhere, and merging into one
line.

[Deprecated (see juliatuttle), 2013/06/13 14:37:04]

Done.

   }
 
   void Reset() {
-    service_.ResetJobsCreated();
     callback_called_ = false;
   }
 
-  base::MessageLoopForIO message_loop_;
-  TestDnsProbeService service_;
-  bool callback_called_;
-  DnsProbeResult callback_result_;
+ protected:
+  void SetRules(MockDnsClientRule::Result system_good_result,
+                MockDnsClientRule::Result public_good_result) {
+    service_.SetSystemClientForTesting(
+        CreateMockDnsClientForProbes(system_good_result));
+    service_.SetPublicClientForTesting(
+        CreateMockDnsClientForProbes(public_good_result));
+  }
+
+  void ExpireServiceResult() {
+    service_.ExpireResultForTesting();
+  }
+
+  bool callback_called() { return callback_called_; }
+  DnsProbeStatus callback_result() { return callback_result_; }
 
  private:
-  void ProbeCallback(DnsProbeResult result) {
+  void ProbeCallback(DnsProbeStatus result) {
     callback_called_ = true;
     callback_result_ = result;
   }
+
+  DnsProbeService service_;
+  bool callback_called_;
+  DnsProbeStatus callback_result_;
+  MessageLoopForIO message_loop_;

[mmenke, 2013/06/12 19:17:12]

Other comment about how to handle message loops applies here as well.

[Deprecated (see juliatuttle), 2013/06/13 14:37:04]

Done.

 };
 
 TEST_F(DnsProbeServiceTest, Null) {
 }
 
 TEST_F(DnsProbeServiceTest, Probe) {
-  service_.set_mock_results(DnsProbeJob::SERVERS_CORRECT,
-                            DnsProbeJob::SERVERS_CORRECT);
+  SetRules(MockDnsClientRule::OK, MockDnsClientRule::OK);
 
   Probe();
-  EXPECT_TRUE(service_.jobs_created());
-  EXPECT_FALSE(callback_called_);
-
   RunUntilIdle();
-  EXPECT_TRUE(callback_called_);
-  EXPECT_EQ(chrome_common_net::DNS_PROBE_NXDOMAIN, callback_result_);
+  EXPECT_TRUE(callback_called());
+  EXPECT_EQ(chrome_common_net::DNS_PROBE_FINISHED_NXDOMAIN, callback_result());
 }
 
+// TODO(ttuttle): More test cases?

[mmenke, 2013/06/12 19:17:12]

Suggestions:  Sync failure, all other different results, and maybe a case with
two callbacks called together?  Could also have a callback that triggers another
probe, but don't think that's too important.

[Deprecated (see juliatuttle), 2013/06/13 14:37:04]

Done.

+
 TEST_F(DnsProbeServiceTest, Cache) {
-  service_.set_mock_results(DnsProbeJob::SERVERS_CORRECT,
-                            DnsProbeJob::SERVERS_CORRECT);
+  SetRules(MockDnsClientRule::OK, MockDnsClientRule::OK);
 
   Probe();
   RunUntilIdle();
-  Reset();
 
   // Cached NXDOMAIN result should persist.

[mmenke, 2013/06/12 19:17:12]

Shouldn't you change the result, so if the result is not cached, we get a new
result?

[Deprecated (see juliatuttle), 2013/06/13 14:37:04]

Done.

 
-  Probe();
-  EXPECT_FALSE(service_.jobs_created());
+  Reset();
 
+  Probe();
   RunUntilIdle();

[mmenke, 2013/06/12 19:17:12]

Not needed, is it?

[Deprecated (see juliatuttle), 2013/06/13 14:37:04]

It is now.

-  EXPECT_TRUE(callback_called_);
-  EXPECT_EQ(chrome_common_net::DNS_PROBE_NXDOMAIN, callback_result_);
+  EXPECT_TRUE(callback_called());
+  EXPECT_EQ(chrome_common_net::DNS_PROBE_FINISHED_NXDOMAIN, callback_result());
 }
 
 TEST_F(DnsProbeServiceTest, Expired) {
-  service_.set_mock_results(DnsProbeJob::SERVERS_CORRECT,
-                            DnsProbeJob::SERVERS_CORRECT);
+  SetRules(MockDnsClientRule::OK, MockDnsClientRule::OK);
 
   Probe();
-  EXPECT_TRUE(service_.jobs_created());
-
   RunUntilIdle();
-  EXPECT_TRUE(callback_called_);
-  EXPECT_EQ(chrome_common_net::DNS_PROBE_NXDOMAIN, callback_result_);
+  EXPECT_TRUE(callback_called());
+  EXPECT_EQ(chrome_common_net::DNS_PROBE_FINISHED_NXDOMAIN, callback_result());
 
+  ExpireServiceResult();
   Reset();
 
-  service_.MockExpireResults();
-
-  Probe();
-  EXPECT_TRUE(service_.jobs_created());
-
-  RunUntilIdle();
-  EXPECT_TRUE(callback_called_);
-  EXPECT_EQ(chrome_common_net::DNS_PROBE_NXDOMAIN, callback_result_);
-}
-
-TEST_F(DnsProbeServiceTest, SystemFail) {
-  service_.set_mock_results(DnsProbeJob::SERVERS_CORRECT,
-                            DnsProbeJob::SERVERS_CORRECT);
-  service_.set_mock_system_fail(true);
-
   Probe();

[mmenke, 2013/06/12 19:17:12]

Should change the result for the second probe.

[Deprecated (see juliatuttle), 2013/06/13 14:37:04]

Done.

-  EXPECT_TRUE(callback_called_);
-  EXPECT_EQ(chrome_common_net::DNS_PROBE_UNKNOWN, callback_result_);
-
-  Reset();
-
   RunUntilIdle();
-  EXPECT_FALSE(callback_called_);
+  EXPECT_TRUE(callback_called());
+  EXPECT_EQ(chrome_common_net::DNS_PROBE_FINISHED_NXDOMAIN, callback_result());
 }
 
 }  // namespace