Add the ability to use DataReductionProxyParams from DataReductionProxyConfigServiceClient.

components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc

Index: components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
index 6b44ceb5b70b51f6579cc46d344fc2cc741226a1..f142db0131d51ff7ad8bf85a33d7b7bb406d89ea 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
@@ -7,8 +7,10 @@
 #include <string>
 
 #include "base/memory/scoped_ptr.h"
+#include "base/time/tick_clock.h"
 #include "base/values.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h"
+#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator_test_utils.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_client_config_parser.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
@@ -19,6 +21,26 @@
 
 namespace data_reduction_proxy {
 
+namespace {
+
+const char kSuccessResponse[] =
+    "{ \"sessionKey\": \"foobar|foobaz\", "
+    "\"expireTime\": \"1970-01-02T00:00:00.000Z\", "
+    "\"proxyConfig\": { \"httpProxyServers\": ["
+    "{ \"scheme\": \"HTTPS\", \"host\": \"origin.net\", \"port\": 443 },"
+    "{ \"scheme\": \"HTTP\", \"host\": \"fallback.net\", \"port\": 80 }"
+    "] } }";
+
+const char kShortSuccessResponse[] =
+    "{ \"sessionKey\": \"foobar|foobaz\", "
+    "\"expireTime\": \"1970-01-01T00:00:01.000Z\", "
+    "\"proxyConfig\": { \"httpProxyServers\": ["
+    "{ \"scheme\": \"HTTPS\", \"host\": \"origin.net\", \"port\": 443 },"
+    "{ \"scheme\": \"HTTP\", \"host\": \"fallback.net\", \"port\": 80 }"
+    "] } }";
+
+}  // namespace
+
 class DataReductionProxyConfigClientTest : public testing::Test {

[sclittle, 2015/03/24 21:34:55]

Rename to DataReductionProxyConfigServiceClientTest

[jeremyim, 2015/03/24 23:20:28]

Done.

  protected:
   void SetUp() override {
@@ -28,13 +50,40 @@ class DataReductionProxyConfigClientTest : public testing::Test {
                              DataReductionProxyParams::kFallbackAllowed |
                              DataReductionProxyParams::kPromoAllowed)
             .WithParamsDefinitions(TestDataReductionProxyParams::HAS_EVERYTHING)
+            .WithTestConfigurator()
+            .WithMockConfigClient()
             .Build();
+    test_context_->mock_config_client()->SetCustomReleaseTime(
+        base::TimeTicks::UnixEpoch());
   }
 
   scoped_ptr<DataReductionProxyConfigServiceClient> BuildConfigClient() {
-    return make_scoped_ptr(new DataReductionProxyConfigServiceClient(
-        test_context_->config()->test_params(),
-        test_context_->io_data()->request_options()));
+    scoped_ptr<DataReductionProxyParams> params(new DataReductionProxyParams(
+        DataReductionProxyParams::kAllowed |
+        DataReductionProxyParams::kFallbackAllowed |
+        DataReductionProxyParams::kPromoAllowed));
+    return scoped_ptr<DataReductionProxyConfigServiceClient>(
+        new DataReductionProxyConfigServiceClient(
+            params.Pass(), GetBackoffPolicy(),
+            test_context_->io_data()->request_options(),
+            test_context_->mutable_config_values(),
+            test_context_->io_data()->config(), test_context_->task_runner()));
+  }
+
+  void SetDataReductionProxyEnabled(bool enabled) {
+    test_context_->config()->SetStateForTest(enabled, false, false);
+  }
+
+  MockDataReductionProxyConfigServiceClient* mock_config_client() {
+    return test_context_->mock_config_client();
+  }
+
+  TestDataReductionProxyConfigurator* configurator() {
+    return test_context_->test_configurator();
+  }
+
+  void RunUntilIdle() {
+    test_context_->RunUntilIdle();
   }
 
  private:
@@ -49,4 +98,77 @@ TEST_F(DataReductionProxyConfigClientTest, TestConstructStaticResponse) {
   EXPECT_TRUE(config_parser::ParseClientConfig(config_data, &config));
 }
 
+TEST_F(DataReductionProxyConfigClientTest, SuccessfulLoop) {
+  SetDataReductionProxyEnabled(true);
+  EXPECT_TRUE(configurator()->origin().empty());
+  EXPECT_TRUE(configurator()->fallback_origin().empty());
+  EXPECT_TRUE(configurator()->ssl_origin().empty());
+  EXPECT_CALL(*mock_config_client(), ConstructStaticResponse())

[sclittle, 2015/03/24 21:34:55]

Why are you mocking the methods of DRPConfigClient while testing
DRPConfigClient? This seems to defeat the purpose of testing DRPConfigClient.

Couldn't you just set up DRPParams such that ConstructStaticResponse works
normally? You could even mock DRPParams if necessary.

IIUC, mocks should only be used for other interfaces that the class being tested
depends on, otherwise the test is just testing less of the class, and even then
fakes are preferable than mocks where feasible due to the overhead of learning
googlemock.

[jeremyim, 2015/03/24 23:20:28]

Done.

+      .Times(1)
+      .WillOnce(testing::Return(kSuccessResponse));
+  base::TimeDelta expected_delay = base::TimeDelta::FromDays(1);
+  EXPECT_CALL(*mock_config_client(), SetConfigRefreshTimer(expected_delay))

[sclittle, 2015/03/24 21:34:55]

Same as above: mocking the class being tested means that it tests less of the
class.

Maybe you could actually simulate that a day passes, and then check that it
constructs a new config. You could use a base::MockTimer or something; maybe you
could set up DRPConfigServiceClient to take in the timer in the constructor to
make this easier. Then you could just call MockTimer::Fire() to advance a day.

Ideally, you shouldn't have to test a fake or mock version of
DRPConfigServiceClient here at all, although you might still need to fake out
Now() somehow.

[jeremyim, 2015/03/24 23:20:28]

Done.

+      .Times(1);
+  RunUntilIdle();
+  EXPECT_EQ("https://origin.net:443", configurator()->origin());
+  EXPECT_EQ("fallback.net:80", configurator()->fallback_origin());
+  EXPECT_TRUE(configurator()->ssl_origin().empty());
+}
+
+TEST_F(DataReductionProxyConfigClientTest, SuccessfulLoopShortDuration) {
+  SetDataReductionProxyEnabled(true);
+  EXPECT_TRUE(configurator()->origin().empty());
+  EXPECT_TRUE(configurator()->fallback_origin().empty());
+  EXPECT_TRUE(configurator()->ssl_origin().empty());
+  EXPECT_CALL(*mock_config_client(), ConstructStaticResponse())
+      .Times(1)
+      .WillOnce(testing::Return(kShortSuccessResponse));
+  base::TimeDelta expected_delay = base::TimeDelta::FromSeconds(10);
+  EXPECT_CALL(*mock_config_client(), SetConfigRefreshTimer(expected_delay))
+      .Times(1);
+  RunUntilIdle();
+  EXPECT_EQ("https://origin.net:443", configurator()->origin());
+  EXPECT_EQ("fallback.net:80", configurator()->fallback_origin());
+  EXPECT_TRUE(configurator()->ssl_origin().empty());
+}
+
+TEST_F(DataReductionProxyConfigClientTest, EnsureBackoff) {
+  SetDataReductionProxyEnabled(true);
+  EXPECT_TRUE(configurator()->origin().empty());
+  EXPECT_TRUE(configurator()->fallback_origin().empty());
+  EXPECT_TRUE(configurator()->ssl_origin().empty());
+  EXPECT_CALL(*mock_config_client(), ConstructStaticResponse())
+      .Times(2)
+      .WillRepeatedly(testing::Return(std::string()));
+  EXPECT_CALL(*mock_config_client(),
+              SetConfigRefreshTimer(base::TimeDelta::FromSeconds(20))).Times(1);
+  EXPECT_CALL(*mock_config_client(),
+              SetConfigRefreshTimer(base::TimeDelta::FromSeconds(40))).Times(1);
+  mock_config_client()->RetrieveConfig();
+  EXPECT_TRUE(configurator()->origin().empty());
+  EXPECT_TRUE(configurator()->fallback_origin().empty());
+  EXPECT_TRUE(configurator()->ssl_origin().empty());
+  mock_config_client()->RetrieveConfig();
+  EXPECT_TRUE(configurator()->origin().empty());
+  EXPECT_TRUE(configurator()->fallback_origin().empty());
+  EXPECT_TRUE(configurator()->ssl_origin().empty());
+}
+
+TEST_F(DataReductionProxyConfigClientTest, ConfigDisabled) {
+  SetDataReductionProxyEnabled(false);
+  EXPECT_TRUE(configurator()->origin().empty());
+  EXPECT_TRUE(configurator()->fallback_origin().empty());
+  EXPECT_TRUE(configurator()->ssl_origin().empty());
+  EXPECT_CALL(*mock_config_client(), ConstructStaticResponse())
+      .Times(1)
+      .WillOnce(testing::Return(kSuccessResponse));
+  base::TimeDelta expected_delay = base::TimeDelta::FromDays(1);
+  EXPECT_CALL(*mock_config_client(), SetConfigRefreshTimer(expected_delay))
+      .Times(1);
+  RunUntilIdle();
+  EXPECT_TRUE(configurator()->origin().empty());
+  EXPECT_TRUE(configurator()->fallback_origin().empty());
+  EXPECT_TRUE(configurator()->ssl_origin().empty());
+}
+
 }  // namespace data_reduction_proxy