Commit 867be2f7 authored by Sylvain Defresne's avatar Sylvain Defresne Committed by Commit Bot

Convert SyncUIUtilTest to use IdentityTestEnvironment

Instead of creating a TestingProfile with custom factories, create
fake for the required services and pass them directly to the code
that is tested.

This makes the tests simpler (as they do not have to create a full
TestingProfile but only need to create the necessary services). It
required adding an overload of sync_ui_util::GetStatusLabels() that
received the services instead of accessing them via their factories.

Bug: 984487
Change-Id: Ia596fb46b9915e240bfce2ef2e4ee888c99e16c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1715297
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Auto-Submit: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680488}
parent b96a52ad
......@@ -217,24 +217,33 @@ MessageType GetStatusLabelsImpl(
} // namespace
MessageType GetStatusLabels(Profile* profile,
MessageType GetStatusLabels(syncer::SyncService* sync_service,
signin::IdentityManager* identity_manager,
bool is_user_signout_allowed,
base::string16* status_label,
base::string16* link_label,
ActionType* action_type) {
DCHECK(profile);
syncer::SyncService* service =
ProfileSyncServiceFactory::GetForProfile(profile);
if (!service) {
if (!sync_service) {
// This can happen if Sync is disabled via the command line.
return PRE_SYNCED;
}
const bool is_user_signout_allowed =
signin_util::IsUserSignoutAllowedForProfile(profile);
CoreAccountInfo account_info = service->GetAuthenticatedAccountInfo();
DCHECK(identity_manager);
CoreAccountInfo account_info = sync_service->GetAuthenticatedAccountInfo();
GoogleServiceAuthError auth_error =
IdentityManagerFactory::GetForProfile(profile)
->GetErrorStateOfRefreshTokenForAccount(account_info.account_id);
return GetStatusLabelsImpl(service, is_user_signout_allowed, auth_error,
identity_manager->GetErrorStateOfRefreshTokenForAccount(
account_info.account_id);
return GetStatusLabelsImpl(sync_service, is_user_signout_allowed, auth_error,
status_label, link_label, action_type);
}
MessageType GetStatusLabels(Profile* profile,
base::string16* status_label,
base::string16* link_label,
ActionType* action_type) {
DCHECK(profile);
return GetStatusLabels(ProfileSyncServiceFactory::GetForProfile(profile),
IdentityManagerFactory::GetForProfile(profile),
signin_util::IsUserSignoutAllowedForProfile(profile),
status_label, link_label, action_type);
}
......
......@@ -10,6 +10,10 @@
class Profile;
namespace signin {
class IdentityManager;
} // namespace signin
namespace syncer {
class SyncService;
} // namespace syncer
......@@ -46,7 +50,20 @@ enum AvatarSyncErrorType {
};
// Returns the high-level sync status, and populates status and link label
// strings for the current sync status by querying |profile|.
// strings for the current sync status by querying |sync_service| and
// |identity_manager|. Any of |status_label|, |link_label|, and |action_type|
// may be null if the caller isn't interested in it.
MessageType GetStatusLabels(syncer::SyncService* sync_service,
signin::IdentityManager* identity_manager,
bool is_user_signout_allowed,
base::string16* status_label,
base::string16* link_label,
ActionType* action_type);
// Returns the high-level sync status, and populates status and link label
// strings for the current sync status by querying |profile|. This is a
// convenience version of GetStatusLabels that use the |sync_service| and
// |identity_manager| associated to |profile| via their respective factories.
// Any of |status_label|, |link_label|, and |action_type| may be null if the
// caller isn't interested in it.
MessageType GetStatusLabels(Profile* profile,
......
......@@ -2,32 +2,21 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include <memory>
#include "chrome/browser/sync/sync_ui_util.h"
#include <set>
#include <string>
#include <utility>
#include "base/bind.h"
#include "base/strings/string16.h"
#include "base/strings/utf_string_conversions.h"
#include "build/build_config.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/signin/identity_test_environment_profile_adaptor.h"
#include "chrome/browser/sync/profile_sync_service_factory.h"
#include "chrome/browser/sync/sync_ui_util.h"
#include "chrome/test/base/testing_profile.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "base/test/scoped_task_environment.h"
#include "components/signin/public/identity_manager/identity_test_environment.h"
#include "components/signin/public/identity_manager/identity_test_utils.h"
#include "components/sync/driver/test_sync_service.h"
#include "components/sync/engine/sync_engine.h"
#include "components/unified_consent/feature.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
using syncer::TestSyncService;
namespace {
// A number of distinct states of the SyncService can be generated for tests.
......@@ -46,27 +35,20 @@ enum DistinctState {
const char kTestUser[] = "test_user@test.com";
} // namespace
class SyncUIUtilTest : public testing::Test {
private:
content::TestBrowserThreadBundle thread_bundle_;
};
// Sets up a TestSyncService to emulate one of a number of distinct cases in
// order to perform tests on the generated messages. Returns the expected values
// for the MessageType and ActionType that sync_ui_util::GetStatusLabels should
// return.
std::pair<sync_ui_util::MessageType, sync_ui_util::ActionType>
SetUpDistinctCase(TestSyncService* service,
signin::IdentityManager* identity_manager,
int case_number) {
SetUpDistinctCase(syncer::TestSyncService* service,
signin::IdentityTestEnvironment* test_environment,
DistinctState case_number) {
switch (case_number) {
case STATUS_CASE_SETUP_IN_PROGRESS: {
service->SetFirstSetupComplete(false);
service->SetSetupInProgress(true);
service->SetDetailedSyncStatus(false, syncer::SyncStatus());
return std::make_tuple(sync_ui_util::PRE_SYNCED, sync_ui_util::NO_ACTION);
return std::make_pair(sync_ui_util::PRE_SYNCED, sync_ui_util::NO_ACTION);
}
case STATUS_CASE_SETUP_ERROR: {
service->SetFirstSetupComplete(false);
......@@ -74,7 +56,7 @@ SetUpDistinctCase(TestSyncService* service,
service->SetDisableReasons(
syncer::SyncService::DISABLE_REASON_UNRECOVERABLE_ERROR);
service->SetDetailedSyncStatus(false, syncer::SyncStatus());
return std::make_tuple(sync_ui_util::SYNC_ERROR,
return std::make_pair(sync_ui_util::SYNC_ERROR,
sync_ui_util::REAUTHENTICATE);
}
case STATUS_CASE_AUTH_ERROR: {
......@@ -84,15 +66,16 @@ SetUpDistinctCase(TestSyncService* service,
service->SetDetailedSyncStatus(false, syncer::SyncStatus());
// Make sure to fail authentication with an error in this case.
std::string account_id = identity_manager->GetPrimaryAccountId();
signin::SetRefreshTokenForPrimaryAccount(identity_manager);
std::string account_id =
test_environment->identity_manager()->GetPrimaryAccountId();
test_environment->SetRefreshTokenForPrimaryAccount();
service->SetAuthenticatedAccountInfo(
identity_manager->GetPrimaryAccountInfo());
signin::UpdatePersistentErrorOfRefreshTokenForAccount(
identity_manager, account_id,
test_environment->identity_manager()->GetPrimaryAccountInfo());
test_environment->UpdatePersistentErrorOfRefreshTokenForAccount(
account_id,
GoogleServiceAuthError(GoogleServiceAuthError::State::SERVICE_ERROR));
service->SetDisableReasons(syncer::SyncService::DISABLE_REASON_NONE);
return std::make_tuple(sync_ui_util::SYNC_ERROR,
return std::make_pair(sync_ui_util::SYNC_ERROR,
sync_ui_util::REAUTHENTICATE);
}
case STATUS_CASE_PROTOCOL_ERROR: {
......@@ -105,14 +88,14 @@ SetUpDistinctCase(TestSyncService* service,
status.sync_protocol_error = protocol_error;
service->SetDetailedSyncStatus(false, status);
service->SetDisableReasons(syncer::SyncService::DISABLE_REASON_NONE);
return std::make_tuple(sync_ui_util::SYNC_ERROR,
return std::make_pair(sync_ui_util::SYNC_ERROR,
sync_ui_util::UPGRADE_CLIENT);
}
case STATUS_CASE_CONFIRM_SYNC_SETTINGS: {
service->SetFirstSetupComplete(false);
service->SetPassphraseRequired(false);
service->SetDetailedSyncStatus(false, syncer::SyncStatus());
return std::make_tuple(sync_ui_util::SYNC_ERROR,
return std::make_pair(sync_ui_util::SYNC_ERROR,
sync_ui_util::CONFIRM_SYNC_SETTINGS);
}
case STATUS_CASE_PASSPHRASE_ERROR: {
......@@ -122,7 +105,7 @@ SetUpDistinctCase(TestSyncService* service,
service->SetDisableReasons(syncer::SyncService::DISABLE_REASON_NONE);
service->SetPassphraseRequired(true);
service->SetPassphraseRequiredForDecryption(true);
return std::make_tuple(sync_ui_util::SYNC_ERROR,
return std::make_pair(sync_ui_util::SYNC_ERROR,
sync_ui_util::ENTER_PASSPHRASE);
}
case STATUS_CASE_SYNCED: {
......@@ -131,7 +114,7 @@ SetUpDistinctCase(TestSyncService* service,
service->SetDetailedSyncStatus(false, syncer::SyncStatus());
service->SetDisableReasons(syncer::SyncService::DISABLE_REASON_NONE);
service->SetPassphraseRequired(false);
return std::make_tuple(sync_ui_util::SYNCED, sync_ui_util::NO_ACTION);
return std::make_pair(sync_ui_util::SYNCED, sync_ui_util::NO_ACTION);
}
case STATUS_CASE_SYNC_DISABLED_BY_POLICY: {
service->SetDisableReasons(
......@@ -140,7 +123,7 @@ SetUpDistinctCase(TestSyncService* service,
service->SetTransportState(syncer::SyncService::TransportState::DISABLED);
service->SetPassphraseRequired(false);
service->SetDetailedSyncStatus(false, syncer::SyncStatus());
return std::make_tuple(sync_ui_util::SYNCED, sync_ui_util::NO_ACTION);
return std::make_pair(sync_ui_util::SYNCED, sync_ui_util::NO_ACTION);
}
case STATUS_CASE_SYNC_RESET_FROM_DASHBOARD: {
// Note: On desktop, if there is a primary account, then
......@@ -158,69 +141,49 @@ SetUpDistinctCase(TestSyncService* service,
unified_consent::IsUnifiedConsentFeatureEnabled()
? sync_ui_util::SYNC_ERROR
: sync_ui_util::PRE_SYNCED;
return std::make_tuple(expected_message_type, sync_ui_util::NO_ACTION);
return std::make_pair(expected_message_type, sync_ui_util::NO_ACTION);
}
case NUMBER_OF_STATUS_CASES:
NOTREACHED();
}
return std::make_tuple(sync_ui_util::PRE_SYNCED, sync_ui_util::NO_ACTION);
return std::make_pair(sync_ui_util::PRE_SYNCED, sync_ui_util::NO_ACTION);
}
std::unique_ptr<KeyedService> BuildTestSyncService(
content::BrowserContext* context) {
return std::make_unique<TestSyncService>();
}
std::unique_ptr<TestingProfile> BuildTestingProfile() {
return IdentityTestEnvironmentProfileAdaptor::
CreateProfileForIdentityTestEnvironment(
{{ProfileSyncServiceFactory::GetInstance(),
base::BindRepeating(&BuildTestSyncService)}});
}
std::unique_ptr<TestingProfile> BuildSignedInTestingProfile() {
std::unique_ptr<TestingProfile> profile = BuildTestingProfile();
IdentityTestEnvironmentProfileAdaptor identity_adaptor(profile.get());
identity_adaptor.identity_test_env()->SetPrimaryAccount(kTestUser);
return profile;
}
} // namespace
// This test ensures that each distinctive SyncService status will return a
// unique combination of status and link messages from GetStatusLabels().
TEST_F(SyncUIUtilTest, DistinctCasesReportUniqueMessageSets) {
std::set<base::string16> messages;
for (int idx = 0; idx != NUMBER_OF_STATUS_CASES; idx++) {
std::unique_ptr<Profile> profile = BuildTestingProfile();
TEST(SyncUIUtilTest, DistinctCasesReportUniqueMessageSets) {
base::test::ScopedTaskEnvironment task_environment;
IdentityTestEnvironmentProfileAdaptor env_adaptor(profile.get());
signin::IdentityTestEnvironment* environment =
env_adaptor.identity_test_env();
signin::IdentityManager* identity_manager = environment->identity_manager();
std::set<base::string16> messages;
for (int index = 0; index != NUMBER_OF_STATUS_CASES; index++) {
syncer::TestSyncService service;
signin::IdentityTestEnvironment environment;
// Need a primary account signed in before calling SetUpDistinctCase().
environment->MakePrimaryAccountAvailable(kTestUser);
environment.MakePrimaryAccountAvailable(kTestUser);
TestSyncService* service = static_cast<TestSyncService*>(
ProfileSyncServiceFactory::GetForProfile(profile.get()));
sync_ui_util::MessageType expected_message_type;
sync_ui_util::ActionType expected_action_type;
std::tie(expected_message_type, expected_action_type) =
SetUpDistinctCase(service, identity_manager, idx);
std::tie(expected_message_type, expected_action_type) = SetUpDistinctCase(
&service, &environment, static_cast<DistinctState>(index));
base::string16 status_label;
base::string16 link_label;
sync_ui_util::ActionType action_type = sync_ui_util::NO_ACTION;
sync_ui_util::MessageType message_type = sync_ui_util::GetStatusLabels(
profile.get(), &status_label, &link_label, &action_type);
&service, environment.identity_manager(), true, &status_label,
&link_label, &action_type);
EXPECT_EQ(expected_message_type, message_type)
<< "Wrong message type returned for case #" << idx;
<< "Wrong message type returned for case #" << index;
EXPECT_EQ(expected_action_type, action_type)
<< "Wrong action returned for case #" << idx;
<< "Wrong action returned for case #" << index;
// If the status and link message combination is already present in the set
// of messages already seen, this is a duplicate rather than a unique
// message, and the test has failed.
EXPECT_FALSE(status_label.empty())
<< "Empty status label returned for case #" << idx;
<< "Empty status label returned for case #" << index;
// Ensures a search for string 'href' (found in links, not a string to be
// found in an English language message) fails, since links are excluded
// from the status label.
......@@ -228,28 +191,29 @@ TEST_F(SyncUIUtilTest, DistinctCasesReportUniqueMessageSets) {
base::string16::npos);
base::string16 combined_label =
status_label + base::ASCIIToUTF16("#") + link_label;
EXPECT_TRUE(messages.find(combined_label) == messages.end()) <<
"Duplicate message for case #" << idx << ": " << combined_label;
EXPECT_TRUE(messages.find(combined_label) == messages.end())
<< "Duplicate message for case #" << index << ": " << combined_label;
messages.insert(combined_label);
}
}
TEST_F(SyncUIUtilTest, UnrecoverableErrorWithActionableError) {
std::unique_ptr<Profile> profile = BuildSignedInTestingProfile();
TEST(SyncUIUtilTest, UnrecoverableErrorWithActionableError) {
base::test::ScopedTaskEnvironment task_environment;
syncer::TestSyncService service;
signin::IdentityTestEnvironment environment;
TestSyncService* service = static_cast<TestSyncService*>(
ProfileSyncServiceFactory::GetForProfile(profile.get()));
service->SetFirstSetupComplete(true);
service->SetDisableReasons(
environment.SetPrimaryAccount(kTestUser);
service.SetFirstSetupComplete(true);
service.SetDisableReasons(
syncer::SyncService::DISABLE_REASON_UNRECOVERABLE_ERROR);
// First time action is not set. We should get unrecoverable error.
service->SetDetailedSyncStatus(true, syncer::SyncStatus());
service.SetDetailedSyncStatus(true, syncer::SyncStatus());
base::string16 link_label;
base::string16 unrecoverable_error_status_label;
sync_ui_util::ActionType action_type = sync_ui_util::NO_ACTION;
sync_ui_util::GetStatusLabels(profile.get(),
sync_ui_util::GetStatusLabels(&service, environment.identity_manager(), true,
&unrecoverable_error_status_label, &link_label,
&action_type);
......@@ -260,77 +224,77 @@ TEST_F(SyncUIUtilTest, UnrecoverableErrorWithActionableError) {
// from previous one.
syncer::SyncStatus status;
status.sync_protocol_error.action = syncer::UPGRADE_CLIENT;
service->SetDetailedSyncStatus(true, status);
service.SetDetailedSyncStatus(true, status);
base::string16 upgrade_client_status_label;
sync_ui_util::GetStatusLabels(profile.get(), &upgrade_client_status_label,
&link_label, &action_type);
sync_ui_util::GetStatusLabels(&service, environment.identity_manager(), true,
&upgrade_client_status_label, &link_label,
&action_type);
// Expect an explicit 'client upgrade' action.
EXPECT_EQ(sync_ui_util::UPGRADE_CLIENT, action_type);
EXPECT_NE(unrecoverable_error_status_label, upgrade_client_status_label);
}
TEST_F(SyncUIUtilTest, ActionableErrorWithPassiveMessage) {
std::unique_ptr<Profile> profile = BuildSignedInTestingProfile();
TEST(SyncUIUtilTest, ActionableErrorWithPassiveMessage) {
base::test::ScopedTaskEnvironment task_environment;
syncer::TestSyncService service;
signin::IdentityTestEnvironment environment;
TestSyncService* service = static_cast<TestSyncService*>(
ProfileSyncServiceFactory::GetForProfile(profile.get()));
service->SetFirstSetupComplete(true);
service->SetDisableReasons(
environment.SetPrimaryAccount(kTestUser);
service.SetFirstSetupComplete(true);
service.SetDisableReasons(
syncer::SyncService::DISABLE_REASON_UNRECOVERABLE_ERROR);
// Set action to UPGRADE_CLIENT.
syncer::SyncStatus status;
status.sync_protocol_error.action = syncer::UPGRADE_CLIENT;
service->SetDetailedSyncStatus(true, status);
service.SetDetailedSyncStatus(true, status);
base::string16 actionable_error_status_label;
base::string16 link_label;
sync_ui_util::ActionType action_type = sync_ui_util::NO_ACTION;
sync_ui_util::GetStatusLabels(profile.get(), &actionable_error_status_label,
&link_label, &action_type);
sync_ui_util::GetStatusLabels(&service, environment.identity_manager(), true,
&actionable_error_status_label, &link_label,
&action_type);
// Expect a 'client upgrade' call to action.
EXPECT_EQ(sync_ui_util::UPGRADE_CLIENT, action_type);
EXPECT_NE(actionable_error_status_label, base::string16());
}
TEST_F(SyncUIUtilTest, SyncSettingsConfirmationNeededTest) {
std::unique_ptr<Profile> profile = BuildSignedInTestingProfile();
TEST(SyncUIUtilTest, SyncSettingsConfirmationNeededTest) {
base::test::ScopedTaskEnvironment task_environment;
syncer::TestSyncService service;
signin::IdentityTestEnvironment environment;
TestSyncService* service = static_cast<TestSyncService*>(
ProfileSyncServiceFactory::GetForProfile(profile.get()));
service->SetFirstSetupComplete(false);
ASSERT_TRUE(sync_ui_util::ShouldRequestSyncConfirmation(service));
environment.SetPrimaryAccount(kTestUser);
service.SetFirstSetupComplete(false);
ASSERT_TRUE(sync_ui_util::ShouldRequestSyncConfirmation(&service));
base::string16 actionable_error_status_label;
base::string16 link_label;
sync_ui_util::ActionType action_type = sync_ui_util::NO_ACTION;
sync_ui_util::GetStatusLabels(profile.get(), &actionable_error_status_label,
&link_label, &action_type);
sync_ui_util::GetStatusLabels(&service, environment.identity_manager(), true,
&actionable_error_status_label, &link_label,
&action_type);
EXPECT_EQ(action_type, sync_ui_util::CONFIRM_SYNC_SETTINGS);
}
// Errors in non-sync accounts should be ignored.
TEST_F(SyncUIUtilTest, IgnoreSyncErrorForNonSyncAccount) {
std::unique_ptr<Profile> profile = BuildTestingProfile();
IdentityTestEnvironmentProfileAdaptor env_adaptor(profile.get());
signin::IdentityTestEnvironment* environment =
env_adaptor.identity_test_env();
signin::IdentityManager* identity_manager = environment->identity_manager();
AccountInfo primary_account_info =
environment->MakePrimaryAccountAvailable(kTestUser);
TestSyncService* service = static_cast<TestSyncService*>(
ProfileSyncServiceFactory::GetForProfile(profile.get()));
service->SetAuthenticatedAccountInfo(primary_account_info);
service->SetFirstSetupComplete(true);
TEST(SyncUIUtilTest, IgnoreSyncErrorForNonSyncAccount) {
base::test::ScopedTaskEnvironment task_environment;
syncer::TestSyncService service;
signin::IdentityTestEnvironment environment;
const AccountInfo primary_account_info =
environment.MakePrimaryAccountAvailable(kTestUser);
service.SetAuthenticatedAccountInfo(primary_account_info);
service.SetFirstSetupComplete(true);
// Setup a secondary account.
AccountInfo secondary_account_info =
environment->MakeAccountAvailable("secondary-user@example.com");
const AccountInfo secondary_account_info =
environment.MakeAccountAvailable("secondary-user@example.com");
// Verify that we do not have any existing errors.
base::string16 actionable_error_status_label;
......@@ -338,20 +302,22 @@ TEST_F(SyncUIUtilTest, IgnoreSyncErrorForNonSyncAccount) {
sync_ui_util::ActionType action_type = sync_ui_util::NO_ACTION;
sync_ui_util::MessageType message = sync_ui_util::GetStatusLabels(
profile.get(), &actionable_error_status_label, &link_label, &action_type);
&service, environment.identity_manager(), true,
&actionable_error_status_label, &link_label, &action_type);
EXPECT_EQ(action_type, sync_ui_util::NO_ACTION);
EXPECT_EQ(message, sync_ui_util::MessageType::SYNCED);
// Add an error to the secondary account.
signin::UpdatePersistentErrorOfRefreshTokenForAccount(
identity_manager, secondary_account_info.account_id,
environment.UpdatePersistentErrorOfRefreshTokenForAccount(
secondary_account_info.account_id,
GoogleServiceAuthError(
GoogleServiceAuthError::State::INVALID_GAIA_CREDENTIALS));
// Verify that we do not see any sign-in errors.
message = sync_ui_util::GetStatusLabels(
profile.get(), &actionable_error_status_label, &link_label, &action_type);
&service, environment.identity_manager(), true,
&actionable_error_status_label, &link_label, &action_type);
EXPECT_EQ(action_type, sync_ui_util::NO_ACTION);
EXPECT_EQ(message, sync_ui_util::MessageType::SYNCED);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment