[NTP Snippets] FetchMore backend

components/ntp_snippets/remote/ntp_snippets_fetcher_unittest.cc

 // 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/ntp_snippets/remote/ntp_snippets_fetcher.h"
 
 #include <map>
 #include <utility>
 
 #include "base/json/json_reader.h"
@@ skipping 155 matching lines @@
     UserClassifier::RegisterProfilePrefs(pref_service_->registry());
     user_classifier_ = base::MakeUnique<UserClassifier>(pref_service_.get());
 
     snippets_fetcher_ = base::MakeUnique<NTPSnippetsFetcher>(
         fake_signin_manager_.get(), fake_token_service_.get(),
         scoped_refptr<net::TestURLRequestContextGetter>(
             new net::TestURLRequestContextGetter(mock_task_runner_.get())),
         pref_service_.get(), &category_factory_, nullptr,
         base::Bind(&ParseJsonDelayed), kAPIKey, user_classifier_.get());
 
-    snippets_fetcher_->SetCallback(
-        base::Bind(&MockSnippetsAvailableCallback::WrappedRun,
-                   base::Unretained(&mock_callback_)));
     snippets_fetcher_->SetTickClockForTesting(
         mock_task_runner_->GetMockTickClock());
     // Increase initial time such that ticks are non-zero.
     mock_task_runner_->FastForwardBy(base::TimeDelta::FromMilliseconds(1234));
   }
 
+  NTPSnippetsFetcher::SnippetsAvailableCallback MakeMockCallback() {
+    return base::BindOnce(&MockSnippetsAvailableCallback::WrappedRun,
+                          base::Unretained(&mock_callback_));
+  }
+
+  base::Optional<Category> NoExclusiveCategory() {
+    return base::Optional<Category>();
+  }
+
+  base::Optional<Category> OptionalArticlesCategory() {
+    return base::Optional<Category>(
+        CategoryFactory().FromKnownCategory(KnownCategories::ARTICLES));
+  }
+
   NTPSnippetsFetcher& snippets_fetcher() { return *snippets_fetcher_; }
   MockSnippetsAvailableCallback& mock_callback() { return mock_callback_; }
   void FastForwardUntilNoTasksRemain() {
     mock_task_runner_->FastForwardUntilNoTasksRemain();
   }
   base::HistogramTester& histogram_tester() { return histogram_tester_; }
 
   NTPSnippetsFetcher::Params test_params() {
     NTPSnippetsFetcher::Params result;
     result.count_to_fetch = 1;
@@ skipping 306 matching lines @@
       "    \"sourceCorpusInfo\" : [{"
       "      \"ampUrl\" : \"http://localhost/amp\","
       "      \"corpusId\" : \"http://localhost/foobar\","
       "      \"publisherData\": { \"sourceName\" : \"Foo News\" }"
       "    }]"
       "  }"
       "}]}";
   SetFakeResponse(/*response_data=*/kJsonStr, net::HTTP_OK,
                   net::URLRequestStatus::SUCCESS);
   EXPECT_CALL(mock_callback(), Run(IsSingleArticle("http://localhost/foobar")));
-  snippets_fetcher().FetchSnippets(test_params());
+  snippets_fetcher().FetchSnippets(test_params(), MakeMockCallback());
   FastForwardUntilNoTasksRemain();
   EXPECT_THAT(snippets_fetcher().last_status(), Eq("OK"));
   EXPECT_THAT(snippets_fetcher().last_json(), Eq(kJsonStr));
   EXPECT_THAT(histogram_tester().GetAllSamples(
                   "NewTabPage.Snippets.FetchHttpResponseOrErrorCode"),
               ElementsAre(base::Bucket(/*min=*/200, /*count=*/1)));
   EXPECT_THAT(histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchTime"),
               ElementsAre(base::Bucket(/*min=*/kTestJsonParsingLatencyMs,
                                        /*count=*/1)));
 }
@@ skipping 12 matching lines @@
       "    \"expirationTime\" : \"2016-07-01T11:01:37.000Z\","
       "    \"attribution\" : \"Foo News\","
       "    \"imageUrl\" : \"http://localhost/foobar.jpg\","
       "    \"ampUrl\" : \"http://localhost/amp\","
       "    \"faviconUrl\" : \"http://localhost/favicon.ico\" "
       "  }]"
       "}]}";
   SetFakeResponse(/*response_data=*/kJsonStr, net::HTTP_OK,
                   net::URLRequestStatus::SUCCESS);
   EXPECT_CALL(mock_callback(), Run(IsSingleArticle("http://localhost/foobar")));
-  snippets_fetcher().FetchSnippets(test_params());
+  snippets_fetcher().FetchSnippets(test_params(), MakeMockCallback());
   FastForwardUntilNoTasksRemain();
   EXPECT_THAT(snippets_fetcher().last_status(), Eq("OK"));
   EXPECT_THAT(snippets_fetcher().last_json(), Eq(kJsonStr));
   EXPECT_THAT(histogram_tester().GetAllSamples(
                   "NewTabPage.Snippets.FetchHttpResponseOrErrorCode"),
               ElementsAre(base::Bucket(/*min=*/200, /*count=*/1)));
   EXPECT_THAT(histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchTime"),
               ElementsAre(base::Bucket(/*min=*/kTestJsonParsingLatencyMs,
                                        /*count=*/1)));
 }
 
 TEST_F(NTPSnippetsContentSuggestionsFetcherTest, EmptyCategoryIsOK) {
   const std::string kJsonStr =
       "{\"categories\" : [{"
       "  \"id\": 1,"
       "  \"localizedTitle\": \"Articles for You\""
       "}]}";
   SetFakeResponse(/*response_data=*/kJsonStr, net::HTTP_OK,
                   net::URLRequestStatus::SUCCESS);
   EXPECT_CALL(mock_callback(), Run(IsEmptyArticleList()));
-  snippets_fetcher().FetchSnippets(test_params());
+  snippets_fetcher().FetchSnippets(test_params(), MakeMockCallback());
   FastForwardUntilNoTasksRemain();
   EXPECT_THAT(snippets_fetcher().last_status(), Eq("OK"));
   EXPECT_THAT(snippets_fetcher().last_json(), Eq(kJsonStr));
   EXPECT_THAT(histogram_tester().GetAllSamples(
                   "NewTabPage.Snippets.FetchHttpResponseOrErrorCode"),
               ElementsAre(base::Bucket(/*min=*/200, /*count=*/1)));
   EXPECT_THAT(histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchTime"),
               ElementsAre(base::Bucket(/*min=*/kTestJsonParsingLatencyMs,
                                        /*count=*/1)));
 }
@@ skipping 29 matching lines @@
       "    \"imageUrl\" : \"http://localhost/foo2.jpg\","
       "    \"ampUrl\" : \"http://localhost/amp\","
       "    \"faviconUrl\" : \"http://localhost/favicon.ico\" "
       "  }]"
       "}]}";
   SetFakeResponse(/*response_data=*/kJsonStr, net::HTTP_OK,
                   net::URLRequestStatus::SUCCESS);
   NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories;
   EXPECT_CALL(mock_callback(), Run(_))
       .WillOnce(WithArg<0>(MovePointeeTo(&fetched_categories)));
-  snippets_fetcher().FetchSnippets(test_params());
+  snippets_fetcher().FetchSnippets(test_params(), MakeMockCallback());
   FastForwardUntilNoTasksRemain();
 
   ASSERT_TRUE(fetched_categories);
   ASSERT_THAT(fetched_categories->size(), Eq(2u));
   for (const auto& category : *fetched_categories) {
     const auto& articles = category.snippets;
     switch (category.category.id()) {
       case static_cast<int>(KnownCategories::ARTICLES):
         ASSERT_THAT(articles.size(), Eq(1u));
         EXPECT_THAT(articles[0]->best_source().url.spec(),
@@ skipping 12 matching lines @@
   EXPECT_THAT(snippets_fetcher().last_status(), Eq("OK"));
   EXPECT_THAT(snippets_fetcher().last_json(), Eq(kJsonStr));
   EXPECT_THAT(histogram_tester().GetAllSamples(
                   "NewTabPage.Snippets.FetchHttpResponseOrErrorCode"),
               ElementsAre(base::Bucket(/*min=*/200, /*count=*/1)));
   EXPECT_THAT(histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchTime"),
               ElementsAre(base::Bucket(/*min=*/kTestJsonParsingLatencyMs,
                                        /*count=*/1)));
 }
 
+TEST_F(NTPSnippetsContentSuggestionsFetcherTest, ExclusiveCategoryOnly) {
+  const std::string kJsonStr =
+      "{\"categories\" : [{"
+      "  \"id\": 1,"
+      "  \"localizedTitle\": \"Articles for You\","
+      "  \"suggestions\" : [{"
+      "    \"ids\" : [\"http://localhost/foobar\"],"
+      "    \"title\" : \"Foo Barred from Baz\","
+      "    \"snippet\" : \"...\","
+      "    \"fullPageUrl\" : \"http://localhost/foobar\","
+      "    \"creationTime\" : \"2016-06-30T11:01:37.000Z\","
+      "    \"expirationTime\" : \"2016-07-01T11:01:37.000Z\","
+      "    \"attribution\" : \"Foo News\","
+      "    \"imageUrl\" : \"http://localhost/foobar.jpg\","
+      "    \"ampUrl\" : \"http://localhost/amp\","
+      "    \"faviconUrl\" : \"http://localhost/favicon.ico\" "
+      "  }]"
+      "}, {"
+      "  \"id\": 2,"
+      "  \"localizedTitle\": \"Articles for Me\","
+      "  \"suggestions\" : [{"
+      "    \"ids\" : [\"http://localhost/foo2\"],"
+      "    \"title\" : \"Foo Barred from Baz\","
+      "    \"snippet\" : \"...\","
+      "    \"fullPageUrl\" : \"http://localhost/foo2\","
+      "    \"creationTime\" : \"2016-06-30T11:01:37.000Z\","
+      "    \"expirationTime\" : \"2016-07-01T11:01:37.000Z\","
+      "    \"attribution\" : \"Foo News\","
+      "    \"imageUrl\" : \"http://localhost/foo2.jpg\","
+      "    \"ampUrl\" : \"http://localhost/amp\","
+      "    \"faviconUrl\" : \"http://localhost/favicon.ico\" "
+      "  }]"
+      "}]}";
+  SetFakeResponse(/*response_data=*/kJsonStr, net::HTTP_OK,
+                  net::URLRequestStatus::SUCCESS);
+  NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories;
+  EXPECT_CALL(mock_callback(), Run(_))
+      .WillOnce(WithArg<0>(MovePointeeTo(&fetched_categories)));

[Bernhard Bauer, 2016/10/28 16:26:00]

I realize this is just copied from existing code but... this seems like it's
past the point where using GMock actually provides value. The only advantage the
mock seems to provide is that you can verify it's been called, which is a
somewhat contentious pattern anyway (see eg.
http://martinfowler.com/articles/mocksArentStubs.html), and you can verify the
final _state_ by inspecting |fetched_categories| anyway.

[fhorschig, 2016/11/02 05:06:12]

On one hand, it is true that a stub would suffice in most cases. 
On the other hand, the behavior has to be tested at least once: even if there
was no new article fetched, the callback has to return. In this special case
(see test "EmptyCategoryIsOK"), we need a mock.

If it is considered better style, I can define a stub for every case that
doesn't need the explicit verification of the behavior. I would prefer to do
this change in a separate CL as it affects tests that are not explicitly tied to
these changes.

+  NTPSnippetsFetcher::Params params = test_params();
+  params.exclusive_category = OptionalArticlesCategory();
+  snippets_fetcher().FetchSnippets(params, MakeMockCallback());
+  FastForwardUntilNoTasksRemain();
+
+  ASSERT_TRUE(fetched_categories);
+  ASSERT_THAT(fetched_categories->size(), Eq(1u));
+  const auto& fetched_category = (*fetched_categories)[0];
+  EXPECT_TRUE(
+      fetched_category.category.IsKnownCategory(KnownCategories::ARTICLES));
+  const auto& articles = fetched_category.snippets;
+  ASSERT_THAT(articles.size(), Eq(1u));
+  EXPECT_THAT(articles[0]->best_source().url.spec(),
+              Eq("http://localhost/foobar"));
+}
+
 TEST_F(NTPSnippetsFetcherTest, ShouldFetchSuccessfullyEmptyList) {
   const std::string kJsonStr = "{\"recos\": []}";
   SetFakeResponse(/*response_data=*/kJsonStr, net::HTTP_OK,
                   net::URLRequestStatus::SUCCESS);
   EXPECT_CALL(mock_callback(), Run(IsEmptyArticleList()));
-  snippets_fetcher().FetchSnippets(test_params());
+  snippets_fetcher().FetchSnippets(test_params(), MakeMockCallback());
   FastForwardUntilNoTasksRemain();
   EXPECT_THAT(snippets_fetcher().last_status(), Eq("OK"));
   EXPECT_THAT(snippets_fetcher().last_json(), Eq(kJsonStr));
   EXPECT_THAT(
       histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchResult"),
       ElementsAre(base::Bucket(/*min=*/0, /*count=*/1)));
   EXPECT_THAT(histogram_tester().GetAllSamples(
                   "NewTabPage.Snippets.FetchHttpResponseOrErrorCode"),
               ElementsAre(base::Bucket(/*min=*/200, /*count=*/1)));
 }
 
 TEST_F(NTPSnippetsFetcherTest, ShouldRestrictToHosts) {
   net::TestURLFetcherFactory test_url_fetcher_factory;
   NTPSnippetsFetcher::Params params = test_params();
   params.hosts = {"www.somehost1.com", "www.somehost2.com"};
   params.count_to_fetch = 17;
-  snippets_fetcher().FetchSnippets(params);
+  snippets_fetcher().FetchSnippets(params, MakeMockCallback());
   net::TestURLFetcher* fetcher = test_url_fetcher_factory.GetFetcherByID(0);
   ASSERT_THAT(fetcher, NotNull());
   std::unique_ptr<base::Value> value =
       base::JSONReader::Read(fetcher->upload_data());
   ASSERT_TRUE(value) << " failed to parse JSON: "
                      << PrintToString(fetcher->upload_data());
   const base::DictionaryValue* dict = nullptr;
   ASSERT_TRUE(value->GetAsDictionary(&dict));
   const base::DictionaryValue* local_scoring_params = nullptr;
   ASSERT_TRUE(dict->GetDictionary("advanced_options.local_scoring_params",
                                   &local_scoring_params));
   const base::ListValue* content_selectors = nullptr;
   ASSERT_TRUE(
       local_scoring_params->GetList("content_selectors", &content_selectors));
   ASSERT_THAT(content_selectors->GetSize(), Eq(static_cast<size_t>(2)));
   const base::DictionaryValue* content_selector = nullptr;
   ASSERT_TRUE(content_selectors->GetDictionary(0, &content_selector));
   std::string content_selector_value;
   EXPECT_TRUE(content_selector->GetString("value", &content_selector_value));
   EXPECT_THAT(content_selector_value, Eq("www.somehost1.com"));
   ASSERT_TRUE(content_selectors->GetDictionary(1, &content_selector));
   EXPECT_TRUE(content_selector->GetString("value", &content_selector_value));
   EXPECT_THAT(content_selector_value, Eq("www.somehost2.com"));
 }
 
 TEST_F(NTPSnippetsFetcherTest, ShouldReportUrlStatusError) {
   SetFakeResponse(/*response_data=*/std::string(), net::HTTP_NOT_FOUND,
                   net::URLRequestStatus::FAILED);
   EXPECT_CALL(mock_callback(), Run(/*snippets=*/Not(HasValue()))).Times(1);
-  snippets_fetcher().FetchSnippets(test_params());
+  snippets_fetcher().FetchSnippets(test_params(), MakeMockCallback());
   FastForwardUntilNoTasksRemain();
   EXPECT_THAT(snippets_fetcher().last_status(),
               Eq("URLRequestStatus error -2"));
   EXPECT_THAT(snippets_fetcher().last_json(), IsEmpty());
   EXPECT_THAT(
       histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchResult"),
       ElementsAre(base::Bucket(/*min=*/2, /*count=*/1)));
   EXPECT_THAT(histogram_tester().GetAllSamples(
                   "NewTabPage.Snippets.FetchHttpResponseOrErrorCode"),
               ElementsAre(base::Bucket(/*min=*/-2, /*count=*/1)));
   EXPECT_THAT(histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchTime"),
               Not(IsEmpty()));
 }
 
 TEST_F(NTPSnippetsFetcherTest, ShouldReportHttpError) {
   SetFakeResponse(/*response_data=*/std::string(), net::HTTP_NOT_FOUND,
                   net::URLRequestStatus::SUCCESS);
   EXPECT_CALL(mock_callback(), Run(/*snippets=*/Not(HasValue()))).Times(1);
-  snippets_fetcher().FetchSnippets(test_params());
+  snippets_fetcher().FetchSnippets(test_params(), MakeMockCallback());
   FastForwardUntilNoTasksRemain();
   EXPECT_THAT(snippets_fetcher().last_json(), IsEmpty());
   EXPECT_THAT(
       histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchResult"),
       ElementsAre(base::Bucket(/*min=*/3, /*count=*/1)));
   EXPECT_THAT(histogram_tester().GetAllSamples(
                   "NewTabPage.Snippets.FetchHttpResponseOrErrorCode"),
               ElementsAre(base::Bucket(/*min=*/404, /*count=*/1)));
   EXPECT_THAT(histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchTime"),
               Not(IsEmpty()));
 }
 
 TEST_F(NTPSnippetsFetcherTest, ShouldReportJsonError) {
   const std::string kInvalidJsonStr = "{ \"recos\": []";
   SetFakeResponse(/*response_data=*/kInvalidJsonStr, net::HTTP_OK,
                   net::URLRequestStatus::SUCCESS);
   EXPECT_CALL(mock_callback(), Run(/*snippets=*/Not(HasValue()))).Times(1);
-  snippets_fetcher().FetchSnippets(test_params());
+  snippets_fetcher().FetchSnippets(test_params(), MakeMockCallback());
   FastForwardUntilNoTasksRemain();
   EXPECT_THAT(snippets_fetcher().last_status(),
               StartsWith("Received invalid JSON (error "));
   EXPECT_THAT(snippets_fetcher().last_json(), Eq(kInvalidJsonStr));
   EXPECT_THAT(
       histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchResult"),
       ElementsAre(base::Bucket(/*min=*/4, /*count=*/1)));
   EXPECT_THAT(histogram_tester().GetAllSamples(
                   "NewTabPage.Snippets.FetchHttpResponseOrErrorCode"),
               ElementsAre(base::Bucket(/*min=*/200, /*count=*/1)));
   EXPECT_THAT(histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchTime"),
               ElementsAre(base::Bucket(/*min=*/kTestJsonParsingLatencyMs,
                                        /*count=*/1)));
 }
 
 TEST_F(NTPSnippetsFetcherTest, ShouldReportJsonErrorForEmptyResponse) {
   SetFakeResponse(/*response_data=*/std::string(), net::HTTP_OK,
                   net::URLRequestStatus::SUCCESS);
   EXPECT_CALL(mock_callback(), Run(/*snippets=*/Not(HasValue()))).Times(1);
-  snippets_fetcher().FetchSnippets(test_params());
+  snippets_fetcher().FetchSnippets(test_params(), MakeMockCallback());
   FastForwardUntilNoTasksRemain();
   EXPECT_THAT(snippets_fetcher().last_json(), std::string());
   EXPECT_THAT(
       histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchResult"),
       ElementsAre(base::Bucket(/*min=*/4, /*count=*/1)));
   EXPECT_THAT(histogram_tester().GetAllSamples(
                   "NewTabPage.Snippets.FetchHttpResponseOrErrorCode"),
               ElementsAre(base::Bucket(/*min=*/200, /*count=*/1)));
 }
 
 TEST_F(NTPSnippetsFetcherTest, ShouldReportInvalidListError) {
   const std::string kJsonStr =
       "{\"recos\": [{ \"contentInfo\": { \"foo\" : \"bar\" }}]}";
   SetFakeResponse(/*response_data=*/kJsonStr, net::HTTP_OK,
                   net::URLRequestStatus::SUCCESS);
   EXPECT_CALL(mock_callback(), Run(/*snippets=*/Not(HasValue()))).Times(1);
-  snippets_fetcher().FetchSnippets(test_params());
+  snippets_fetcher().FetchSnippets(test_params(), MakeMockCallback());
   FastForwardUntilNoTasksRemain();
   EXPECT_THAT(snippets_fetcher().last_json(), Eq(kJsonStr));
   EXPECT_THAT(
       histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchResult"),
       ElementsAre(base::Bucket(/*min=*/5, /*count=*/1)));
   EXPECT_THAT(histogram_tester().GetAllSamples(
                   "NewTabPage.Snippets.FetchHttpResponseOrErrorCode"),
               ElementsAre(base::Bucket(/*min=*/200, /*count=*/1)));
   EXPECT_THAT(histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchTime"),
               Not(IsEmpty()));
 }
 
 // This test actually verifies that the test setup itself is sane, to prevent
 // hard-to-reproduce test failures.
 TEST_F(NTPSnippetsFetcherTest, ShouldReportHttpErrorForMissingBakedResponse) {
   InitFakeURLFetcherFactory();
   EXPECT_CALL(mock_callback(), Run(/*snippets=*/Not(HasValue()))).Times(1);
-  snippets_fetcher().FetchSnippets(test_params());
+  snippets_fetcher().FetchSnippets(test_params(), MakeMockCallback());
   FastForwardUntilNoTasksRemain();
 }
 
 TEST_F(NTPSnippetsFetcherTest, ShouldCancelOngoingFetch) {
   const std::string kJsonStr = "{ \"recos\": [] }";
   SetFakeResponse(/*response_data=*/kJsonStr, net::HTTP_OK,
                   net::URLRequestStatus::SUCCESS);
   EXPECT_CALL(mock_callback(), Run(IsEmptyArticleList()));
-  snippets_fetcher().FetchSnippets(test_params());
+  snippets_fetcher().FetchSnippets(test_params(), MakeMockCallback());
   // Second call to FetchSnippets() overrides/cancels the previous.
   // Callback is expected to be called once.
-  snippets_fetcher().FetchSnippets(test_params());
+  snippets_fetcher().FetchSnippets(test_params(), MakeMockCallback());
   FastForwardUntilNoTasksRemain();
   EXPECT_THAT(
       histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchResult"),
       ElementsAre(base::Bucket(/*min=*/0, /*count=*/1)));
   EXPECT_THAT(histogram_tester().GetAllSamples(
                   "NewTabPage.Snippets.FetchHttpResponseOrErrorCode"),
               ElementsAre(base::Bucket(/*min=*/200, /*count=*/1)));
   EXPECT_THAT(histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchTime"),
               ElementsAre(base::Bucket(/*min=*/kTestJsonParsingLatencyMs,
                                        /*count=*/1)));
 }
 
 ::std::ostream& operator<<(
     ::std::ostream& os,
     const NTPSnippetsFetcher::OptionalFetchedCategories& fetched_categories) {
   if (fetched_categories) {
     // Matchers above aren't any more precise than this, so this is sufficient
     // for test-failure diagnostics.
     return os << "list with " << fetched_categories->size() << " elements";
   }
   return os << "null";
 }
 
 }  // namespace ntp_snippets