[Doodle] Introduce a DoodleService

components/doodle/doodle_service_unittest.cc

Index: components/doodle/doodle_service_unittest.cc
diff --git a/components/doodle/doodle_service_unittest.cc b/components/doodle/doodle_service_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..c5427559c462f63a8e1a6825b25f346561ef92e8
--- /dev/null
+++ b/components/doodle/doodle_service_unittest.cc
@@ -0,0 +1,202 @@
+// Copyright 2017 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 "components/doodle/doodle_service.h"
+
+#include <memory>
+#include <utility>
+#include <vector>
+
+#include "base/bind.h"
+#include "base/memory/ptr_util.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using testing::Eq;
+using testing::StrictMock;
+
+namespace doodle {
+
+namespace {
+
+class FakeDoodleFetcher : public DoodleFetcher {
+ public:
+  FakeDoodleFetcher() = default;
+  ~FakeDoodleFetcher() override = default;
+
+  void FetchDoodle(FinishedCallback callback) override {
+    callbacks_.push_back(std::move(callback));
+  }
+
+  size_t num_pending_callbacks() const { return callbacks_.size(); }
+
+  void ServeAllCallbacks(DoodleState state,
+                         const base::Optional<DoodleConfig>& config) {
+    for (auto& callback : callbacks_) {
+      std::move(callback).Run(state, config);
+    }
+    callbacks_.clear();
+  }
+
+ private:
+  std::vector<FinishedCallback> callbacks_;
+};
+
+class MockDoodleObserver : public DoodleService::Observer {
+ public:
+  MOCK_METHOD1(OnDoodleConfigUpdated,
+               void(const base::Optional<DoodleConfig>&));
+};
+
+}  // namespace
+
+// Equality operator for DoodleConfigs, for use by testing::Eq.
+// Note: This must be outside of the anonymous namespace.
+bool operator==(const DoodleConfig& lhs, const DoodleConfig& rhs) {
+  return lhs.IsEquivalent(rhs);
+}
+
+class DoodleServiceTest : public testing::Test {
+ public:
+  DoodleServiceTest() : fetcher_(nullptr) {
+    auto fetcher = base::MakeUnique<FakeDoodleFetcher>();
+    fetcher_ = fetcher.get();
+    service_ = base::MakeUnique<DoodleService>(std::move(fetcher));
+  }
+
+  DoodleService* service() { return service_.get(); }
+  FakeDoodleFetcher* fetcher() { return fetcher_; }
+
+ private:
+  std::unique_ptr<DoodleService> service_;
+  FakeDoodleFetcher* fetcher_;
+};
+
+TEST_F(DoodleServiceTest, FetchesConfigOnRefresh) {

[vitaliii, 2017/02/28 11:37:18]

I felt like we prefer 'Should-Do-What-When' to 'Does-What-When'.
However, the testing doc says: '"Does-What-When" notation is okay if you want to
emphasize an action.'

[Marc Treib, 2017/02/28 13:06:49]

Ah, I didn't know we had a preference. I named these consistently with the
existing DoodleFetcherImpl tests, which also use this style.

+  ASSERT_THAT(service()->config(), Eq(base::nullopt));
+
+  // Request a refresh of the doodle config.
+  service()->Refresh();
+  // The request should have arrived at the fetcher.
+  EXPECT_THAT(fetcher()->num_pending_callbacks(), Eq(1u));
+
+  // Serve it (with an arbitrary config).
+  DoodleConfig config;
+  config.doodle_type = DoodleType::SIMPLE;
+  fetcher()->ServeAllCallbacks(DoodleState::AVAILABLE, config);
+
+  // The config should be available.
+  EXPECT_THAT(service()->config(), Eq(config));
+
+  // Request a refresh again.
+  service()->Refresh();
+  // The request should have arrived at the fetcher again.
+  EXPECT_THAT(fetcher()->num_pending_callbacks(), Eq(1u));
+
+  // Serve it with a different config.
+  DoodleConfig config2;
+  config2.doodle_type = DoodleType::SLIDESHOW;
+  DCHECK(!config.IsEquivalent(config2));
+  fetcher()->ServeAllCallbacks(DoodleState::AVAILABLE, config2);
+
+  // The config should have been updated.
+  EXPECT_THAT(service()->config(), Eq(config2));
+}
+
+TEST_F(DoodleServiceTest, CallsObserverOnConfigReceived) {
+  StrictMock<MockDoodleObserver> observer;
+  service()->AddObserver(&observer);
+
+  ASSERT_THAT(service()->config(), Eq(base::nullopt));
+
+  // Request a refresh of the doodle config.
+  service()->Refresh();
+  // The request should have arrived at the fetcher.
+  ASSERT_THAT(fetcher()->num_pending_callbacks(), Eq(1u));
+
+  // Serve it (with an arbitrary config). The observer should get notified.
+  DoodleConfig config;
+  config.doodle_type = DoodleType::SIMPLE;
+  EXPECT_CALL(observer, OnDoodleConfigUpdated(Eq(config)));
+  fetcher()->ServeAllCallbacks(DoodleState::AVAILABLE, config);
+
+  // Remove the observer before the service gets destroyed.
+  service()->RemoveObserver(&observer);
+}
+
+TEST_F(DoodleServiceTest, CallsObserverOnConfigRemoved) {
+  // Load some doodle config.
+  service()->Refresh();
+  DoodleConfig config;
+  config.doodle_type = DoodleType::SIMPLE;
+  fetcher()->ServeAllCallbacks(DoodleState::AVAILABLE, config);
+  ASSERT_THAT(service()->config(), Eq(config));
+
+  // Register an observer and request a refresh.
+  StrictMock<MockDoodleObserver> observer;
+  service()->AddObserver(&observer);
+  service()->Refresh();
+  ASSERT_THAT(fetcher()->num_pending_callbacks(), Eq(1u));
+
+  // Serve the request with an empty doodle config. The observer should get
+  // notified.
+  EXPECT_CALL(observer, OnDoodleConfigUpdated(Eq(base::nullopt)));
+  fetcher()->ServeAllCallbacks(DoodleState::NO_DOODLE, base::nullopt);
+
+  // Remove the observer before the service gets destroyed.
+  service()->RemoveObserver(&observer);
+}
+
+TEST_F(DoodleServiceTest, CallsObserverOnConfigUpdated) {
+  // Load some doodle config.
+  service()->Refresh();
+  DoodleConfig config;
+  config.doodle_type = DoodleType::SIMPLE;
+  fetcher()->ServeAllCallbacks(DoodleState::AVAILABLE, config);
+  ASSERT_THAT(service()->config(), Eq(config));
+
+  // Register an observer and request a refresh.
+  StrictMock<MockDoodleObserver> observer;
+  service()->AddObserver(&observer);
+  service()->Refresh();
+  ASSERT_THAT(fetcher()->num_pending_callbacks(), Eq(1u));
+
+  // Serve the request with a different doodle config. The observer should get
+  // notified.
+  DoodleConfig config2;
+  config2.doodle_type = DoodleType::SLIDESHOW;
+  DCHECK(!config.IsEquivalent(config2));

[vitaliii, 2017/02/28 11:37:18]

nit: making a typo in config2 is much easier than in second_config.

[Marc Treib, 2017/02/28 13:06:49]

Good point. Renamed.

+  EXPECT_CALL(observer, OnDoodleConfigUpdated(Eq(config2)));
+  fetcher()->ServeAllCallbacks(DoodleState::AVAILABLE, config2);
+
+  // Remove the observer before the service gets destroyed.
+  service()->RemoveObserver(&observer);
+}
+
+TEST_F(DoodleServiceTest, DoesNotCallObserverWhenConfigEquivalent) {
+  // Load some doodle config.
+  service()->Refresh();
+  DoodleConfig config;
+  config.doodle_type = DoodleType::SIMPLE;
+  fetcher()->ServeAllCallbacks(DoodleState::AVAILABLE, config);
+  ASSERT_THAT(service()->config(), Eq(config));
+
+  // Register an observer and request a refresh.
+  StrictMock<MockDoodleObserver> observer;
+  service()->AddObserver(&observer);
+  service()->Refresh();
+  ASSERT_THAT(fetcher()->num_pending_callbacks(), Eq(1u));
+
+  // Serve the request with an equivalent doodle config. The observer should get

[vitaliii, 2017/02/28 11:37:18]

should not?

[Marc Treib, 2017/02/28 13:06:49]

Indeed, thanks! Too much copypasta...

+  // notified.
+  DoodleConfig config2;
+  config2.doodle_type = DoodleType::SIMPLE;
+  DCHECK(config.IsEquivalent(config2));
+  fetcher()->ServeAllCallbacks(DoodleState::AVAILABLE, config2);
+
+  // Remove the observer before the service gets destroyed.
+  service()->RemoveObserver(&observer);
+}
+
+}  // namespace doodle