Add unit test for request coordiator

components/offline_pages/background/request_coordinator_unittest.cc

Index: components/offline_pages/background/request_coordinator_unittest.cc
diff --git a/components/offline_pages/background/request_coordinator_unittest.cc b/components/offline_pages/background/request_coordinator_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..3942b67d98d1820b9995c3edf96c8e279edd8356
--- /dev/null
+++ b/components/offline_pages/background/request_coordinator_unittest.cc
@@ -0,0 +1,72 @@
+// Copyright 2016 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/offline_pages/background/request_coordinator.h"
+
+#include "base/bind.h"
+#include "base/memory/weak_ptr.h"
+#include "components/offline_pages/background/save_page_request.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace offline_pages {
+
+namespace {
+// put test constants here
+const int64_t kRequestId = 42;
+const GURL kUrl("http://universe.com/everything");
+const ClientId kClientId("life", "1234");

[fgorski, 2016/05/03 22:55:49]

can we have some test namespace name, that will work later when the policy
arrives?

[Pete Williamson, 2016/05/04 17:29:27]

Done.

+
+

[dougarnett, 2016/05/03 22:42:06]

drop extra line?

[Pete Williamson, 2016/05/03 22:51:47]

Done.

+// any helper functions that we need to here
+}  // namespace
+
+class RequestCoordinatorTest
+    : public testing::Test,
+      public base::SupportsWeakPtr<RequestCoordinatorTest> {
+ public:
+  RequestCoordinatorTest();
+  ~RequestCoordinatorTest() override;
+
+  void SetUp() override;
+  void TearDown() override;
+
+  void callbackFunction() {
+    // Notify that we have been called
+    callback_called = true;
+  }
+
+ private:
+  // Any test class data we might need
+  bool callback_called;

[fgorski, 2016/05/03 22:55:50]

callback_called_

Remove please if you don't intend to use it in the test.

[Pete Williamson, 2016/05/04 17:29:27]

Done.

+};
+
+RequestCoordinatorTest::RequestCoordinatorTest() {}
+
+RequestCoordinatorTest::~RequestCoordinatorTest() {}
+
+void RequestCoordinatorTest::SetUp() {
+  callback_called = false;
+}
+
+void RequestCoordinatorTest::TearDown() {

[fgorski, 2016/05/03 22:55:50]

not needed.

[Pete Williamson, 2016/05/04 17:29:27]

Done.

+}
+
+/* Test with out/d/bin/run_components_unittests -vv -f RequestCoordinator* */

[fgorski, 2016/05/03 22:55:50]

Don't put that in. Perhaps this go in the test section on the team page.
Also it does not work on my machine as my debug dir is different...

[Pete Williamson, 2016/05/04 17:29:27]

Done.

+
+TEST_F(RequestCoordinatorTest, StartProcessingTest) {

[fgorski, 2016/05/03 22:55:50]

don't append test to method name. You already have it on the fixture name

[Pete Williamson, 2016/05/04 17:29:27]

Done.

+  RequestCoordinator::ProcessingDoneCallback callback =
+      base::Bind(&RequestCoordinatorTest::callbackFunction, AsWeakPtr());
+  RequestCoordinator coordinator;

[fgorski, 2016/05/03 22:55:50]

please add a comment above explaining what you are testing for, or rename the
test appropriately, e.g. StartProcessingWithNoRequests

[Pete Williamson, 2016/05/04 17:29:27]

Done.

+  bool return_value = coordinator.StartProcessing(callback);

[fgorski, 2016/05/03 22:55:50]

this and next is a single line.

[Pete Williamson, 2016/05/04 17:29:27]

Done.

+  EXPECT_FALSE(return_value);
+}
+
+TEST_F(RequestCoordinatorTest, SavePageLaterTest) {

[fgorski, 2016/05/03 22:55:49]

remove Test from second part.

[Pete Williamson, 2016/05/04 17:29:27]

Done.

+  RequestCoordinator coordinator;
+  SavePageRequest request(kRequestId, kUrl, kClientId, base::Time::Now());
+  bool return_value = coordinator.SavePageLater(request);

[fgorski, 2016/05/03 22:55:50]

single line.

[Pete Williamson, 2016/05/04 17:29:27]

Done.

+  EXPECT_TRUE(return_value);
+}
+
+}  // namespace offline_pages