MediaPerceptionPrivate API impl and testing.

extensions/browser/api/media_perception_private/media_perception_api_manager_unittest.cc

Index: extensions/browser/api/media_perception_private/media_perception_api_manager_unittest.cc
diff --git a/extensions/browser/api/media_perception_private/media_perception_api_manager_unittest.cc b/extensions/browser/api/media_perception_private/media_perception_api_manager_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..2872b5346ffdc180cc1b2ad69074a27270eaa176
--- /dev/null
+++ b/extensions/browser/api/media_perception_private/media_perception_api_manager_unittest.cc
@@ -0,0 +1,102 @@
+// 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 "extensions/browser/api/media_perception_private/media_perception_api_manager.h"
+
+#include "base/bind.h"
+#include "base/memory/ptr_util.h"
+#include "base/run_loop.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/fake_media_analytics_client.h"
+#include "chromeos/dbus/fake_upstart_client.h"
+#include "chromeos/dbus/media_analytics_client.h"
+#include "chromeos/dbus/upstart_client.h"
+#include "content/public/test/test_browser_context.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace media_perception = extensions::api::media_perception_private;
+
+namespace extensions {
+
+using CallbackStatus = MediaPerceptionAPIManager::CallbackStatus;
+
+class MediaPerceptionAPIManagerTest : public testing::Test {
+ public:
+  MediaPerceptionAPIManagerTest()
+      : thread_bundle_(content::TestBrowserThreadBundle::DEFAULT) {}
+
+  void SetUp() override {
+    browser_context_ = base::MakeUnique<content::TestBrowserContext>();
+
+    std::unique_ptr<chromeos::DBusThreadManagerSetter> dbus_setter =
+        chromeos::DBusThreadManager::GetSetterForTesting();
+    auto media_analytics_client =
+        base::MakeUnique<chromeos::FakeMediaAnalyticsClient>();
+    media_analytics_client_ = media_analytics_client.get();
+    dbus_setter->SetMediaAnalyticsClient(std::move(media_analytics_client));
+
+    auto upstart_client = base::MakeUnique<chromeos::FakeUpstartClient>();
+    upstart_client_ = upstart_client.get();
+    dbus_setter->SetUpstartClient(std::move(upstart_client));
+
+    chromeos::DBusThreadManager::Initialize();

[tbarzic, 2017/05/16 20:56:31]

you don't need this :)

[Luke Sorenson, 2017/05/16 21:33:33]

Done.

+    manager_ =
+        base::MakeUnique<MediaPerceptionAPIManager>(browser_context_.get());
+  }
+
+  void TearDown() override {
+    // Need to make sure that the MediaPerceptionAPIManager is destructed before
+    // the DbusThreadManager.
+    manager_.reset();
+    chromeos::DBusThreadManager::Shutdown();
+  }
+
+  std::unique_ptr<MediaPerceptionAPIManager> manager_;
+  std::unique_ptr<content::TestBrowserContext> browser_context_;

[tbarzic, 2017/05/16 20:56:31]

content::TestBrowserContext browser_context_;

[Luke Sorenson, 2017/05/16 21:33:33]

Done.

+
+  // Ownership of both is passed on to chromeos::DbusThreadManager.
+  chromeos::FakeMediaAnalyticsClient* media_analytics_client_;
+  chromeos::FakeUpstartClient* upstart_client_;
+
+  content::TestBrowserThreadBundle thread_bundle_;

[tbarzic, 2017/05/16 20:56:30]

nit: move up with browser_context

[Luke Sorenson, 2017/05/16 21:33:33]

Done.

+};

[tbarzic, 2017/05/16 20:56:31]

Add DISALLOW_COPY_AND_ASSIGN

[Luke Sorenson, 2017/05/16 21:33:33]

Done.

+
+void SetStateUpstartFailureCallback(CallbackStatus status,

[tbarzic, 2017/05/16 20:56:31]

move before test calss, and put it into an anonymous namespace

[Luke Sorenson, 2017/05/16 21:33:33]

Done.

+                                    media_perception::State state) {
+  EXPECT_EQ(status, CallbackStatus::PROCESS_IDLE_ERROR);
+}
+
+TEST_F(MediaPerceptionAPIManagerTest, UpstartFailure) {
+  upstart_client_->SetStartMediaAnalyticsWillSucceed(false);
+  media_perception::State state;
+  state.status = media_perception::STATUS_RUNNING;
+  manager_->SetState(state, base::Bind(&SetStateUpstartFailureCallback));

[tbarzic, 2017/05/16 20:56:31]

given that SetState is async you should use run loop here too (e.g. see if the
test fails if you change SetStateUpstartFailureCallback to expect something
else).

What I like to do is:

void RecordStatusAndRunClosure(Closure closure,
                               CallbackStatus* status,
                               CallbackStatus result_status,
                               State result_state) {
  *status = result_status;
  closure.Run();
}

CallbackStatus SetState(state) {
  base::RunLoop run_loop;
  CallbackStatus status;
  manager_->SetState(state, 
                     base::Bind(&RecordStatusAndRunClosure,
                                run_loop.QuitClosure(),
                                &status));
  run_loop.Run();
  return status;
}


TEST_F() {
  EXPECT_EQ(STATUS, SetState(state));
}


Though, I'm OK with inlining run_loop handing into tests (might make tests
easier to follow).

[Luke Sorenson, 2017/05/16 21:33:33]

Done.

+}
+
+void SetStateSuccessCallback(const base::Closure& quit_run_loop,
+                             CallbackStatus status,
+                             media_perception::State state) {
+  EXPECT_EQ(status, CallbackStatus::SUCCESS);
+  EXPECT_EQ(state.status, media_perception::STATUS_RUNNING);
+  quit_run_loop.Run();
+}
+
+void GetStateFailureDbusCallback(CallbackStatus status,
+                                 media_perception::State state) {
+  EXPECT_EQ(status, CallbackStatus::DBUS_ERROR);
+}
+
+TEST_F(MediaPerceptionAPIManagerTest, MediaAnalyticsDbusError) {
+  media_perception::State state;
+  state.status = media_perception::STATUS_RUNNING;
+  base::RunLoop run_loop;
+  manager_->SetState(
+      state, base::Bind(&SetStateSuccessCallback, run_loop.QuitClosure()));
+  run_loop.Run();
+  media_analytics_client_->set_process_running(false);
+  manager_->GetState(base::Bind(&GetStateFailureDbusCallback));
+}
+
+}  // namespace extensions