[Remote suggestions] Clean up variation params in the fetcher.

components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc

 // Copyright 2015 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/remote_suggestions_provider_impl.h"
 
 #include <memory>
 #include <utility>
 #include <vector>
 
@@ skipping 14 matching lines @@
 #include "base/time/time.h"
 #include "components/image_fetcher/image_decoder.h"
 #include "components/image_fetcher/image_fetcher.h"
 #include "components/image_fetcher/image_fetcher_delegate.h"
 #include "components/ntp_snippets/category.h"
 #include "components/ntp_snippets/category_info.h"
 #include "components/ntp_snippets/category_rankers/category_ranker.h"
 #include "components/ntp_snippets/category_rankers/constant_category_ranker.h"
 #include "components/ntp_snippets/category_rankers/mock_category_ranker.h"
 #include "components/ntp_snippets/fake_content_suggestions_provider_observer.h"
+#include "components/ntp_snippets/features.h"
 #include "components/ntp_snippets/ntp_snippets_constants.h"
 #include "components/ntp_snippets/pref_names.h"
 #include "components/ntp_snippets/remote/persistent_scheduler.h"
 #include "components/ntp_snippets/remote/remote_suggestion.h"
 #include "components/ntp_snippets/remote/remote_suggestions_database.h"
 #include "components/ntp_snippets/remote/remote_suggestions_fetcher.h"
 #include "components/ntp_snippets/remote/test_utils.h"
 #include "components/ntp_snippets/user_classifier.h"
 #include "components/prefs/testing_pref_service.h"
 #include "components/signin/core/browser/fake_profile_oauth2_token_service.h"
@@ skipping 39 matching lines @@
 MATCHER_P(IsCategory, id, "") {
   return arg.id() == static_cast<int>(id);
 }
 
 MATCHER_P(HasCode, code, "") {
   return arg.code == code;
 }
 
 const base::Time::Exploded kDefaultCreationTime = {2015, 11, 4, 25, 13, 46, 45};
 const char kTestContentSuggestionsServerEndpoint[] =
-    "https://localunittest-chromecontentsuggestions-pa.googleapis.com/v1/"
+    "https://alpha-chromecontentsuggestions-pa.sandbox.googleapis.com/v1/"
     "suggestions/fetch";
 const char kAPIKey[] = "fakeAPIkey";
 const char kTestContentSuggestionsServerWithAPIKey[] =
-    "https://localunittest-chromecontentsuggestions-pa.googleapis.com/v1/"
+    "https://alpha-chromecontentsuggestions-pa.sandbox.googleapis.com/v1/"
     "suggestions/fetch?key=fakeAPIkey";
 
 const char kSuggestionUrl[] = "http://localhost/foobar";
 const char kSuggestionTitle[] = "Title";
 const char kSuggestionText[] = "Suggestion";
 const char kSuggestionSalientImage[] = "http://localhost/salient_image";
 const char kSuggestionPublisherName[] = "Foo News";
 const char kSuggestionAmpUrl[] = "http://localhost/amp";
 
 const char kSuggestionUrl2[] = "http://foo.com/bar";
@@ skipping 252 matching lines @@
   gfx::Image decoded_image_;
 };
 
 }  // namespace
 
 class RemoteSuggestionsProviderImplTest : public ::testing::Test {
  public:
   RemoteSuggestionsProviderImplTest()
       : params_manager_(ntp_snippets::kStudyName,
                         {{"content_suggestions_backend",
-                          kTestContentSuggestionsServerEndpoint},
-                         {"fetching_personalization", "non_personal"}}),
+                          kTestContentSuggestionsServerEndpoint}},
+                        {ntp_snippets::kArticleSuggestionsFeature.name}),
         fake_url_fetcher_factory_(
             /*default_factory=*/&failing_url_fetcher_factory_),
         test_url_(kTestContentSuggestionsServerWithAPIKey),
         category_ranker_(base::MakeUnique<ConstantCategoryRanker>()),
         user_classifier_(/*pref_service=*/nullptr),
         suggestions_fetcher_(nullptr),
         image_fetcher_(nullptr),
         image_decoder_(nullptr),
         database_(nullptr) {
     RemoteSuggestionsProviderImpl::RegisterProfilePrefs(
@@ skipping 21 matching lines @@
 
   std::unique_ptr<RemoteSuggestionsProviderImpl>
   MakeSuggestionsProviderWithoutInitialization() {
     scoped_refptr<base::SingleThreadTaskRunner> task_runner(
         base::ThreadTaskRunnerHandle::Get());
     scoped_refptr<net::TestURLRequestContextGetter> request_context_getter =
         new net::TestURLRequestContextGetter(task_runner.get());
 
     utils_.ResetSigninManager();
     auto suggestions_fetcher = base::MakeUnique<RemoteSuggestionsFetcher>(
-        utils_.fake_signin_manager(), fake_token_service_.get(),
+        utils_.fake_signin_manager(), /*token_service=*/nullptr,

[Marc Treib, 2017/02/03 10:52:08]

Does this work without any "may be null in tests" in the fetcher?!
I guess due to the non-signed-in state, the fetcher never actually does anything
with the service?

I suppose this is okay for now (this CL doesn't make the situation any worse),
but we really should add tests for this. Did we have a bug already? Either way,
we should at least add a TODO, also to document the current state.

[jkrcal, 2017/02/03 11:04:16]

I think the tests belong to the fetcher_unittest. Added a TODO there.

[Marc Treib, 2017/02/03 11:10:46]

That makes sense! We have a separate TODO on the Fetcher class to make it an
interface, so that we could inject a fake fetcher here.

         std::move(request_context_getter), utils_.pref_service(), nullptr,
         base::Bind(&ParseJson), kAPIKey, &user_classifier_);
     suggestions_fetcher_ = suggestions_fetcher.get();
 
-    utils_.fake_signin_manager()->SignIn("foo@bar.com");
-
     auto image_fetcher = base::MakeUnique<NiceMock<MockImageFetcher>>();
 
     image_fetcher_ = image_fetcher.get();
     EXPECT_CALL(*image_fetcher, SetImageFetcherDelegate(_));
     auto image_decoder = base::MakeUnique<FakeImageDecoder>();
     image_decoder_ = image_decoder.get();
     EXPECT_FALSE(observer_);
     observer_ = base::MakeUnique<FakeContentSuggestionsProviderObserver>();
     auto database = base::MakeUnique<RemoteSuggestionsDatabase>(
         database_dir_.GetPath(), task_runner);
@@ skipping 101 matching lines @@
   }
 
  private:
   variations::testing::VariationParamsManager params_manager_;
   test::RemoteSuggestionsTestUtils utils_;
   base::MessageLoop message_loop_;
   FailingFakeURLFetcherFactory failing_url_fetcher_factory_;
   // Instantiation of factory automatically sets itself as URLFetcher's factory.
   net::FakeURLFetcherFactory fake_url_fetcher_factory_;
   const GURL test_url_;
-  std::unique_ptr<OAuth2TokenService> fake_token_service_;
   std::unique_ptr<CategoryRanker> category_ranker_;
   UserClassifier user_classifier_;
   std::unique_ptr<FakeContentSuggestionsProviderObserver> observer_;
   RemoteSuggestionsFetcher* suggestions_fetcher_;
   NiceMock<MockImageFetcher>* image_fetcher_;
   FakeImageDecoder* image_decoder_;
 
   base::ScopedTempDir database_dir_;
   RemoteSuggestionsDatabase* database_;
 
@@ skipping 1263 matching lines @@
                          RemoteSuggestionsProvider::ProviderStatus)>::Call,
                      base::Unretained(&status_callback))));
 
   // Should be called when becoming disabled.
   EXPECT_CALL(status_callback,
               Call(RemoteSuggestionsProvider::ProviderStatus::INACTIVE));
   service->EnterState(RemoteSuggestionsProviderImpl::State::DISABLED);
 }
 
 }  // namespace ntp_snippets