diff --git a/components/brave_news/browser/DEPS b/components/brave_news/browser/DEPS index 4a8e01279545..f664f8bddb9c 100644 --- a/components/brave_news/browser/DEPS +++ b/components/brave_news/browser/DEPS @@ -7,35 +7,3 @@ include_rules = [ "+services/network/test", "+third_party/re2", ] - -specific_include_rules = { - # unit tests should not depend on Profile - # also components should not depend on Profile - "brave_news_pref_manager_unittest\.cc": [ - "!chrome/test/base/testing_profile.h", - ], - "channels_controller_unittest\.cc": [ - "!chrome/test/base/testing_profile.h", - ], - "direct_feed_controller_unittest\.cc": [ - "!chrome/test/base/testing_profile.h", - ], - "direct_feed_controller_unittest\.cc": [ - "!chrome/test/base/testing_profile.h", - ], - "initialization_promise_unittest\.cc": [ - "!chrome/test/base/testing_profile.h", - ], - "publishers_controller_unittest\.cc": [ - "!chrome/test/base/testing_profile.h", - ], - "suggestions_controller_unittest\.cc": [ - "!chrome/test/base/testing_profile.h", - ], - "topics_fetcher_unittest\.cc": [ - "!chrome/test/base/testing_profile.h", - ], - "feed_building_unittest\.cc": [ - "!chrome/test/base/testing_profile.h", - ], -} diff --git a/components/brave_news/browser/brave_news_pref_manager_unittest.cc b/components/brave_news/browser/brave_news_pref_manager_unittest.cc index e810f13cef86..0446575e7e09 100644 --- a/components/brave_news/browser/brave_news_pref_manager_unittest.cc +++ b/components/brave_news/browser/brave_news_pref_manager_unittest.cc @@ -5,45 +5,50 @@ #include "brave/components/brave_news/browser/brave_news_pref_manager.h" +#include + #include "base/containers/contains.h" #include "base/containers/flat_set.h" #include "brave/components/brave_news/common/brave_news.mojom.h" +#include "brave/components/brave_news/common/pref_names.h" #include "brave/components/brave_news/common/subscriptions_snapshot.h" -#include "chrome/test/base/testing_profile.h" -#include "content/public/test/browser_task_environment.h" +#include "components/sync_preferences/testing_pref_service_syncable.h" #include "testing/gtest/include/gtest/gtest.h" namespace brave_news { class BraveNewsPrefManagerTest : public testing::Test { public: - BraveNewsPrefManagerTest() : pref_manager_(*profile_.GetPrefs()) {} + BraveNewsPrefManagerTest() { + prefs::RegisterProfilePrefs(pref_service_.registry()); + + pref_manager_ = std::make_unique(pref_service_); + } protected: - content::BrowserTaskEnvironment browser_task_environment_; + sync_preferences::TestingPrefServiceSyncable pref_service_; - TestingProfile profile_; - BraveNewsPrefManager pref_manager_; + std::unique_ptr pref_manager_; }; TEST_F(BraveNewsPrefManagerTest, CanAddDirectFeed) { - EXPECT_NE("", pref_manager_.AddDirectPublisher(GURL("https://example.com"), - "Example")); + EXPECT_NE("", pref_manager_->AddDirectPublisher(GURL("https://example.com"), + "Example")); } TEST_F(BraveNewsPrefManagerTest, CantAddDuplicateFeed) { - auto id = pref_manager_.AddDirectPublisher(GURL("https://example.com"), - "Example 1"); + auto id = pref_manager_->AddDirectPublisher(GURL("https://example.com"), + "Example 1"); // As the feed is already present, this should be the same publisher. - EXPECT_EQ(id, pref_manager_.AddDirectPublisher(GURL("https://example.com"), - "Example 2")); + EXPECT_EQ(id, pref_manager_->AddDirectPublisher(GURL("https://example.com"), + "Example 2")); } TEST_F(BraveNewsPrefManagerTest, EmptyTitleFallsBackToFeedSource) { constexpr char kFeedSource[] = "https://example.com/"; - auto id = pref_manager_.AddDirectPublisher(GURL(kFeedSource), ""); - auto subscriptions = pref_manager_.GetSubscriptions().direct_feeds(); + auto id = pref_manager_->AddDirectPublisher(GURL(kFeedSource), ""); + auto subscriptions = pref_manager_->GetSubscriptions().direct_feeds(); ASSERT_EQ(1u, subscriptions.size()); for (const auto& subscription : subscriptions) { @@ -54,25 +59,25 @@ TEST_F(BraveNewsPrefManagerTest, EmptyTitleFallsBackToFeedSource) { } TEST_F(BraveNewsPrefManagerTest, ChannelsDiffIsSane) { - auto one = pref_manager_.GetSubscriptions(); - pref_manager_.SetChannelSubscribed("en_US", "News", true); + auto one = pref_manager_->GetSubscriptions(); + pref_manager_->SetChannelSubscribed("en_US", "News", true); - auto two = pref_manager_.GetSubscriptions(); + auto two = pref_manager_->GetSubscriptions(); auto diff = two.DiffChannels(one); ASSERT_EQ(1u, diff.changed.size()); EXPECT_EQ("News", diff.changed[0]); - pref_manager_.SetChannelSubscribed("en_NZ", "News", true); - auto three = pref_manager_.GetSubscriptions(); + pref_manager_->SetChannelSubscribed("en_NZ", "News", true); + auto three = pref_manager_->GetSubscriptions(); ASSERT_EQ(0u, three.DiffChannels(two).changed.size()); - pref_manager_.SetChannelSubscribed("en_NZ", "The Spinoff", true); - pref_manager_.SetChannelSubscribed("en_NZ", "Politics", true); - auto four = pref_manager_.GetSubscriptions(); + pref_manager_->SetChannelSubscribed("en_NZ", "The Spinoff", true); + pref_manager_->SetChannelSubscribed("en_NZ", "Politics", true); + auto four = pref_manager_->GetSubscriptions(); - pref_manager_.SetChannelSubscribed("en_NZ", "FooBar", true); - pref_manager_.SetChannelSubscribed("en_NZ", "Politics", false); - auto five = pref_manager_.GetSubscriptions(); + pref_manager_->SetChannelSubscribed("en_NZ", "FooBar", true); + pref_manager_->SetChannelSubscribed("en_NZ", "Politics", false); + auto five = pref_manager_->GetSubscriptions(); diff = five.DiffChannels(four); EXPECT_EQ(2u, diff.changed.size()); @@ -81,28 +86,29 @@ TEST_F(BraveNewsPrefManagerTest, ChannelsDiffIsSane) { } TEST_F(BraveNewsPrefManagerTest, PublishersDiffIsSane) { - pref_manager_.SetPublisherSubscribed("One", mojom::UserEnabled::ENABLED); - pref_manager_.SetPublisherSubscribed("Two", mojom::UserEnabled::ENABLED); - pref_manager_.SetPublisherSubscribed("Three", mojom::UserEnabled::ENABLED); - pref_manager_.SetPublisherSubscribed("Four", mojom::UserEnabled::DISABLED); - pref_manager_.SetPublisherSubscribed("Five", mojom::UserEnabled::DISABLED); - pref_manager_.SetPublisherSubscribed("Six", mojom::UserEnabled::DISABLED); + pref_manager_->SetPublisherSubscribed("One", mojom::UserEnabled::ENABLED); + pref_manager_->SetPublisherSubscribed("Two", mojom::UserEnabled::ENABLED); + pref_manager_->SetPublisherSubscribed("Three", mojom::UserEnabled::ENABLED); + pref_manager_->SetPublisherSubscribed("Four", mojom::UserEnabled::DISABLED); + pref_manager_->SetPublisherSubscribed("Five", mojom::UserEnabled::DISABLED); + pref_manager_->SetPublisherSubscribed("Six", mojom::UserEnabled::DISABLED); auto direct_one = - pref_manager_.AddDirectPublisher(GURL("https://example.com"), ""); - pref_manager_.AddDirectPublisher(GURL("https://foobar.com"), ""); - - auto s1 = pref_manager_.GetSubscriptions(); - pref_manager_.SetPublisherSubscribed("One", mojom::UserEnabled::NOT_MODIFIED); - pref_manager_.SetPublisherSubscribed("Two", mojom::UserEnabled::DISABLED); - pref_manager_.SetPublisherSubscribed("Four", - mojom::UserEnabled::NOT_MODIFIED); - pref_manager_.SetPublisherSubscribed("Five", mojom::UserEnabled::ENABLED); - pref_manager_.SetPublisherSubscribed(direct_one, - mojom::UserEnabled::DISABLED); + pref_manager_->AddDirectPublisher(GURL("https://example.com"), ""); + pref_manager_->AddDirectPublisher(GURL("https://foobar.com"), ""); + + auto s1 = pref_manager_->GetSubscriptions(); + pref_manager_->SetPublisherSubscribed("One", + mojom::UserEnabled::NOT_MODIFIED); + pref_manager_->SetPublisherSubscribed("Two", mojom::UserEnabled::DISABLED); + pref_manager_->SetPublisherSubscribed("Four", + mojom::UserEnabled::NOT_MODIFIED); + pref_manager_->SetPublisherSubscribed("Five", mojom::UserEnabled::ENABLED); + pref_manager_->SetPublisherSubscribed(direct_one, + mojom::UserEnabled::DISABLED); auto direct_three = - pref_manager_.AddDirectPublisher(GURL("https://foo.nz"), ""); + pref_manager_->AddDirectPublisher(GURL("https://foo.nz"), ""); - auto s2 = pref_manager_.GetSubscriptions(); + auto s2 = pref_manager_->GetSubscriptions(); auto diff = s2.DiffPublishers(s1); base::flat_set distinct_changes(diff.changed); @@ -119,109 +125,109 @@ TEST_F(BraveNewsPrefManagerTest, PublishersDiffIsSane) { TEST_F(BraveNewsPrefManagerTest, DirectFeedCanBeInspectedAndRemoved) { auto id = - pref_manager_.AddDirectPublisher(GURL("https://example.com"), "Example"); + pref_manager_->AddDirectPublisher(GURL("https://example.com"), "Example"); - auto parsed = pref_manager_.GetSubscriptions().direct_feeds(); + auto parsed = pref_manager_->GetSubscriptions().direct_feeds(); EXPECT_EQ(parsed.size(), 1u); EXPECT_EQ(id, parsed[0].id); EXPECT_EQ("Example", parsed[0].title); EXPECT_EQ(GURL("https://example.com"), parsed[0].url); - pref_manager_.SetPublisherSubscribed(id, mojom::UserEnabled::DISABLED); - parsed = pref_manager_.GetSubscriptions().direct_feeds(); + pref_manager_->SetPublisherSubscribed(id, mojom::UserEnabled::DISABLED); + parsed = pref_manager_->GetSubscriptions().direct_feeds(); EXPECT_EQ(0u, parsed.size()); } TEST_F(BraveNewsPrefManagerTest, CanToggleChannelSubscribed) { EXPECT_FALSE( - pref_manager_.GetSubscriptions().GetChannelSubscribed("en_US", "Test")); + pref_manager_->GetSubscriptions().GetChannelSubscribed("en_US", "Test")); - pref_manager_.SetChannelSubscribed("en_US", "Test", true); + pref_manager_->SetChannelSubscribed("en_US", "Test", true); EXPECT_TRUE( - pref_manager_.GetSubscriptions().GetChannelSubscribed("en_US", "Test")); + pref_manager_->GetSubscriptions().GetChannelSubscribed("en_US", "Test")); - pref_manager_.SetChannelSubscribed("en_US", "Test", false); + pref_manager_->SetChannelSubscribed("en_US", "Test", false); EXPECT_FALSE( - pref_manager_.GetSubscriptions().GetChannelSubscribed("en_US", "Test")); + pref_manager_->GetSubscriptions().GetChannelSubscribed("en_US", "Test")); } TEST_F(BraveNewsPrefManagerTest, ChangingAChannelInOneLocaleDoesNotAffectOtherLocales) { EXPECT_FALSE( - pref_manager_.GetSubscriptions().GetChannelSubscribed("en_US", "Test")); + pref_manager_->GetSubscriptions().GetChannelSubscribed("en_US", "Test")); EXPECT_FALSE( - pref_manager_.GetSubscriptions().GetChannelSubscribed("ja_JA", "Test")); + pref_manager_->GetSubscriptions().GetChannelSubscribed("ja_JA", "Test")); - pref_manager_.SetChannelSubscribed("en_US", "Test", true); + pref_manager_->SetChannelSubscribed("en_US", "Test", true); EXPECT_TRUE( - pref_manager_.GetSubscriptions().GetChannelSubscribed("en_US", "Test")); + pref_manager_->GetSubscriptions().GetChannelSubscribed("en_US", "Test")); EXPECT_FALSE( - pref_manager_.GetSubscriptions().GetChannelSubscribed("ja_JA", "Test")); + pref_manager_->GetSubscriptions().GetChannelSubscribed("ja_JA", "Test")); - pref_manager_.SetChannelSubscribed("ja_JA", "Test", true); + pref_manager_->SetChannelSubscribed("ja_JA", "Test", true); EXPECT_TRUE( - pref_manager_.GetSubscriptions().GetChannelSubscribed("en_US", "Test")); + pref_manager_->GetSubscriptions().GetChannelSubscribed("en_US", "Test")); EXPECT_TRUE( - pref_manager_.GetSubscriptions().GetChannelSubscribed("ja_JA", "Test")); + pref_manager_->GetSubscriptions().GetChannelSubscribed("ja_JA", "Test")); - pref_manager_.SetChannelSubscribed("en_US", "Test", false); + pref_manager_->SetChannelSubscribed("en_US", "Test", false); EXPECT_FALSE( - pref_manager_.GetSubscriptions().GetChannelSubscribed("en_US", "Test")); + pref_manager_->GetSubscriptions().GetChannelSubscribed("en_US", "Test")); EXPECT_TRUE( - pref_manager_.GetSubscriptions().GetChannelSubscribed("ja_JA", "Test")); + pref_manager_->GetSubscriptions().GetChannelSubscribed("ja_JA", "Test")); - pref_manager_.SetChannelSubscribed("ja_JA", "Test", false); + pref_manager_->SetChannelSubscribed("ja_JA", "Test", false); EXPECT_FALSE( - pref_manager_.GetSubscriptions().GetChannelSubscribed("en_US", "Test")); + pref_manager_->GetSubscriptions().GetChannelSubscribed("en_US", "Test")); EXPECT_FALSE( - pref_manager_.GetSubscriptions().GetChannelSubscribed("ja_JA", "Test")); + pref_manager_->GetSubscriptions().GetChannelSubscribed("ja_JA", "Test")); } TEST_F(BraveNewsPrefManagerTest, NoChannelsNoChannelLocales) { - EXPECT_EQ(0u, pref_manager_.GetSubscriptions().GetChannelLocales().size()); + EXPECT_EQ(0u, pref_manager_->GetSubscriptions().GetChannelLocales().size()); } TEST_F(BraveNewsPrefManagerTest, SubscribedChannelLocalesIncluded) { - pref_manager_.SetChannelSubscribed("en_US", "Test", true); + pref_manager_->SetChannelSubscribed("en_US", "Test", true); - auto locales = pref_manager_.GetSubscriptions().GetChannelLocales(); + auto locales = pref_manager_->GetSubscriptions().GetChannelLocales(); EXPECT_EQ(1u, locales.size()); EXPECT_EQ("en_US", locales[0]); - pref_manager_.SetChannelSubscribed("en_US", "Foo", true); - locales = pref_manager_.GetSubscriptions().GetChannelLocales(); + pref_manager_->SetChannelSubscribed("en_US", "Foo", true); + locales = pref_manager_->GetSubscriptions().GetChannelLocales(); EXPECT_EQ(1u, locales.size()); - pref_manager_.SetChannelSubscribed("ja_JA", "Foo", true); - locales = pref_manager_.GetSubscriptions().GetChannelLocales(); + pref_manager_->SetChannelSubscribed("ja_JA", "Foo", true); + locales = pref_manager_->GetSubscriptions().GetChannelLocales(); EXPECT_EQ(2u, locales.size()); EXPECT_EQ("en_US", locales[0]); EXPECT_EQ("ja_JA", locales[1]); } TEST_F(BraveNewsPrefManagerTest, LocaleWithNoSubscribedChannelsIsNotIncluded) { - pref_manager_.SetChannelSubscribed("en_US", "Test", true); + pref_manager_->SetChannelSubscribed("en_US", "Test", true); - auto locales = pref_manager_.GetSubscriptions().GetChannelLocales(); + auto locales = pref_manager_->GetSubscriptions().GetChannelLocales(); EXPECT_EQ(1u, locales.size()); EXPECT_EQ("en_US", locales[0]); - pref_manager_.SetChannelSubscribed("en_US", "Test", false); - locales = pref_manager_.GetSubscriptions().GetChannelLocales(); + pref_manager_->SetChannelSubscribed("en_US", "Test", false); + locales = pref_manager_->GetSubscriptions().GetChannelLocales(); EXPECT_EQ(0u, locales.size()); } TEST_F(BraveNewsPrefManagerTest, ChannelMigrationsAreApplied) { - pref_manager_.SetChannelSubscribed("en_US", "Tech News", true); - pref_manager_.SetChannelSubscribed("en_US", "Sport", true); + pref_manager_->SetChannelSubscribed("en_US", "Tech News", true); + pref_manager_->SetChannelSubscribed("en_US", "Sport", true); - EXPECT_FALSE(pref_manager_.GetSubscriptions().GetChannelSubscribed( + EXPECT_FALSE(pref_manager_->GetSubscriptions().GetChannelSubscribed( "en_US", "Tech News")); - EXPECT_TRUE(pref_manager_.GetSubscriptions().GetChannelSubscribed( + EXPECT_TRUE(pref_manager_->GetSubscriptions().GetChannelSubscribed( "en_US", "Technology")); EXPECT_FALSE( - pref_manager_.GetSubscriptions().GetChannelSubscribed("en_US", "Sport")); - EXPECT_TRUE( - pref_manager_.GetSubscriptions().GetChannelSubscribed("en_US", "Sports")); + pref_manager_->GetSubscriptions().GetChannelSubscribed("en_US", "Sport")); + EXPECT_TRUE(pref_manager_->GetSubscriptions().GetChannelSubscribed("en_US", + "Sports")); } } // namespace brave_news diff --git a/components/brave_news/browser/channels_controller_unittest.cc b/components/brave_news/browser/channels_controller_unittest.cc index 16a94e142e97..40a4277dbced 100644 --- a/components/brave_news/browser/channels_controller_unittest.cc +++ b/components/brave_news/browser/channels_controller_unittest.cc @@ -5,6 +5,7 @@ #include "brave/components/brave_news/browser/channels_controller.h" +#include #include #include #include @@ -19,8 +20,8 @@ #include "brave/components/brave_news/browser/urls.h" #include "brave/components/brave_news/common/brave_news.mojom.h" #include "brave/components/brave_news/common/pref_names.h" -#include "chrome/test/base/testing_profile.h" #include "components/prefs/scoped_user_pref_update.h" +#include "components/sync_preferences/testing_pref_service_syncable.h" #include "content/public/test/browser_task_environment.h" #include "net/traffic_annotation/network_traffic_annotation_test_helper.h" #include "services/data_decoder/public/cpp/test_support/in_process_data_decoder.h" @@ -76,13 +77,18 @@ class BraveNewsChannelsControllerTest : public testing::Test { public: BraveNewsChannelsControllerTest() : api_request_helper_(TRAFFIC_ANNOTATION_FOR_TESTS, - test_url_loader_factory_.GetSafeWeakWrapper()), - pref_manager_(*profile_.GetPrefs()), - publishers_controller_(&api_request_helper_), - channels_controller_(&publishers_controller_) { - profile_.GetPrefs()->SetBoolean(brave_news::prefs::kBraveNewsOptedIn, true); - profile_.GetPrefs()->SetBoolean(brave_news::prefs::kNewTabPageShowToday, - true); + test_url_loader_factory_.GetSafeWeakWrapper()) { + prefs::RegisterProfilePrefs(pref_service_.registry()); + + pref_manager_ = std::make_unique(pref_service_); + + // Ensure Brave News is enabled. + pref_manager_->SetConfig(mojom::Configuration::New(true, true, true)); + + publishers_controller_ = + std::make_unique(&api_request_helper_); + channels_controller_ = + std::make_unique(publishers_controller_.get()); } std::string GetPublishersURL() { @@ -91,10 +97,10 @@ class BraveNewsChannelsControllerTest : public testing::Test { } brave_news::Channels GetAllChannels() { - auto [channels] = - WaitForCallback(base::BindOnce(&ChannelsController::GetAllChannels, - base::Unretained(&channels_controller_), - pref_manager_.GetSubscriptions())); + auto [channels] = WaitForCallback( + base::BindOnce(&ChannelsController::GetAllChannels, + base::Unretained(channels_controller_.get()), + pref_manager_->GetSubscriptions())); return std::move(channels); } @@ -103,11 +109,11 @@ class BraveNewsChannelsControllerTest : public testing::Test { data_decoder::test::InProcessDataDecoder data_decoder_; network::TestURLLoaderFactory test_url_loader_factory_; api_request_helper::APIRequestHelper api_request_helper_; - TestingProfile profile_; + sync_preferences::TestingPrefServiceSyncable pref_service_; - BraveNewsPrefManager pref_manager_; - PublishersController publishers_controller_; - ChannelsController channels_controller_; + std::unique_ptr pref_manager_; + std::unique_ptr publishers_controller_; + std::unique_ptr channels_controller_; }; TEST_F(BraveNewsChannelsControllerTest, CanGetAllChannels) { @@ -128,8 +134,8 @@ TEST_F(BraveNewsChannelsControllerTest, CanGetAllChannels) { } TEST_F(BraveNewsChannelsControllerTest, GetAllChannelsLoadsSubscribedState) { - pref_manager_.SetChannelSubscribed("en_US", "One", true); - pref_manager_.SetChannelSubscribed("en_US", "Five", true); + pref_manager_->SetChannelSubscribed("en_US", "One", true); + pref_manager_->SetChannelSubscribed("en_US", "Five", true); test_url_loader_factory_.AddResponse(GetPublishersURL(), kPublishersResponse, net::HTTP_OK); @@ -156,8 +162,8 @@ TEST_F(BraveNewsChannelsControllerTest, GetAllChannelsLoadsSubscribedState) { TEST_F(BraveNewsChannelsControllerTest, GetAllChannelsLoadsCorrectLocaleSubscriptionStatus) { - pref_manager_.SetChannelSubscribed("en_US", "One", true); - pref_manager_.SetChannelSubscribed("ja_JA", "Five", true); + pref_manager_->SetChannelSubscribed("en_US", "One", true); + pref_manager_->SetChannelSubscribed("ja_JA", "Five", true); test_url_loader_factory_.AddResponse(GetPublishersURL(), kPublishersResponse, net::HTTP_OK); diff --git a/components/brave_news/browser/direct_feed_controller_unittest.cc b/components/brave_news/browser/direct_feed_controller_unittest.cc index 2e6df6fe5e37..7d6c72b9b949 100644 --- a/components/brave_news/browser/direct_feed_controller_unittest.cc +++ b/components/brave_news/browser/direct_feed_controller_unittest.cc @@ -14,7 +14,6 @@ #include "base/memory/scoped_refptr.h" #include "brave/components/brave_news/browser/brave_news_pref_manager.h" #include "brave/components/brave_news/browser/test/wait_for_callback.h" -#include "chrome/test/base/testing_profile.h" #include "content/public/test/browser_task_environment.h" #include "services/data_decoder/public/cpp/test_support/in_process_data_decoder.h" #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h" @@ -79,8 +78,7 @@ constexpr char kFeedURL[] = "https://example.com/feed"; class BraveNewsDirectFeedControllerTest : public testing::Test { public: BraveNewsDirectFeedControllerTest() - : pref_manager_(*profile_.GetPrefs()), - direct_feed_controller_(test_url_loader_factory_.GetSafeWeakWrapper()) { + : direct_feed_controller_(test_url_loader_factory_.GetSafeWeakWrapper()) { } protected: @@ -97,12 +95,10 @@ class BraveNewsDirectFeedControllerTest : public testing::Test { base::Unretained(&direct_feed_controller_), possible_feed_or_site_url)); return std::move(feeds); } + content::BrowserTaskEnvironment task_environment_; data_decoder::test::InProcessDataDecoder data_decoder_; network::TestURLLoaderFactory test_url_loader_factory_; - - TestingProfile profile_; - BraveNewsPrefManager pref_manager_; DirectFeedController direct_feed_controller_; }; diff --git a/components/brave_news/browser/feed_building_unittest.cc b/components/brave_news/browser/feed_building_unittest.cc index db14ff347675..30e6fab8795c 100644 --- a/components/brave_news/browser/feed_building_unittest.cc +++ b/components/brave_news/browser/feed_building_unittest.cc @@ -22,11 +22,10 @@ #include "brave/components/brave_news/browser/brave_news_pref_manager.h" #include "brave/components/brave_news/browser/channels_controller.h" #include "brave/components/brave_news/browser/combined_feed_parsing.h" -#include "brave/components/brave_news/common/brave_news.mojom-shared.h" #include "brave/components/brave_news/common/brave_news.mojom.h" +#include "brave/components/brave_news/common/pref_names.h" #include "brave/components/brave_news/common/subscriptions_snapshot.h" -#include "chrome/test/base/testing_profile.h" -#include "content/public/test/browser_task_environment.h" +#include "components/sync_preferences/testing_pref_service_syncable.h" #include "testing/gtest/include/gtest/gtest.h" #include "url/mojom/url.mojom.h" @@ -169,7 +168,11 @@ void PopulatePublishers(Publishers* publisher_list) { class BraveNewsFeedBuildingTest : public testing::Test { public: - BraveNewsFeedBuildingTest() : pref_manager_(*profile_.GetPrefs()) {} + BraveNewsFeedBuildingTest() { + prefs::RegisterProfilePrefs(pref_service_.registry()); + + pref_manager_ = std::make_unique(pref_service_); + } BraveNewsFeedBuildingTest(const BraveNewsFeedBuildingTest&) = delete; BraveNewsFeedBuildingTest& operator=(const BraveNewsFeedBuildingTest&) = @@ -177,13 +180,13 @@ class BraveNewsFeedBuildingTest : public testing::Test { ~BraveNewsFeedBuildingTest() override = default; protected: - content::BrowserTaskEnvironment browser_task_environment_; - TestingProfile profile_; - BraveNewsPrefManager pref_manager_; + sync_preferences::TestingPrefServiceSyncable pref_service_; + + std::unique_ptr pref_manager_; }; TEST_F(BraveNewsFeedBuildingTest, BuildFeed) { - pref_manager_.SetChannelSubscribed("en_US", "Top Sources", true); + pref_manager_->SetChannelSubscribed("en_US", "Top Sources", true); Publishers publisher_list; PopulatePublishers(&publisher_list); @@ -195,7 +198,7 @@ TEST_F(BraveNewsFeedBuildingTest, BuildFeed) { mojom::Feed feed; ASSERT_TRUE(BuildFeed(feed_items, history_hosts, &publisher_list, &feed, - pref_manager_.GetSubscriptions())); + pref_manager_->GetSubscriptions())); ASSERT_EQ(feed.pages.size(), 1u); // Validate featured article is top news ASSERT_TRUE(feed.featured_item->is_article()); @@ -365,7 +368,7 @@ TEST_F(BraveNewsFeedBuildingTest, ChannelIsUsed) { } TEST_F(BraveNewsFeedBuildingTest, DuplicateItemsAreNotIncluded) { - pref_manager_.SetChannelSubscribed("en_US", "Top Sources", true); + pref_manager_->SetChannelSubscribed("en_US", "Top Sources", true); Publishers publisher_list; PopulatePublishers(&publisher_list); @@ -379,7 +382,7 @@ TEST_F(BraveNewsFeedBuildingTest, DuplicateItemsAreNotIncluded) { mojom::Feed feed; ASSERT_TRUE(BuildFeed(feed_items, history_hosts, &publisher_list, &feed, - pref_manager_.GetSubscriptions())); + pref_manager_->GetSubscriptions())); ASSERT_EQ(feed.pages.size(), 1u); ASSERT_EQ(feed.pages[0]->items.size(), 18u); } diff --git a/components/brave_news/browser/initialization_promise_unittest.cc b/components/brave_news/browser/initialization_promise_unittest.cc index 9983fb17f2ef..4cb0fc889480 100644 --- a/components/brave_news/browser/initialization_promise_unittest.cc +++ b/components/brave_news/browser/initialization_promise_unittest.cc @@ -5,6 +5,7 @@ #include "brave/components/brave_news/browser/initialization_promise.h" +#include #include #include @@ -17,9 +18,10 @@ #include "brave/components/brave_news/browser/publishers_controller.h" #include "brave/components/brave_news/browser/urls.h" #include "brave/components/brave_news/common/brave_news.mojom.h" +#include "brave/components/brave_news/common/pref_names.h" #include "brave/components/brave_news/common/subscriptions_snapshot.h" #include "brave/components/l10n/common/test/scoped_default_locale.h" -#include "chrome/test/base/testing_profile.h" +#include "components/sync_preferences/testing_pref_service_syncable.h" #include "content/public/test/browser_task_environment.h" #include "net/http/http_status_code.h" #include "net/traffic_annotation/network_traffic_annotation_test_helper.h" @@ -52,30 +54,34 @@ class BraveNewsInitializationPromiseTest : public testing::Test { public: BraveNewsInitializationPromiseTest() : api_request_helper_(TRAFFIC_ANNOTATION_FOR_TESTS, - test_url_loader_factory_.GetSafeWeakWrapper()), - pref_manager_(*profile_.GetPrefs()), - publishers_controller_(&api_request_helper_), - initialization_promise_( - 3, - pref_manager_, - base::BindRepeating(&PublishersController::GetLocale, - base::Unretained(&publishers_controller_), - pref_manager_.GetSubscriptions())) { + test_url_loader_factory_.GetSafeWeakWrapper()) { + prefs::RegisterProfilePrefs(pref_service_.registry()); + + pref_manager_ = std::make_unique(pref_service_); // Ensure Brave News is enabled. - pref_manager_.SetConfig(mojom::Configuration::New(true, true, true)); + pref_manager_->SetConfig(mojom::Configuration::New(true, true, true)); + + publishers_controller_ = + std::make_unique(&api_request_helper_); + + initialization_promise_ = std::make_unique( + 3, *pref_manager_, + base::BindRepeating(&PublishersController::GetLocale, + base::Unretained(publishers_controller_.get()), + pref_manager_->GetSubscriptions())); // Disable backoffs, so we can test. - initialization_promise_.set_no_retry_delay_for_testing(true); + initialization_promise_->set_no_retry_delay_for_testing(true); } ~BraveNewsInitializationPromiseTest() override = default; protected: std::string InitializeAndGetLocale() { base::RunLoop loop; - initialization_promise_.OnceInitialized(loop.QuitClosure()); + initialization_promise_->OnceInitialized(loop.QuitClosure()); loop.Run(); - auto channels = pref_manager_.GetSubscriptions().channels(); + auto channels = pref_manager_->GetSubscriptions().channels(); if (channels.empty()) { return ""; } @@ -91,7 +97,7 @@ class BraveNewsInitializationPromiseTest : public testing::Test { test_url_loader_factory_.SetInterceptor(base::BindLambdaForTesting( [&, retries](const network::ResourceRequest& request) { bool should_succeed = - initialization_promise_.attempts_for_testing() >= retries; + initialization_promise_->attempts_for_testing() >= retries; test_url_loader_factory_.AddResponse( request.url.spec(), should_succeed ? kPublishersResponse : "", should_succeed ? net::HTTP_OK : net::HTTP_SERVICE_UNAVAILABLE); @@ -103,10 +109,10 @@ class BraveNewsInitializationPromiseTest : public testing::Test { network::TestURLLoaderFactory test_url_loader_factory_; api_request_helper::APIRequestHelper api_request_helper_; - TestingProfile profile_; - BraveNewsPrefManager pref_manager_; - PublishersController publishers_controller_; - InitializationPromise initialization_promise_; + sync_preferences::TestingPrefServiceSyncable pref_service_; + std::unique_ptr pref_manager_; + std::unique_ptr publishers_controller_; + std::unique_ptr initialization_promise_; private: const brave_l10n::test::ScopedDefaultLocale locale_{"en_NZ"}; @@ -115,26 +121,26 @@ class BraveNewsInitializationPromiseTest : public testing::Test { TEST_F(BraveNewsInitializationPromiseTest, WaitingForInitializationChangesState) { EXPECT_EQ(InitializationPromise::State::kNone, - initialization_promise_.state()); - initialization_promise_.OnceInitialized(base::DoNothing()); + initialization_promise_->state()); + initialization_promise_->OnceInitialized(base::DoNothing()); EXPECT_EQ(InitializationPromise::State::kInitializing, - initialization_promise_.state()); + initialization_promise_->state()); } TEST_F(BraveNewsInitializationPromiseTest, InitializedLocaleIsCorrect) { SucceedAfter(0); auto locale = InitializeAndGetLocale(); EXPECT_EQ("en_NZ", locale); - EXPECT_EQ(1u, initialization_promise_.attempts_for_testing()); + EXPECT_EQ(1u, initialization_promise_->attempts_for_testing()); } TEST_F(BraveNewsInitializationPromiseTest, InitializationRetries) { SucceedAfter(1); auto locale = InitializeAndGetLocale(); EXPECT_EQ("en_NZ", locale); - EXPECT_TRUE(initialization_promise_.complete()); - EXPECT_FALSE(initialization_promise_.failed()); - EXPECT_EQ(2u, initialization_promise_.attempts_for_testing()); + EXPECT_TRUE(initialization_promise_->complete()); + EXPECT_FALSE(initialization_promise_->failed()); + EXPECT_EQ(2u, initialization_promise_->attempts_for_testing()); } TEST_F(BraveNewsInitializationPromiseTest, @@ -142,9 +148,9 @@ TEST_F(BraveNewsInitializationPromiseTest, SucceedAfter(4); auto locale = InitializeAndGetLocale(); EXPECT_EQ("", locale); - EXPECT_TRUE(initialization_promise_.complete()); - EXPECT_TRUE(initialization_promise_.failed()); - EXPECT_EQ(3u, initialization_promise_.attempts_for_testing()); + EXPECT_TRUE(initialization_promise_->complete()); + EXPECT_TRUE(initialization_promise_->failed()); + EXPECT_EQ(3u, initialization_promise_->attempts_for_testing()); } } // namespace brave_news diff --git a/components/brave_news/browser/publishers_controller_unittest.cc b/components/brave_news/browser/publishers_controller_unittest.cc index 1bce0447df39..a52efcc9c707 100644 --- a/components/brave_news/browser/publishers_controller_unittest.cc +++ b/components/brave_news/browser/publishers_controller_unittest.cc @@ -11,20 +11,18 @@ #include "base/containers/contains.h" #include "base/functional/bind.h" -#include "base/test/bind.h" #include "base/test/scoped_feature_list.h" #include "base/values.h" #include "brave/components/api_request_helper/api_request_helper.h" -#include "brave/components/brave_news/browser/brave_news_controller.h" #include "brave/components/brave_news/browser/brave_news_pref_manager.h" -#include "brave/components/brave_news/browser/direct_feed_controller.h" #include "brave/components/brave_news/browser/test/wait_for_callback.h" #include "brave/components/brave_news/browser/urls.h" #include "brave/components/brave_news/common/brave_news.mojom-forward.h" #include "brave/components/brave_news/common/locales_helper.h" #include "brave/components/brave_news/common/pref_names.h" -#include "chrome/test/base/testing_profile.h" +#include "brave/components/brave_news/common/subscriptions_snapshot.h" #include "components/prefs/scoped_user_pref_update.h" +#include "components/sync_preferences/testing_pref_service_syncable.h" #include "content/public/test/browser_task_environment.h" #include "net/traffic_annotation/network_traffic_annotation_test_helper.h" #include "services/data_decoder/public/cpp/test_support/in_process_data_decoder.h" @@ -86,12 +84,15 @@ class BraveNewsPublishersControllerTest : public testing::Test { public: BraveNewsPublishersControllerTest() : api_request_helper_(TRAFFIC_ANNOTATION_FOR_TESTS, - test_url_loader_factory_.GetSafeWeakWrapper()), - pref_manager_(*profile_.GetPrefs()), - publishers_controller_(&api_request_helper_) { - profile_.GetPrefs()->SetBoolean(brave_news::prefs::kBraveNewsOptedIn, true); - profile_.GetPrefs()->SetBoolean(brave_news::prefs::kNewTabPageShowToday, - true); + test_url_loader_factory_.GetSafeWeakWrapper()) { + prefs::RegisterProfilePrefs(pref_service_.registry()); + + pref_manager_ = std::make_unique(pref_service_); + // Ensure Brave News is enabled. + pref_manager_->SetConfig(mojom::Configuration::New(true, true, true)); + + publishers_controller_ = + std::make_unique(&api_request_helper_); } std::string GetSourcesUrl() { @@ -100,19 +101,19 @@ class BraveNewsPublishersControllerTest : public testing::Test { } void SetSubscribedSources(const std::vector& publisher_ids) { - ScopedDictPrefUpdate update(profile_.GetPrefs(), prefs::kBraveNewsSources); + ScopedDictPrefUpdate update(&pref_service_, prefs::kBraveNewsSources); for (const auto& id : publisher_ids) { update->Set(id, true); } } bool CombinedSourceExists(const std::string& publisher_id) { - const auto& value = profile_.GetPrefs()->GetDict(prefs::kBraveNewsSources); + const auto& value = pref_service_.GetDict(prefs::kBraveNewsSources); return value.FindBool(publisher_id).has_value(); } bool DirectSourceExists(const std::string& publisher_id) { - return base::Contains(pref_manager_.GetSubscriptions().direct_feeds(), + return base::Contains(pref_manager_->GetSubscriptions().direct_feeds(), publisher_id, &DirectFeed::id); } @@ -120,8 +121,8 @@ class BraveNewsPublishersControllerTest : public testing::Test { auto [publishers] = WaitForCallback(base::BindOnce( [](BraveNewsPublishersControllerTest* test, GetPublishersCallback callback) { - test->publishers_controller_.GetOrFetchPublishers( - test->pref_manager_.GetSubscriptions(), std::move(callback), + test->publishers_controller_->GetOrFetchPublishers( + test->pref_manager_->GetSubscriptions(), std::move(callback), true); }, base::Unretained(this))); @@ -132,8 +133,9 @@ class BraveNewsPublishersControllerTest : public testing::Test { auto [publisher] = WaitForCallback(base::BindOnce( [](BraveNewsPublishersControllerTest* test, const GURL& url, GetPublisherCallback callback) { - test->publishers_controller_.GetPublisherForSite( - test->pref_manager_.GetSubscriptions(), url, std::move(callback)); + test->publishers_controller_->GetPublisherForSite( + test->pref_manager_->GetSubscriptions(), url, + std::move(callback)); }, base::Unretained(this), url)); return std::move(publisher); @@ -143,8 +145,9 @@ class BraveNewsPublishersControllerTest : public testing::Test { auto [publisher] = WaitForCallback(base::BindOnce( [](BraveNewsPublishersControllerTest* test, const GURL& url, GetPublisherCallback callback) { - test->publishers_controller_.GetPublisherForFeed( - test->pref_manager_.GetSubscriptions(), url, std::move(callback)); + test->publishers_controller_->GetPublisherForFeed( + test->pref_manager_->GetSubscriptions(), url, + std::move(callback)); }, base::Unretained(this), url)); return std::move(publisher); @@ -156,15 +159,15 @@ class BraveNewsPublishersControllerTest : public testing::Test { data_decoder::test::InProcessDataDecoder data_decoder_; network::TestURLLoaderFactory test_url_loader_factory_; api_request_helper::APIRequestHelper api_request_helper_; - TestingProfile profile_; - BraveNewsPrefManager pref_manager_; - PublishersController publishers_controller_; + + sync_preferences::TestingPrefServiceSyncable pref_service_; + std::unique_ptr pref_manager_; + std::unique_ptr publishers_controller_; }; TEST_F(BraveNewsPublishersControllerTest, CanReceiveFeeds) { test_url_loader_factory_.AddResponse(GetSourcesUrl(), kPublishersResponse, net::HTTP_OK); - LOG(ERROR) << "Sources URL: " << GetSourcesUrl(); auto result = GetPublishers(); ASSERT_EQ(3u, result.size()); EXPECT_TRUE(base::Contains(result, "111")); @@ -231,10 +234,10 @@ TEST_F(BraveNewsPublishersControllerTest, "enabled": false }])", net::HTTP_OK); - auto [locale] = - WaitForCallback(base::BindOnce(&PublishersController::GetLocale, - base::Unretained(&publishers_controller_), - pref_manager_.GetSubscriptions())); + auto [locale] = WaitForCallback( + base::BindOnce(&PublishersController::GetLocale, + base::Unretained(publishers_controller_.get()), + pref_manager_->GetSubscriptions())); EXPECT_EQ("en_US", locale); @@ -281,10 +284,10 @@ TEST_F(BraveNewsPublishersControllerTest, net::HTTP_OK); GetPublishers(); - auto [locale] = - WaitForCallback(base::BindOnce(&PublishersController::GetLocale, - base::Unretained(&publishers_controller_), - pref_manager_.GetSubscriptions())); + auto [locale] = WaitForCallback( + base::BindOnce(&PublishersController::GetLocale, + base::Unretained(publishers_controller_.get()), + pref_manager_->GetSubscriptions())); EXPECT_EQ("en_US", locale); @@ -330,10 +333,10 @@ TEST_F(BraveNewsPublishersControllerTest, NoPreferredLocale_ReturnsFirstMatch) { net::HTTP_OK); GetPublishers(); - auto [locale] = - WaitForCallback(base::BindOnce(&PublishersController::GetLocale, - base::Unretained(&publishers_controller_), - pref_manager_.GetSubscriptions())); + auto [locale] = WaitForCallback( + base::BindOnce(&PublishersController::GetLocale, + base::Unretained(publishers_controller_.get()), + pref_manager_->GetSubscriptions())); EXPECT_EQ("en_US", locale); @@ -360,7 +363,7 @@ TEST_F(BraveNewsPublishersControllerTest, CacheCanBeCleared) { EXPECT_TRUE(GetPublisherForFeed(GURL("https://tp5.example.com/feed"))); - publishers_controller_.ClearCache(); + publishers_controller_->ClearCache(); // When there's nothing in the cache, we shouldn't be able to look up a // publisher by feed. @@ -384,10 +387,10 @@ TEST_F(BraveNewsPublishersControllerTest, LocaleDefaultsToENUS) { }])", net::HTTP_OK); - auto [locale] = - WaitForCallback(base::BindOnce(&PublishersController::GetLocale, - base::Unretained(&publishers_controller_), - pref_manager_.GetSubscriptions())); + auto [locale] = WaitForCallback( + base::BindOnce(&PublishersController::GetLocale, + base::Unretained(publishers_controller_.get()), + pref_manager_->GetSubscriptions())); EXPECT_EQ("en_US", locale); } diff --git a/components/brave_news/browser/suggestions_controller_unittest.cc b/components/brave_news/browser/suggestions_controller_unittest.cc index 38ea4f6c5ce2..83b5e5f73ce0 100644 --- a/components/brave_news/browser/suggestions_controller_unittest.cc +++ b/components/brave_news/browser/suggestions_controller_unittest.cc @@ -5,6 +5,7 @@ #include "brave/components/brave_news/browser/suggestions_controller.h" +#include #include #include @@ -15,12 +16,11 @@ #include "brave/components/api_request_helper/api_request_helper.h" #include "brave/components/brave_news/browser/background_history_querier.h" #include "brave/components/brave_news/browser/publishers_controller.h" -#include "brave/components/brave_news/common/brave_news.mojom-shared.h" #include "brave/components/brave_news/common/brave_news.mojom.h" #include "brave/components/brave_news/common/pref_names.h" -#include "chrome/test/base/testing_profile.h" #include "components/history/core/browser/history_types.h" #include "components/history/core/browser/url_row.h" +#include "components/sync_preferences/testing_pref_service_syncable.h" #include "content/public/test/browser_task_environment.h" #include "net/traffic_annotation/network_traffic_annotation_test_helper.h" #include "services/data_decoder/public/cpp/test_support/in_process_data_decoder.h" @@ -61,14 +61,19 @@ class BraveNewsSuggestionsControllerTest : public testing::Test { public: BraveNewsSuggestionsControllerTest() : api_request_helper_(TRAFFIC_ANNOTATION_FOR_TESTS, - test_url_loader_factory_.GetSafeWeakWrapper()), - publishers_controller_(&api_request_helper_), - suggestions_controller_(&publishers_controller_, - &api_request_helper_, - querier_) { - profile_.GetPrefs()->SetBoolean(brave_news::prefs::kBraveNewsOptedIn, true); - profile_.GetPrefs()->SetBoolean(brave_news::prefs::kNewTabPageShowToday, - true); + test_url_loader_factory_.GetSafeWeakWrapper()) { + prefs::RegisterProfilePrefs(pref_service_.registry()); + + pref_manager_ = std::make_unique(pref_service_); + // Ensure Brave News is enabled. + pref_manager_->SetConfig(mojom::Configuration::New(true, true, true)); + + publishers_controller_ = + std::make_unique(&api_request_helper_); + + suggestions_controller_ = std::make_unique( + publishers_controller_.get(), &api_request_helper_, querier_); + SetLocale("en_US"); } ~BraveNewsSuggestionsControllerTest() override = default; @@ -77,27 +82,28 @@ class BraveNewsSuggestionsControllerTest : public testing::Test { std::vector GetSuggestedPublisherIds( const Publishers& publishers, const history::QueryResults& history) { - return suggestions_controller_.GetSuggestedPublisherIdsWithHistory( + return suggestions_controller_->GetSuggestedPublisherIdsWithHistory( publishers, history); } void SetSimilarityMatrix( SuggestionsController::PublisherSimilarities similarities) { - suggestions_controller_.similarities_ = std::move(similarities); + suggestions_controller_->similarities_ = std::move(similarities); } void SetLocale(const std::string& locale) { - suggestions_controller_.locale_ = locale; + suggestions_controller_->locale_ = locale; } content::BrowserTaskEnvironment browser_task_environment_; - TestingProfile profile_; network::TestURLLoaderFactory test_url_loader_factory_; api_request_helper::APIRequestHelper api_request_helper_; BackgroundHistoryQuerier querier_ = base::DoNothing(); - PublishersController publishers_controller_; - SuggestionsController suggestions_controller_; + sync_preferences::TestingPrefServiceSyncable pref_service_; + std::unique_ptr pref_manager_; + std::unique_ptr publishers_controller_; + std::unique_ptr suggestions_controller_; }; TEST_F(BraveNewsSuggestionsControllerTest, VisitedSourcesAreSuggested) { diff --git a/components/brave_news/browser/test/BUILD.gn b/components/brave_news/browser/test/BUILD.gn index c387934be7a4..8ea5493bb109 100644 --- a/components/brave_news/browser/test/BUILD.gn +++ b/components/brave_news/browser/test/BUILD.gn @@ -40,9 +40,14 @@ source_set("brave_news_unit_tests") { "//brave/components/l10n/common", "//brave/components/l10n/common:test_support", "//brave/components/time_period_storage", - "//chrome/browser", - "//chrome/test:test_support", + "//components/history/core/browser", + "//components/prefs", + "//components/prefs:test_support", + "//components/sync_preferences:test_support", "//content/test:test_support", + "//net:test_support", + "//services/data_decoder/public/cpp:test_support", + "//services/network:test_support", "//testing/gmock", "//testing/gtest", "//url",