[NTP::Push] Add the classes for sending a breaking news subscription request

components/ntp_snippets/breaking_news/subscription_json_request_unittest.cc

Index: components/ntp_snippets/breaking_news/subscription_json_request_unittest.cc
diff --git a/components/ntp_snippets/breaking_news/subscription_json_request_unittest.cc b/components/ntp_snippets/breaking_news/subscription_json_request_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b017f6e472be37a675b5a720d92c9edd0db9db23
--- /dev/null
+++ b/components/ntp_snippets/breaking_news/subscription_json_request_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 "components/ntp_snippets/breaking_news/subscription_json_request.h"
+
+#include <set>
+#include <utility>
+
+#include "base/json/json_reader.h"
+#include "base/memory/ptr_util.h"
+#include "base/strings/stringprintf.h"
+#include "base/test/test_mock_time_task_runner.h"
+#include "base/time/tick_clock.h"
+#include "base/time/time.h"
+#include "base/values.h"
+#include "components/ntp_snippets/breaking_news/subscription_request_params.h"
+#include "components/ntp_snippets/features.h"
+#include "components/ntp_snippets/ntp_snippets_constants.h"
+#include "components/prefs/testing_pref_service.h"
+#include "components/variations/variations_params_manager.h"

[fhorschig, 2017/05/31 14:51:14]

There are quite some includes you wont need here:

set
test_mock_time_task_runner
time
tick_clock
features
snippets_contants?
pref_service
variations_params_manager
test_url_fetcher_factory

and maybe more ^^

[mamir, 2017/05/31 18:46:03]

Well, actually test_mock_time_task_runner is needed ;-)

But I have cleaned the imports.

[fhorschig, 2017/06/01 09:23:55]

Let's discuss :D

[mamir, 2017/06/01 14:52:30]

Acknowledged.

+#include "net/url_request/test_url_fetcher_factory.h"
+#include "net/url_request/url_request_test_util.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace ntp_snippets {
+
+namespace internal {
+
+namespace {
+
+using testing::_;

[fhorschig, 2017/05/31 14:51:14]

never used

[mamir, 2017/05/31 18:46:03]

Done.

+using testing::Eq;

[fhorschig, 2017/05/31 14:51:14]

never used

[mamir, 2017/05/31 18:46:03]

Done.

+using testing::Not;

[fhorschig, 2017/05/31 14:51:14]

never used

[mamir, 2017/05/31 18:46:03]

Done.

+using testing::NotNull;

[fhorschig, 2017/05/31 14:51:14]

never used

[mamir, 2017/05/31 18:46:03]

Done.

+using testing::StrEq;
+
+MATCHER_P(EqualsJSON, json, "equals JSON") {

[fhorschig, 2017/05/31 14:51:14]

Consider creating a test_helper.cc file instead of duplicating all this code.

[mamir, 2017/05/31 18:46:03]

Could we please discuss this offline?

[fhorschig, 2017/06/01 09:23:55]

sure

[mamir, 2017/06/01 14:52:30]

Adding a TODO as per our offline discussion.

+  std::unique_ptr<base::Value> expected = base::JSONReader::Read(json);
+  if (!expected) {
+    *result_listener << "INTERNAL ERROR: couldn't parse expected JSON";
+    return false;
+  }
+
+  std::string err_msg;
+  int err_line, err_col;
+  std::unique_ptr<base::Value> actual = base::JSONReader::ReadAndReturnError(
+      arg, base::JSON_PARSE_RFC, nullptr, &err_msg, &err_line, &err_col);
+  if (!actual) {
+    *result_listener << "input:" << err_line << ":" << err_col << ": "
+                     << "parse error: " << err_msg;
+    return false;
+  }
+  return base::Value::Equals(actual.get(), expected.get());
+}
+
+}  // namespace
+
+class SubscriptionJsonRequestTest : public testing::Test {
+ public:
+  SubscriptionJsonRequestTest()
+      : pref_service_(base::MakeUnique<TestingPrefServiceSimple>()),
+        mock_task_runner_(new base::TestMockTimeTaskRunner()),
+        request_context_getter_(
+            new net::TestURLRequestContextGetter(mock_task_runner_.get())) {}
+
+  SubscriptionJsonRequest::Builder CreateMinimalBuilder() {
+    SubscriptionJsonRequest::Builder builder;
+    builder.SetUrl(GURL("http://valid-url.test"))
+        .SetUrlRequestContextGetter(request_context_getter_.get());
+    return builder;
+  }
+
+ private:
+  std::unique_ptr<TestingPrefServiceSimple> pref_service_;

[fhorschig, 2017/05/31 14:51:14]

You don't need that right now.

[mamir, 2017/05/31 18:46:04]

Done.

+  scoped_refptr<base::TestMockTimeTaskRunner> mock_task_runner_;

[fhorschig, 2017/05/31 14:51:14]

You don't need that right now.

[mamir, 2017/05/31 18:46:03]

I do the initialize the TestURLRequestContextGetter

[fhorschig, 2017/06/01 09:23:55]

This is a very heavy cannon you use there.
It's easier to just use the message loop.
(the helpful link you look for is in another comment)

[mamir, 2017/06/01 14:52:30]

Acknowledged.

+  scoped_refptr<net::TestURLRequestContextGetter> request_context_getter_;
+  net::TestURLFetcherFactory fetcher_factory_;

[fhorschig, 2017/05/31 14:51:14]

You never used that anywhere.

[mamir, 2017/05/31 18:46:03]

When I remove it the test crashes.  I don't know why especially it makes no
sense.

[fhorschig, 2017/06/01 09:23:55]

Oh, right. It replaces the factory for URLFetchers.
You actually called "Build()" (even if unnecessary) which
instantiates a fetcher.
Would be nice to have tests to actually use that (see later PS)

[Bernhard Bauer, 2017/06/01 09:31:23]

Yeah, if you're mocking out the URLFetchers anyway, you could just send off the
"network requests" and verify the request data. Then once you have more
sophisticated response parsing, you could exercise that as well.

[mamir, 2017/06/01 14:52:30]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(SubscriptionJsonRequestTest);
+};
+
+TEST_F(SubscriptionJsonRequestTest, BuildRequest) {
+  SubscriptionJsonRequest::Builder builder = CreateMinimalBuilder();
+  SubscriptionRequestParams params;
+  params.token = "1234567890";
+  builder.SetParams(params).SetUrl(GURL("http://valid-url.test")).Build();
+
+  EXPECT_THAT(builder.PreviewRequestHeadersForTesting(),
+              StrEq("Content-Type: application/json; charset=UTF-8\r\n"
+                    "\r\n"));
+  EXPECT_THAT(builder.PreviewRequestBodyForTesting(),
+              EqualsJSON("{"
+                         "  \"token\": "
+                         "    \"1234567890\""
+                         "}"));
+}
+
+}  // namespace internal
+
+}  // namespace ntp_snippets