Test cases for the cloud policy subsystem

chrome/browser/policy/cloud_policy_subsystem_unittest.cc

Index: chrome/browser/policy/cloud_policy_subsystem_unittest.cc
diff --git a/chrome/browser/policy/cloud_policy_subsystem_unittest.cc b/chrome/browser/policy/cloud_policy_subsystem_unittest.cc
index a8c0ffe915a817a7a8c30909329be1d4cd1d8378..8debf9b61a120f8c75113d753946bde16fdc1893 100644
--- a/chrome/browser/policy/cloud_policy_subsystem_unittest.cc
+++ b/chrome/browser/policy/cloud_policy_subsystem_unittest.cc
@@ -105,16 +105,60 @@ class TestingIdentityStrategy : public CloudPolicyIdentityStrategy {
   DISALLOW_COPY_AND_ASSIGN(TestingIdentityStrategy);
 };
 
+// An action that returns an URLRequestJob with an HTTP error code.
+ACTION_P(CreateFailedResponse, http_error_code) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  em::DeviceManagementResponse response_data;
+
+  arg2->response_data = response_data.SerializeAsString();
+  arg2->response_code = http_error_code;
+}
+
+// An action that returns an URLRequestJob with a successful device
+// registration response.
+ACTION_P(CreateSuccessfulRegisterResponse, token) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  em::DeviceManagementResponse response_data;
+  response_data.mutable_register_response()->set_device_management_token(token);
+
+  arg2->response_data = response_data.SerializeAsString();
+  arg2->response_code = 200;
+}
+
+// An action that returns an URLRequestJob with a successful policy response.
+ACTION_P(CreateSuccessfulPolicyResponse, homepage_location) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  em::CloudPolicySettings settings;
+  settings.mutable_homepagelocation()->set_homepagelocation(homepage_location);
+  em::PolicyData policy_data;
+  policy_data.set_policy_type(kPolicyType);
+  policy_data.set_policy_value(settings.SerializeAsString());
+
+  em::DeviceManagementResponse response_data;
+  em::DevicePolicyResponse* policy_response =
+      response_data.mutable_policy_response();
+  em::PolicyFetchResponse* fetch_response = policy_response->add_response();
+  fetch_response->set_error_code(200);
+  fetch_response->set_policy_data(policy_data.SerializeAsString());
+
+  arg2->response_data = response_data.SerializeAsString();
+  arg2->response_code = 200;
+}
+
 // Tests CloudPolicySubsystem by intercepting its network requests.
 // The requests are intercepted by PolicyRequestInterceptor and they are
 // logged by LoggingWorkScheduler for further examination.
-class CloudPolicySubsystemTest : public testing::Test {
+template<typename TESTBASE>
+class CloudPolicySubsystemTestBase : public TESTBASE {
  public:
-  CloudPolicySubsystemTest()
+  CloudPolicySubsystemTestBase()
       : ui_thread_(BrowserThread::UI, &loop_),
         io_thread_(BrowserThread::IO, &loop_) {
   }
 
+  virtual ~CloudPolicySubsystemTestBase() {
+  }
+
  protected:
   void StopMessageLoop() {
     loop_.QuitNow();
@@ -148,6 +192,55 @@ class CloudPolicySubsystemTest : public testing::Test {
     prefs_.reset();
   }
 
+  void ExecuteTest() {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    // The first unexptected request of the policy subsystem will stop the

[Joao da Silva, 2011/06/21 15:58:19]

Nit: *unexpected

[gfeher, 2011/06/23 12:22:59]

Done.

+    // message loop.
+    // This code relies on the fact that an InSequence object is active.
+    EXPECT_CALL(*(factory_.get()), Intercept(_, _, _)).
+       Times(AtMost(1)).
+       WillOnce(InvokeWithoutArgs(
+           this,
+           &CloudPolicySubsystemTestBase::StopMessageLoop));
+    loop_.RunAllPending();
+  }
+
+  void VerifyTest(const std::string& homepage_location) {
+    // Test conditions.
+    EXPECT_EQ(CloudPolicySubsystem::SUCCESS, cloud_policy_subsystem_->state());
+    StringValue homepage_value(homepage_location);
+    VerifyPolicy(kPolicyHomepageLocation, &homepage_value);
+    VerifyServerLoad();
+  }
+
+  void VerifyState(CloudPolicySubsystem::PolicySubsystemState state) {
+    EXPECT_EQ(state, cloud_policy_subsystem_->state());
+  }
+
+  void ExpectSuccessfulRegistration() {
+    EXPECT_CALL(*(factory_.get()), Intercept(kGaiaAuthHeader, "register", _)).

[Mattias Nissler (ping if slow), 2011/06/21 20:35:27]

It seems that most cases put the . on the next line, which I also find more
readable. I don't know what the more common form is though.

[gfeher, 2011/06/23 12:22:59]

gmock_unittest.cc puts the . on the next lines, that's a good enough reason for
me to do the same...

+        WillOnce(CreateSuccessfulRegisterResponse(kDMToken));
+  }
+
+  void ExpectFailedRegistration(int n, int code) {
+    EXPECT_CALL(*(factory_.get()), Intercept(kGaiaAuthHeader, "register", _)).
+        Times(n).
+        WillRepeatedly(CreateFailedResponse(code));
+  }
+
+  void ExpectFailedPolicy(int n, int code) {
+    EXPECT_CALL(*(factory_.get()), Intercept(kDMAuthHeader, "policy", _)).
+        Times(n).
+        WillRepeatedly(CreateFailedResponse(code));
+  }
+
+  void ExpectSuccessfulPolicy(int n, const std::string& homepage) {
+    EXPECT_CALL(*(factory_.get()), Intercept(kDMAuthHeader, "policy", _)).
+        Times(n).
+        WillRepeatedly(CreateSuccessfulPolicyResponse(homepage));
+  }
+
+ private:
   // Verifies for a given policy that it is provided by the subsystem.
   void VerifyPolicy(enum ConfigurationPolicyType type, Value* expected) {
     MockConfigurationPolicyStore store;
@@ -161,7 +254,7 @@ class CloudPolicySubsystemTest : public testing::Test {
   // - no more than 10 requests in the first 10 minutes
   // - no more then 12 requests per hour in the next 10 hours
   // TODO(gfeher): Thighten these conditions further. This will require
-  // fine-tuning of the subsystem. See: http://crosbug.com/15195
+  // fine-tuning of the subsystem. See: http://crosbug.com/16637
   void VerifyServerLoad() {
     std::vector<int64> events;
     logger_->Swap(&events);
@@ -190,28 +283,6 @@ class CloudPolicySubsystemTest : public testing::Test {
         << "No enough requests were fired during the test run.";
   }
 
-  void ExecuteTest(const std::string& homepage_location) {
-    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-    // The first unexptected request of the policy subsystem will stop the
-    // message loop.
-    // This code relies on the fact that an InSequence object is active.
-    EXPECT_CALL(*(factory_.get()), Intercept(_, _, _)).
-       Times(AtMost(1)).
-       WillOnce(InvokeWithoutArgs(
-           this,
-           &CloudPolicySubsystemTest::StopMessageLoop));
-    loop_.RunAllPending();
-
-    // Test conditions.
-    EXPECT_EQ(CloudPolicySubsystem::SUCCESS, cloud_policy_subsystem_->state());
-    StringValue homepage_value(homepage_location);
-    VerifyPolicy(kPolicyHomepageLocation, &homepage_value);
-    VerifyServerLoad();
-  }
-
-  scoped_ptr<TestingPolicyURLFetcherFactory> factory_;
-
- private:
   ScopedTempDir temp_user_data_dir_;
 
   MessageLoop loop_;
@@ -224,81 +295,151 @@ class CloudPolicySubsystemTest : public testing::Test {
   scoped_ptr<PrefService> prefs_;
   CloudPolicyCacheBase* cache_;
 
-  DISALLOW_COPY_AND_ASSIGN(CloudPolicySubsystemTest);
+  scoped_ptr<TestingPolicyURLFetcherFactory> factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(CloudPolicySubsystemTestBase);
 };
 
-// An action that returns an URLRequestJob with an HTTP error code.
-ACTION_P(CreateFailedResponse, http_error_code) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  em::DeviceManagementResponse response_data;
+// A parameterized test case that simulates 100 failed registration attempts,
+// then a successful one, then 100 failed policy fetch attempts and then 100
+// successful policy fetches. The two parameters are the error codes for the
+// failed registration and policy responses.
 
-  arg2->response_data = response_data.SerializeAsString();
-  arg2->response_code = http_error_code;
+class CombinedTestDesc {
+ public:
+  CombinedTestDesc(int registration_error_code, int policy_error_code)
+      : registration_error_code_(registration_error_code),
+        policy_error_code_(policy_error_code) {
+  }
+
+  ~CombinedTestDesc() {}
+
+  int registration_error_code() const { return registration_error_code_; }
+  int policy_error_code() const { return policy_error_code_; }
+
+ private:
+  int registration_error_code_;
+  int policy_error_code_;
+};
+
+class CloudPolicySubsystemCombinedTest
+    : public CloudPolicySubsystemTestBase<
+                 testing::TestWithParam<CombinedTestDesc> > {
+};
+
+TEST_P(CloudPolicySubsystemCombinedTest, Combined) {
+  InSequence s;
+  ExpectFailedRegistration(100, GetParam().registration_error_code());
+  ExpectSuccessfulRegistration();
+  ExpectFailedPolicy(100, GetParam().policy_error_code());
+  ExpectSuccessfulPolicy(100, "http://www.google.com");
+  ExpectSuccessfulPolicy(1, "http://www.chromium.org");
+  ExecuteTest();
+  VerifyTest("http://www.chromium.org");
 }
 
-// An action that returns an URLRequestJob with a successful device
-// registration response.
-ACTION_P(CreateSuccessfulRegisterResponse, token) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  em::DeviceManagementResponse response_data;
-  response_data.mutable_register_response()->set_device_management_token(token);
+// A random sample of error code pairs. Note that the following policy error
+// codes (401, 403, 410) make the policy subsystem to try and reregister, and
+// that is not expected in these tests.
+INSTANTIATE_TEST_CASE_P(
+    CloudPolicySubsystemCombinedTestInstance,
+    CloudPolicySubsystemCombinedTest,
+    testing::Values(
+        CombinedTestDesc(403, 400),
+        CombinedTestDesc(403, 404),
+        CombinedTestDesc(403, 412),
+        CombinedTestDesc(403, 500),
+        CombinedTestDesc(403, 503),
+        CombinedTestDesc(403, 902),
+        CombinedTestDesc(902, 400),
+        CombinedTestDesc(503, 404),
+        CombinedTestDesc(500, 412),
+        CombinedTestDesc(412, 500),
+        CombinedTestDesc(404, 503),
+        CombinedTestDesc(400, 902)));
+
+// A parameterized test case that simulates 100 failed registration attempts,
+// then a successful one, and then a succesful policy fetch. The parameter is
+// the error code returned for registration attempts.
+
+class CloudPolicySubsystemRegistrationTest
+    : public CloudPolicySubsystemTestBase<testing::TestWithParam<int> > {
+};
 
-  arg2->response_data = response_data.SerializeAsString();
-  arg2->response_code = 200;
+TEST_P(CloudPolicySubsystemRegistrationTest, Registration) {
+  InSequence s;
+  ExpectFailedRegistration(100, GetParam());
+  ExpectSuccessfulRegistration();
+  ExpectSuccessfulPolicy(1, "http://www.youtube.com");
+  ExecuteTest();
+  VerifyTest("http://www.youtube.com");
 }
 
-// An action that returns an URLRequestJob with a successful policy response.
-ACTION_P(CreateSuccessfulPolicyResponse, homepage_location) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  em::CloudPolicySettings settings;
-  settings.mutable_homepagelocation()->set_homepagelocation(homepage_location);
-  em::PolicyData policy_data;
-  policy_data.set_policy_type(kPolicyType);
-  policy_data.set_policy_value(settings.SerializeAsString());
+INSTANTIATE_TEST_CASE_P(
+    CloudPolicySubsystemRegistrationTestInstance,
+    CloudPolicySubsystemRegistrationTest,
+    // For the case of 401 see CloudPolicySubsystemRegistrationFailureTest
+    testing::Values(400, 403, 404, 410, 412, 500, 503, 902));
 
-  em::DeviceManagementResponse response_data;
-  em::DevicePolicyResponse* policy_response =
-      response_data.mutable_policy_response();
-  em::PolicyFetchResponse* fetch_response = policy_response->add_response();
-  fetch_response->set_error_code(200);
-  fetch_response->set_policy_data(policy_data.SerializeAsString());
+// A test case that verifies that the subsystem understands the "not managed"
+// response from the server.
 
-  arg2->response_data = response_data.SerializeAsString();
-  arg2->response_code = 200;
-}
+class CloudPolicySubsystemRegistrationFailureTest
+    : public CloudPolicySubsystemTestBase<testing::Test> {
+};
 
-TEST_F(CloudPolicySubsystemTest, SuccessAfterSingleErrors) {
-  InSequence c;
+TEST_F(CloudPolicySubsystemRegistrationFailureTest, RegistrationFailure) {
+  InSequence s;
+  ExpectFailedRegistration(1, 401);
+  ExecuteTest();
+  VerifyState(CloudPolicySubsystem::BAD_GAIA_TOKEN);
+}
 
-  EXPECT_CALL(*(factory_.get()), Intercept(kGaiaAuthHeader, "register", _)).
-      WillOnce(CreateFailedResponse(901));
-  EXPECT_CALL(*(factory_.get()), Intercept(kGaiaAuthHeader, "register", _)).
-      WillOnce(CreateSuccessfulRegisterResponse(kDMToken));
+// A parameterized test case that simulates a successful registration, then 100
+// failed policy fetch attempts and then a successful one. The parameter is
+// the error code returned for failed policy attempts.
 
-  EXPECT_CALL(*(factory_.get()), Intercept(kDMAuthHeader, "policy", _)).
-      WillOnce(CreateFailedResponse(501));
-  EXPECT_CALL(*(factory_.get()), Intercept(kDMAuthHeader, "policy", _)).
-      Times(200).
-      WillRepeatedly(CreateSuccessfulPolicyResponse("http://www.google.com"));
-  EXPECT_CALL(*(factory_.get()), Intercept(kDMAuthHeader, "policy", _)).
-      WillOnce(CreateSuccessfulPolicyResponse("http://www.chromium.org"));
+class CloudPolicySubsystemPolicyTest
+    : public CloudPolicySubsystemTestBase<testing::TestWithParam<int> > {
+};
 
-  ExecuteTest("http://www.chromium.org");
+TEST_P(CloudPolicySubsystemPolicyTest, Policy) {
+  InSequence s;
+  ExpectSuccessfulRegistration();
+  ExpectFailedPolicy(100, GetParam());
+  ExpectSuccessfulPolicy(1, "http://www.youtube.com");
+  ExecuteTest();
+  VerifyTest("http://www.youtube.com");
 }
 
-TEST_F(CloudPolicySubsystemTest, SuccessAfterRegistrationErrors) {
-  InSequence c;
+INSTANTIATE_TEST_CASE_P(
+    CloudPolicySubsystemPolicyTestInstance,
+    CloudPolicySubsystemPolicyTest,
+    testing::Values(400, 404, 412, 500, 503, 902));
 
-  EXPECT_CALL(*(factory_.get()), Intercept(kGaiaAuthHeader, "register", _)).
-      Times(200).
-      WillRepeatedly(CreateFailedResponse(901));
-  EXPECT_CALL(*(factory_.get()), Intercept(kGaiaAuthHeader, "register", _)).
-      WillOnce(CreateSuccessfulRegisterResponse(kDMToken));
+// A parameterized test case that simulates a successful registration, then 40
+// failed policy fetch attempts and a successful registration after each of
+// them. The parameter is the error code returned for registration attempts.
 
-  EXPECT_CALL(*(factory_.get()), Intercept(kDMAuthHeader, "policy", _)).
-      WillOnce(CreateSuccessfulPolicyResponse("http://www.example.com"));
+class CloudPolicySubsystemPolicyReregisterTest
+    : public CloudPolicySubsystemTestBase<testing::TestWithParam<int> > {
+};
 
-  ExecuteTest("http://www.example.com");
+TEST_P(CloudPolicySubsystemPolicyReregisterTest, Policy) {
+  InSequence s;
+  for (int i = 0; i < 40; i++) {
+    ExpectSuccessfulRegistration();
+    ExpectFailedPolicy(1, GetParam());
+  }
+  ExpectSuccessfulRegistration();
+  ExpectSuccessfulPolicy(1, "http://www.youtube.com");
+  ExecuteTest();
+  VerifyTest("http://www.youtube.com");
 }
 
+INSTANTIATE_TEST_CASE_P(
+    CloudPolicySubsystemPolicyReregisterTestInstance,
+    CloudPolicySubsystemPolicyReregisterTest,
+    testing::Values(401, 403, 410));
+
 }  // policy