MD Settings: Adding unit test for ResetSettingsHandler.

chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc

+// Copyright 2015 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 "base/message_loop/message_loop.h"
+#include "base/values.h"
+#include "chrome/browser/profile_resetter/profile_resetter.h"
+#include "chrome/browser/ui/webui/settings/reset_settings_handler.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/public/browser/web_ui_data_source.h"
+#include "content/public/test/test_browser_thread.h"
+#include "content/public/test/test_web_ui.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using settings::ResetSettingsHandler;
+
+namespace {
+
+/* Mock version of ProfileResetter to be used in tests. */

[Dan Beam, 2015/12/01 04:27:37]

// Single line comment.

[dpapad, 2015/12/01 19:06:04]

Done.

+class MockProfileResetter : public ProfileResetter {
+ public:
+  explicit MockProfileResetter(TestingProfile* profile)
+      : ProfileResetter(profile) {
+  }
+
+  bool IsActive() const override {

[Dan Beam, 2015/12/01 17:29:04]

battre@: here ^

+    return false;
+  }
+
+  void Reset(ResettableFlags resettable_flags,
+             scoped_ptr<BrandcodedDefaultSettings> master_settings,
+             const base::Closure& callback) override {

[Dan Beam, 2015/12/01 17:29:04]

battre@: and here ^

+    resets_++;
+    callback.Run();
+  }
+
+  int resets() const {
+    return resets_;
+  }
+
+ private:
+  int resets_ = 0;

[Dan Beam, 2015/12/01 04:27:37]

nit: size_t

[dpapad, 2015/12/01 19:06:04]

Done.

+};
+
+
+class TestingResetSettingsHandler : public ResetSettingsHandler {
+ public:
+  TestingResetSettingsHandler(
+      TestingProfile* profile, bool allow_powerwash, content::WebUI* web_ui)
+      : ResetSettingsHandler(profile, allow_powerwash),
+        profile_(make_scoped_ptr(profile)),
+        resetter_(profile) {
+    set_web_ui(web_ui);
+  }
+
+  ~TestingResetSettingsHandler() override {
+    set_web_ui(nullptr);
+  }
+
+  MockProfileResetter* GetResetter() override {
+    return &resetter_;
+  }
+
+  using settings::ResetSettingsHandler::HandleResetProfileSettings;
+
+ private:
+  scoped_ptr<TestingProfile> profile_;

[Dan Beam, 2015/12/01 04:27:37]

why should the handler own the profile?  this should be owned by the fixture

+  MockProfileResetter resetter_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestingResetSettingsHandler);
+};
+
+// The fixture for testing class Foo.
+class ResetSettingsHandlerTest : public testing::Test {
+ public:
+  ResetSettingsHandlerTest()
+    : web_ui_(new content::TestWebUI()),
+      ui_thread_(content::BrowserThread::UI, &message_loop_) {
+    handler_.reset(new TestingResetSettingsHandler(
+        new TestingProfile(), false, web_ui_.get()));
+  }
+
+ protected:
+  // The order here matters, because the |message_loop_| needs to exist during
+  // destruction of |handler_|.
+  base::MessageLoopForUI message_loop_;
+  scoped_ptr<TestingResetSettingsHandler> handler_;

[Dan Beam, 2015/12/01 04:27:37]

nit: instead of protected member, private member + public accessor to mutable
pointer

[Dan Beam, 2015/12/01 04:27:37]

why does this need to be a scoped_ptr?  just initialize these in the right
order:

 public:
  ResetSettingsHandlerTest
      : ui_thread_(content::BrowserThread::UI, &message_loop_),
        handler_(&profile_, false, &web_ui_) {}

 private:
  base::MessageLoopForUI message_loop_;
  content::TestBrowserThread ui_thread_;
  TestingProfile profile_;
  content::TestWebUI web_ui_;
  TestResetSettingsHandler handler_;

[dpapad, 2015/12/01 19:06:04]

Done. Also removed usage of deprecated TestBrowserThread (see
https://code.google.com/p/chromium/codesearch#chromium/src/content/public/tes...)
and replaced with content::TestBrowserThreadBundle.

+  scoped_ptr<content::TestWebUI> web_ui_;

[Dan Beam, 2015/12/01 04:27:37]

why does this need to be a scoped_ptr?

[dpapad, 2015/12/01 19:06:04]

Removed.

+
+ private:
+  content::TestBrowserThread ui_thread_;
+};
+
+
+TEST_F(ResetSettingsHandlerTest, HandleResetProfileSettings) {
+  base::ListValue list;
+  list.AppendBoolean(false);
+  handler_->HandleResetProfileSettings(&list);
+  // Check that the delegate ProfileResetter was called.
+  EXPECT_EQ(1, handler_->GetResetter()->resets());
+  // Check that Javascript side is notified after resetting is done.
+  EXPECT_EQ("SettingsResetPage.doneResetting",
+            web_ui_->call_data()[0]->function_name());

[Dan Beam, 2015/12/01 04:27:37]

doesn't this effectively check that resets_ >= 1?

[dpapad, 2015/12/01 19:06:04]

It does. The assertion at line 101 checks that resets_ == 1, not just >=1.

[Dan Beam, 2015/12/02 04:40:07]

ok

+}
+
+
+}  // namespace