Add DeviceSettingProvider unit tests.

chrome/browser/chromeos/device_settings_provider_unittest.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.
+
+#include "chrome/browser/chromeos/login/signed_settings.h"
+
+#include <map>
+#include <string>
+
+#include "base/bind.h"
+#include "base/memory/weak_ptr.h"
+#include "base/message_loop.h"
+#include "base/values.h"
+#include "chrome/browser/chromeos/device_settings_provider.h"

[Mattias Nissler (ping if slow), 2012/03/16 15:21:11]

alphabetize

[pastarmovj, 2012/03/21 15:23:27]

Done.

+#include "chrome/browser/chromeos/cros/cros_library.h"
+#include "chrome/browser/chromeos/cros_settings_names.h"
+#include "chrome/browser/chromeos/login/mock_signed_settings_helper.h"
+#include "chrome/browser/chromeos/login/mock_user_manager.h"
+#include "chrome/browser/chromeos/login/ownership_service.h"
+#include "chrome/browser/chromeos/login/signed_settings_cache.h"

[Mattias Nissler (ping if slow), 2012/03/16 15:21:11]

needed?

[pastarmovj, 2012/03/21 15:23:27]

Done.

+#include "chrome/browser/metrics/metrics_service.h"
+#include "chrome/browser/policy/browser_policy_connector.h"

[Mattias Nissler (ping if slow), 2012/03/16 15:21:11]

needed?

[pastarmovj, 2012/03/21 15:23:27]

Done.

+#include "chrome/browser/policy/cloud_policy_data_store.h"

[Mattias Nissler (ping if slow), 2012/03/16 15:21:11]

needed?

[pastarmovj, 2012/03/21 15:23:27]

Done.

+#include "chrome/browser/policy/proto/chrome_device_policy.pb.h"
+#include "chrome/browser/policy/proto/device_management_backend.pb.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chrome/test/base/testing_pref_service.h"
+#include "content/test/test_browser_thread.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace em = enterprise_management;
+namespace chromeos {
+
+using ::testing::_;
+using ::testing::AnyNumber;
+using ::testing::Mock;
+using ::testing::Return;
+using ::testing::SaveArg;
+
+class DeviceSettingsProviderTest: public testing::Test {
+public:
+  void SettingChanged(const std::string& name) {
+    notifier_called_ = true;

[Mattias Nissler (ping if slow), 2012/03/16 15:21:11]

You're never reading |notifier_called_|...

[pastarmovj, 2012/03/21 15:23:27]

Done.

+  }
+
+  void GetTrustedCallback() {

[Mattias Nissler (ping if slow), 2012/03/16 15:21:11]

Both SettingChanged and GetTrustedCallback (which are named inconsistently btw.)
could be MOCK_METHODs. If you do so, you don't have to keep flags, but you can
instead install expectations on the mocked methods.

[pastarmovj, 2012/03/21 15:23:27]

Done.

+    trusted_called_ = true;
+  }

[Mattias Nissler (ping if slow), 2012/03/16 15:21:11]

newline

[pastarmovj, 2012/03/21 15:23:27]

Done.

+protected:
+  DeviceSettingsProviderTest()
+      : message_loop_(MessageLoop::TYPE_UI),
+        ui_thread_(content::BrowserThread::UI, &message_loop_),
+        file_thread_(content::BrowserThread::FILE, &message_loop_),
+        pointer_factory_(this),
+        local_state_(static_cast<TestingBrowserProcess*>(g_browser_process)) {
+  }
+
+  virtual ~DeviceSettingsProviderTest() {
+  }
+
+  virtual void SetUp() {
+    metrics_service_ = new MetricsService;
+    static_cast<TestingBrowserProcess*>(g_browser_process)->SetMetricsService(
+        metrics_service_);
+
+    PrepareEmptyPolicy();
+
+    EXPECT_CALL(signed_settings_helper_, StartRetrievePolicyOp(_))
+        .Times(AnyNumber())
+        .WillRepeatedly(
+            MockSignedSettingsHelperRetrievePolicy(SignedSettings::SUCCESS,
+                                                   policy_blob_));
+    EXPECT_CALL(signed_settings_helper_, StartStorePolicyOp(_,_))
+        .Times(AnyNumber())
+        .WillRepeatedly(DoAll(
+            SaveArg<0>(&policy_blob_),
+            MockSignedSettingsHelperStorePolicy(SignedSettings::SUCCESS)));
+
+    mock_user_manager_.reset(new MockUserManager());
+    old_user_manager_ = UserManager::Set(mock_user_manager_.get());
+    EXPECT_CALL(*mock_user_manager_, IsCurrentUserOwner())
+        .Times(AnyNumber())
+        .WillRepeatedly(Return(true));
+
+    provider_.reset(
+        new DeviceSettingsProvider(
+            base::Bind(&DeviceSettingsProviderTest::SettingChanged,
+                       pointer_factory_.GetWeakPtr()),
+            &signed_settings_helper_,
+            OwnershipService::OWNERSHIP_TAKEN));
+
+    trusted_called_ = false;
+    notifier_called_ = false;
+  }
+
+  virtual void TearDown() {
+    UserManager::Set(old_user_manager_);
+  }
+
+  void PrepareEmptyPolicy() {
+    em::PolicyData policy;
+    em::ChromeDeviceSettingsProto pol;
+    // Set metrics to disabled to prevent us from running into code that is not
+    // mocked.
+    pol.mutable_metrics_enabled()->set_metrics_enabled(false);
+    policy.set_policy_type(chromeos::kDevicePolicyType);
+    policy.set_username("me@owner");
+    policy.set_policy_value(pol.SerializeAsString());
+    // Wipe the signed settings store.
+    policy_blob_.set_policy_data(policy.SerializeAsString());
+    policy_blob_.set_policy_data_signature("false");
+  }
+
+  em::PolicyFetchResponse policy_blob_;
+
+  scoped_ptr<DeviceSettingsProvider> provider_;
+
+  MessageLoop message_loop_;
+  content::TestBrowserThread ui_thread_;
+  content::TestBrowserThread file_thread_;
+
+  base::WeakPtrFactory<DeviceSettingsProviderTest> pointer_factory_;
+
+  ScopedTestingLocalState local_state_;
+
+  MockSignedSettingsHelper signed_settings_helper_;
+  scoped_ptr<MockUserManager> mock_user_manager_;

[Mattias Nissler (ping if slow), 2012/03/16 15:21:11]

Does this need the scoped_ptr wrapping?

[pastarmovj, 2012/03/21 15:23:27]

Yes after my UserManager CL that Nikita is reviewing lands it's responsibility
of the unit test to destroy the mock manager.

+  UserManager* old_user_manager_;
+
+  ScopedStubCrosEnabler stub_cros_enabler_;
+
+  MetricsService* metrics_service_;

[Mattias Nissler (ping if slow), 2012/03/16 15:21:11]

Seems there is no reason to keep this pointer in a member.

[pastarmovj, 2012/03/21 15:23:27]

True fixed.

+
+  bool trusted_called_;
+  bool notifier_called_;
+};
+
+TEST_F(DeviceSettingsProviderTest, InitializationTest) {
+  // Verify that the policy blob has been correctly parsed and trusted.
+  ASSERT_TRUE(provider_->GetTrusted(

[Mattias Nissler (ping if slow), 2012/03/16 15:21:11]

EXPECT_TRUE seems to be good enough

[pastarmovj, 2012/03/21 15:23:27]

Done.

+      kStatsReportingPref,
+      base::Bind(&DeviceSettingsProviderTest::GetTrustedCallback,
+                 pointer_factory_.GetWeakPtr())));
+  // The trusted flag should be established already prior to calling GetTrusted.
+  message_loop_.RunAllPending();
+  ASSERT_FALSE(trusted_called_);

[Mattias Nissler (ping if slow), 2012/03/16 15:21:11]

same here.

[pastarmovj, 2012/03/21 15:23:27]

Line is gone.

+  const base::Value* value = provider_->Get(kStatsReportingPref);
+  ASSERT_TRUE(value);
+  bool bool_value;
+  ASSERT_TRUE(value->GetAsBoolean(&bool_value));
+  ASSERT_FALSE(bool_value);

[Mattias Nissler (ping if slow), 2012/03/16 15:21:11]

and here
and below

[pastarmovj, 2012/03/21 15:23:27]

Done.

+}
+
+TEST_F(DeviceSettingsProviderTest, InitializationTestUnowned) {
+  // No calls to the SignedSettingsHelper shoud occur in this case!

[Mattias Nissler (ping if slow), 2012/03/16 15:21:11]

s/shoud/should/
also below

[pastarmovj, 2012/03/21 15:23:27]

Done.

+  Mock::VerifyAndClear(&signed_settings_helper_);
+
+  provider_->set_ownership_status(OwnershipService::OWNERSHIP_NONE);
+  provider_->Reload();
+  // Verify that the cache policy blob is "trusted".
+  ASSERT_TRUE(provider_->GetTrusted(
+      kReleaseChannel,
+      base::Bind(&DeviceSettingsProviderTest::GetTrustedCallback,
+                 pointer_factory_.GetWeakPtr())));
+  // The trusted flag should be established already prior to calling GetTrusted.
+  message_loop_.RunAllPending();
+  ASSERT_FALSE(trusted_called_);
+  const base::Value* value = provider_->Get(kReleaseChannel);
+  ASSERT_TRUE(value);
+  std::string string_value;
+  ASSERT_TRUE(value->GetAsString(&string_value));
+  ASSERT_TRUE(string_value.empty());
+
+  // Sets should succeed though and be readable from the cache.
+  base::StringValue new_value("stable-channel");
+  provider_->Set(kReleaseChannel, new_value);
+  // Do one more reload here to make sure we don't flip randomly between stores.
+  provider_->Reload();
+  // Verify the change has not been apply.

[Mattias Nissler (ping if slow), 2012/03/16 15:21:11]

s/apply/applied/

[pastarmovj, 2012/03/21 15:23:27]

Done.

+  const base::Value* saved_value = provider_->Get(kReleaseChannel);
+  ASSERT_TRUE(saved_value);
+  ASSERT_TRUE(saved_value->GetAsString(&string_value));
+  ASSERT_EQ("stable-channel", string_value);
+}
+
+TEST_F(DeviceSettingsProviderTest, SetPrefFailed) {
+  // If we are not the owner no sets should work.
+  EXPECT_CALL(*mock_user_manager_, IsCurrentUserOwner())
+      .WillOnce(Return(false));
+  base::FundamentalValue value(true);
+  provider_->Set(kStatsReportingPref, value);
+  // Verify the change has not been apply.

[Mattias Nissler (ping if slow), 2012/03/16 15:21:11]

s/apply/applied/
also below.

[pastarmovj, 2012/03/21 15:23:27]

Done.

+  const base::Value* saved_value = provider_->Get(kStatsReportingPref);
+  ASSERT_TRUE(saved_value);
+  bool bool_value;
+  ASSERT_TRUE(saved_value->GetAsBoolean(&bool_value));
+  ASSERT_FALSE(bool_value);
+}
+
+TEST_F(DeviceSettingsProviderTest, SetPrefSucceed) {
+  base::FundamentalValue value(true);
+  provider_->Set(kStatsReportingPref, value);
+  // Verify the change has not been apply.
+  const base::Value* saved_value = provider_->Get(kStatsReportingPref);
+  ASSERT_TRUE(saved_value);
+  bool bool_value;
+  ASSERT_TRUE(saved_value->GetAsBoolean(&bool_value));
+  ASSERT_TRUE(bool_value);
+}
+
+TEST_F(DeviceSettingsProviderTest, PolicyRetrievalFailed) {
+  // No calls to the SignedSettingsHelper shoud occur in this case!
+  Mock::VerifyAndClear(&signed_settings_helper_);
+  EXPECT_CALL(signed_settings_helper_, StartRetrievePolicyOp(_))
+      .Times(AnyNumber())
+      .WillRepeatedly(
+          MockSignedSettingsHelperRetrievePolicy(
+              SignedSettings::BAD_SIGNATURE,
+              policy_blob_));
+  provider_->Reload();
+  // Verify that the cache policy blob is "trusted".

[Mattias Nissler (ping if slow), 2012/03/16 15:21:11]

Don't you initialize with OWNERSHIP_TAKEN? Shouldn't the trusted checks fail
then?

[pastarmovj, 2012/03/21 15:23:27]

True and this is what the code does but the comment doesn't. :)

+  ASSERT_FALSE(provider_->GetTrusted(
+      kReleaseChannel,
+      base::Bind(&DeviceSettingsProviderTest::GetTrustedCallback,
+                 pointer_factory_.GetWeakPtr())));
+  // The trusted flag should be established already prior to calling GetTrusted.
+  message_loop_.RunAllPending();
+  ASSERT_FALSE(trusted_called_);
+}
+
+TEST_F(DeviceSettingsProviderTest, PolicyRetrievalFailed2) {

[Mattias Nissler (ping if slow), 2012/03/16 15:21:11]

Can you make the naming of the test reflect the difference between the two?

[pastarmovj, 2012/03/21 15:23:27]

Done.

+  // No calls to the SignedSettingsHelper shoud occur in this case!
+  Mock::VerifyAndClear(&signed_settings_helper_);
+  EXPECT_CALL(signed_settings_helper_, StartRetrievePolicyOp(_))
+      .Times(AnyNumber())
+      .WillRepeatedly(
+          MockSignedSettingsHelperRetrievePolicy(
+              SignedSettings::OPERATION_FAILED,
+              policy_blob_));
+  provider_->Reload();
+  // Verify that the cache policy blob is "trusted".
+  ASSERT_FALSE(provider_->GetTrusted(
+      kReleaseChannel,
+      base::Bind(&DeviceSettingsProviderTest::GetTrustedCallback,
+                 pointer_factory_.GetWeakPtr())));
+  // The trusted flag should be established already prior to calling GetTrusted.
+  message_loop_.RunAllPending();
+  ASSERT_FALSE(trusted_called_);
+}
+
+} // namespace chromeos