Skip to content

Commit

Permalink
Migrate iOS ClearBrowsingData tests to TestSyncService
Browse files Browse the repository at this point in the history 
...instead of ProfileSyncServiceMock, which is an antipattern.

Bug: 910518
Change-Id: Iec983a22725d3d766c40dbbf52953909134b7b18
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1528190
Auto-Submit: Marc Treib <treib@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#641651}
  • Loading branch information
Marc Treib authored and Commit Bot committed Mar 18, 2019
1 parent 524e636 commit a251a50
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 34 deletions.
3 changes: 1 addition & 2 deletions ios/chrome/browser/ui/settings/clear_browsing_data/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ source_set("unit_tests") {
":clear_browsing_data",
"//base",
"//base/test:test_support",
"//components/browser_sync:test_support",
"//components/browsing_data/core",
"//components/pref_registry",
"//components/prefs",
"//components/sync:test_support_driver",
"//components/sync_preferences",
"//components/sync_preferences:test_support",
"//ios/chrome/app/strings",
Expand All @@ -79,7 +79,6 @@ source_set("unit_tests") {
"//ios/chrome/browser/prefs:browser_prefs",
"//ios/chrome/browser/signin:test_support",
"//ios/chrome/browser/sync",
"//ios/chrome/browser/sync:test_support",
"//ios/chrome/browser/ui/collection_view",
"//ios/chrome/browser/ui/collection_view:test_support",
"//ios/chrome/browser/ui/settings/cells",
Expand Down
43 changes: 20 additions & 23 deletions ...i/settings/clear_browsing_data/clear_browsing_data_collection_view_controller_unittest.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@
#include "base/mac/foundation_util.h"
#include "base/memory/ptr_util.h"
#include "base/strings/sys_string_conversions.h"
#include "components/browser_sync/profile_sync_service_mock.h"
#include "components/browsing_data/core/browsing_data_utils.h"
#include "components/browsing_data/core/pref_names.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/sync/driver/test_sync_service.h"
#include "components/sync_preferences/pref_service_mock_factory.h"
#include "components/sync_preferences/pref_service_syncable.h"
#include "ios/chrome/browser/application_context.h"
Expand All @@ -23,7 +23,6 @@
#include "ios/chrome/browser/pref_names.h"
#include "ios/chrome/browser/prefs/browser_prefs.h"
#include "ios/chrome/browser/signin/identity_test_environment_chrome_browser_state_adaptor.h"
#include "ios/chrome/browser/sync/ios_chrome_profile_sync_test_util.h"
#include "ios/chrome/browser/sync/profile_sync_service_factory.h"
#import "ios/chrome/browser/ui/collection_view/collection_view_controller_test.h"
#import "ios/chrome/browser/ui/settings/cells/settings_text_item.h"
Expand All @@ -39,8 +38,6 @@
#error "This file requires ARC support."
#endif

using testing::Return;

@interface ClearBrowsingDataCollectionViewController (ExposedForTesting)
- (NSString*)counterTextFromResult:
(const browsing_data::BrowsingDataCounter::Result&)result;
Expand All @@ -56,6 +53,11 @@ - (NSString*)counterTextFromResult:
kDeleteFormDataItem
};

std::unique_ptr<KeyedService> CreateTestSyncService(
web::BrowserState* context) {
return std::make_unique<syncer::TestSyncService>();
}

class ClearBrowsingDataCollectionViewControllerTest
: public CollectionViewControllerTest {
protected:
Expand All @@ -65,7 +67,7 @@ void SetUp() override {
// Setup identity services.
TestChromeBrowserState::TestingFactories factories = {
{ProfileSyncServiceFactory::GetInstance(),
base::BindRepeating(&BuildMockProfileSyncService)},
base::BindRepeating(&CreateTestSyncService)},
};
browser_state_ = IdentityTestEnvironmentChromeBrowserStateAdaptor::
CreateChromeBrowserStateForIdentityTestEnvironment(factories);
Expand All @@ -74,7 +76,7 @@ void SetUp() override {
new IdentityTestEnvironmentChromeBrowserStateAdaptor(
browser_state_.get()));

mock_sync_service_ = static_cast<browser_sync::ProfileSyncServiceMock*>(
test_sync_service_ = static_cast<syncer::TestSyncService*>(
ProfileSyncServiceFactory::GetForBrowserState(browser_state_.get()));
}

Expand Down Expand Up @@ -108,14 +110,14 @@ void SelectItem(int item, int section) {
std::unique_ptr<TestChromeBrowserState> browser_state_;
std::unique_ptr<IdentityTestEnvironmentChromeBrowserStateAdaptor>
identity_test_env_adaptor_;
browser_sync::ProfileSyncServiceMock* mock_sync_service_;
syncer::TestSyncService* test_sync_service_;
};

// Tests ClearBrowsingDataCollectionViewControllerTest is set up with all
// appropriate items and sections.
TEST_F(ClearBrowsingDataCollectionViewControllerTest, TestModel) {
EXPECT_CALL(*mock_sync_service_, GetDisableReasons())
.WillRepeatedly(Return(syncer::SyncService::DISABLE_REASON_USER_CHOICE));
test_sync_service_->SetDisableReasons(
syncer::SyncService::DISABLE_REASON_USER_CHOICE);
CreateController();
CheckController();

Expand Down Expand Up @@ -148,8 +150,8 @@ void SelectItem(int item, int section) {

TEST_F(ClearBrowsingDataCollectionViewControllerTest,
TestItemsSignedInSyncOff) {
EXPECT_CALL(*mock_sync_service_, GetDisableReasons())
.WillRepeatedly(Return(syncer::SyncService::DISABLE_REASON_USER_CHOICE));
test_sync_service_->SetDisableReasons(
syncer::SyncService::DISABLE_REASON_USER_CHOICE);
identity_test_env()->SetPrimaryAccount("syncuser@example.com");
CreateController();
CheckController();
Expand All @@ -176,18 +178,13 @@ void SelectItem(int item, int section) {

TEST_F(ClearBrowsingDataCollectionViewControllerTest,
TestItemsSignedInSyncActiveHistoryOff) {
EXPECT_CALL(*mock_sync_service_, GetDisableReasons())
.WillRepeatedly(Return(syncer::SyncService::DISABLE_REASON_NONE));
EXPECT_CALL(*mock_sync_service_, GetTransportState())
.WillRepeatedly(Return(syncer::SyncService::TransportState::ACTIVE));
EXPECT_CALL(*mock_sync_service_->GetUserSettingsMock(),
IsFirstSetupComplete())
.WillRepeatedly(Return(true));
EXPECT_CALL(*mock_sync_service_, GetActiveDataTypes())
.WillRepeatedly(Return(syncer::ModelTypeSet()));
EXPECT_CALL(*mock_sync_service_->GetUserSettingsMock(),
IsUsingSecondaryPassphrase())
.WillRepeatedly(Return(true));
test_sync_service_->SetDisableReasons(
syncer::SyncService::DISABLE_REASON_NONE);
test_sync_service_->SetTransportState(
syncer::SyncService::TransportState::ACTIVE);
test_sync_service_->SetFirstSetupComplete(true);
test_sync_service_->SetActiveDataTypes(syncer::ModelTypeSet());
test_sync_service_->SetIsUsingSecondaryPassphrase(true);

identity_test_env()->SetPrimaryAccount("syncuser@example.com");
CreateController();
Expand Down
20 changes: 11 additions & 9 deletions ios/chrome/browser/ui/settings/clear_browsing_data/clear_browsing_data_manager_unittest.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
#import "ios/chrome/browser/ui/settings/clear_browsing_data/clear_browsing_data_manager.h"

#include "base/bind.h"
#include "components/browser_sync/profile_sync_service_mock.h"
#include "components/browsing_data/core/pref_names.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/sync/driver/test_sync_service.h"
#include "components/sync_preferences/pref_service_mock_factory.h"
#include "components/sync_preferences/pref_service_syncable.h"
#include "ios/chrome/browser/application_context.h"
Expand All @@ -17,7 +17,6 @@
#include "ios/chrome/browser/pref_names.h"
#include "ios/chrome/browser/prefs/browser_prefs.h"
#include "ios/chrome/browser/signin/identity_test_environment_chrome_browser_state_adaptor.h"
#include "ios/chrome/browser/sync/ios_chrome_profile_sync_test_util.h"
#include "ios/chrome/browser/sync/profile_sync_service_factory.h"
#import "ios/chrome/browser/ui/collection_view/collection_view_model.h"
#include "ios/web/public/test/test_web_thread_bundle.h"
Expand All @@ -30,14 +29,17 @@
#error "This file requires ARC support."
#endif

using testing::Return;

@interface ClearBrowsingDataManager (ExposedForTesting)
- (void)loadModel:(ListModel*)model;
@end

namespace {

std::unique_ptr<KeyedService> CreateTestSyncService(
web::BrowserState* context) {
return std::make_unique<syncer::TestSyncService>();
}

class ClearBrowsingDataManagerTest : public PlatformTest {
public:
ClearBrowsingDataManagerTest() {
Expand All @@ -50,7 +52,7 @@ - (void)loadModel:(ListModel*)model;

TestChromeBrowserState::TestingFactories factories = {
{ProfileSyncServiceFactory::GetInstance(),
base::BindRepeating(&BuildMockProfileSyncService)},
base::BindRepeating(&CreateTestSyncService)},
};
browser_state_ = IdentityTestEnvironmentChromeBrowserStateAdaptor::
CreateChromeBrowserStateForIdentityTestEnvironment(factories);
Expand All @@ -65,7 +67,7 @@ new IdentityTestEnvironmentChromeBrowserStateAdaptor(
listType:ClearBrowsingDataListType::
kListTypeCollectionView];

mock_sync_service_ = static_cast<browser_sync::ProfileSyncServiceMock*>(
test_sync_service_ = static_cast<syncer::TestSyncService*>(
ProfileSyncServiceFactory::GetForBrowserState(browser_state_.get()));
}

Expand All @@ -79,7 +81,7 @@ new IdentityTestEnvironmentChromeBrowserStateAdaptor(
identity_test_env_adaptor_;
CollectionViewModel* model_;
ClearBrowsingDataManager* manager_;
browser_sync::ProfileSyncServiceMock* mock_sync_service_;
syncer::TestSyncService* test_sync_service_;
web::TestWebThreadBundle thread_bundle_;
};

Expand All @@ -104,8 +106,8 @@ new IdentityTestEnvironmentChromeBrowserStateAdaptor(
// but sync is off.
TEST_F(ClearBrowsingDataManagerTest, TestModelSignedInSyncOff) {
// Ensure that sync is not running.
EXPECT_CALL(*mock_sync_service_, GetDisableReasons())
.WillRepeatedly(Return(syncer::SyncService::DISABLE_REASON_USER_CHOICE));
test_sync_service_->SetDisableReasons(
syncer::SyncService::DISABLE_REASON_USER_CHOICE);

identity_test_env()->SetPrimaryAccount("syncuser@example.com");

Expand Down

0 comments on commit a251a50

Please sign in to comment.