Commit d3d67d64 authored by Tanmoy Mollik's avatar Tanmoy Mollik Committed by Commit Bot

[Android] Change AccountManagerTestRule#addAccount return type to CoreAccountInfo

The sign in API uses CoreAccountInfo. So in the long run its better to
use CoreAccountInfo for helper methods that interact with sign in API.
This cl refactors AccountManagerTestRule so that the methods to add
account and check signed in account return CoreAccountInfo.

Bug: 1117006
Change-Id: I08245ea7b6325b1a62b3c27d53cc1f9877e9e75d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2521571Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Commit-Queue: Tanmoy Mollik <triploblastic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824832}
parent e2eb0d54
......@@ -13,7 +13,6 @@ import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import android.accounts.Account;
import android.content.ClipData;
import android.content.ClipboardManager;
import android.content.Context;
......@@ -55,6 +54,7 @@ import org.chromium.components.browser_ui.widget.DateDividedAdapter;
import org.chromium.components.browser_ui.widget.RecyclerViewTestUtils;
import org.chromium.components.browser_ui.widget.selectable_list.SelectableItemView;
import org.chromium.components.browser_ui.widget.selectable_list.SelectableItemViewHolder;
import org.chromium.components.signin.base.CoreAccountInfo;
import org.chromium.components.signin.metrics.SigninAccessPoint;
import org.chromium.components.signin.metrics.SignoutReason;
import org.chromium.components.user_prefs.UserPrefs;
......@@ -559,7 +559,7 @@ public class HistoryActivityTest {
// Sign in to account. Note that if supervised user is set before sign in, the supervised
// user setting will be reset.
final Account account =
final CoreAccountInfo coreAccountInfo =
mAccountManagerTestRule.addAccount(AccountManagerTestRule.TEST_ACCOUNT_EMAIL);
TestThreadUtils.runOnUiThreadBlocking(() -> {
Profile profile = Profile.getLastUsedRegularProfile();
......@@ -567,12 +567,12 @@ public class HistoryActivityTest {
IdentityServicesProvider.get().getSigninManager(profile).addSignInStateObserver(
mTestObserver);
IdentityServicesProvider.get().getSigninManager(profile).signinAndEnableSync(
SigninAccessPoint.UNKNOWN, account, null);
SigninAccessPoint.UNKNOWN, coreAccountInfo, null);
});
mTestObserver.onSigninStateChangedCallback.waitForCallback(
0, 1, SyncTestUtil.TIMEOUT_MS, TimeUnit.MILLISECONDS);
Assert.assertEquals(account, mAccountManagerTestRule.getCurrentSignedInAccount());
Assert.assertEquals(coreAccountInfo, mAccountManagerTestRule.getCurrentSignedInAccount());
// Wait for recycler view changes after sign in.
CriteriaHelper.pollUiThread(() -> !mRecyclerView.isAnimating());
......
......@@ -6,8 +6,6 @@ package org.chromium.chrome.browser.password_manager.settings;
import static org.mockito.Mockito.when;
import android.accounts.Account;
import androidx.test.filters.SmallTest;
import org.junit.Assert;
......@@ -53,13 +51,12 @@ public class PasswordViewingTypeTest {
private ChromeBasePreference mPasswordsPref;
private String mAuthority;
private Account mAccount;
@Mock
private AndroidSyncSettings mAndroidSyncSettings;
@Before
public void setUp() {
mAccount = mChromeBrowserTestRule.addAccount("account@example.com");
mChromeBrowserTestRule.addAccount("account@example.com");
mSettingsActivityTestRule.startSettingsActivity();
MainSettings mainSettings = mSettingsActivityTestRule.getFragment();
mPasswordsPref =
......
......@@ -16,7 +16,6 @@ import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.MockitoAnnotations.initMocks;
import android.accounts.Account;
import android.support.test.InstrumentationRegistry;
import androidx.test.filters.LargeTest;
......@@ -45,6 +44,7 @@ import org.chromium.chrome.test.util.ActivityUtils;
import org.chromium.chrome.test.util.BookmarkTestUtil;
import org.chromium.chrome.test.util.browser.signin.AccountManagerTestRule;
import org.chromium.components.signin.GAIAServiceType;
import org.chromium.components.signin.base.CoreAccountInfo;
import org.chromium.components.signin.identitymanager.ConsentLevel;
import org.chromium.components.signin.metrics.SigninAccessPoint;
import org.chromium.content_public.browser.test.util.CriteriaHelper;
......@@ -108,7 +108,7 @@ public class SigninSignoutIntegrationTest {
@Test
@LargeTest
public void testSignIn() {
Account account = mAccountManagerTestRule.addAccountAndWaitForSeeding(
CoreAccountInfo coreAccountInfo = mAccountManagerTestRule.addAccountAndWaitForSeeding(
AccountManagerTestRule.TEST_ACCOUNT_EMAIL);
SigninActivity signinActivity = ActivityUtils.waitForActivity(
InstrumentationRegistry.getInstrumentation(), SigninActivity.class, () -> {
......@@ -122,10 +122,8 @@ public class SigninSignoutIntegrationTest {
verify(mSignInStateObserverMock).onSignedIn();
verify(mSignInStateObserverMock, never()).onSignedOut();
TestThreadUtils.runOnUiThreadBlocking(() -> {
Assert.assertEquals(account.name,
mSigninManager.getIdentityManager()
.getPrimaryAccountInfo(ConsentLevel.SYNC)
.getEmail());
Assert.assertEquals(coreAccountInfo,
mSigninManager.getIdentityManager().getPrimaryAccountInfo(ConsentLevel.SYNC));
});
}
......@@ -226,10 +224,10 @@ public class SigninSignoutIntegrationTest {
}
private void signIn() {
Account account = mAccountManagerTestRule.addAccountAndWaitForSeeding(
CoreAccountInfo coreAccountInfo = mAccountManagerTestRule.addAccountAndWaitForSeeding(
AccountManagerTestRule.TEST_ACCOUNT_EMAIL);
TestThreadUtils.runOnUiThreadBlocking(() -> {
mSigninManager.signinAndEnableSync(SigninAccessPoint.SETTINGS, account, null);
mSigninManager.signinAndEnableSync(SigninAccessPoint.SETTINGS, coreAccountInfo, null);
});
assertSignedIn();
}
......
......@@ -169,26 +169,29 @@ public class SyncTestRule extends ChromeTabbedActivityTestRule {
/**
* Adds an account of default account name to AccountManagerFacade and waits for the seeding.
* TODO(https://crbug.com/1117006): Return CoreAccountInfo object
*/
public Account addTestAccount() {
Account account = mAccountManagerTestRule.addAccountAndWaitForSeeding(
AccountManagerTestRule.TEST_ACCOUNT_EMAIL);
Assert.assertFalse(SyncTestUtil.isSyncRequested());
return account;
return addAccount(AccountManagerTestRule.TEST_ACCOUNT_EMAIL);
}
/**
* Adds an account of given account name to AccountManagerFacade and waits for the seeding.
*/
public Account addAccount(String accountName) {
return mAccountManagerTestRule.addAccountAndWaitForSeeding(accountName);
CoreAccountInfo coreAccountInfo =
mAccountManagerTestRule.addAccountAndWaitForSeeding(accountName);
Assert.assertFalse(SyncTestUtil.isSyncRequested());
return CoreAccountInfo.getAndroidAccountFrom(coreAccountInfo);
}
/**
* Returns the currently signed in account.
* TODO(https://crbug.com/1117006): Return CoreAccountInfo object
*/
public Account getCurrentSignedInAccount() {
return mAccountManagerTestRule.getCurrentSignedInAccount();
return CoreAccountInfo.getAndroidAccountFrom(
mAccountManagerTestRule.getCurrentSignedInAccount());
}
/**
......
......@@ -4,10 +4,9 @@
package org.chromium.chrome.browser.sync;
import android.accounts.Account;
import org.chromium.base.annotations.CalledByNative;
import org.chromium.chrome.test.util.browser.signin.AccountManagerTestRule;
import org.chromium.components.signin.base.CoreAccountInfo;
/**
* Utility class for sign-in functionalities in native Sync browser tests.
......@@ -40,9 +39,9 @@ public final class SyncTestSigninUtils {
*/
@CalledByNative
private static void tearDownAuthForTesting() {
Account account = sAccountManagerTestRule.getCurrentSignedInAccount();
if (account != null) {
sAccountManagerTestRule.removeAccountAndWaitForSeeding(account.name);
CoreAccountInfo coreAccountInfo = sAccountManagerTestRule.getCurrentSignedInAccount();
if (coreAccountInfo != null) {
sAccountManagerTestRule.removeAccountAndWaitForSeeding(coreAccountInfo.getEmail());
}
sAccountManagerTestRule.tearDownRule();
}
......
......@@ -16,8 +16,6 @@ import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.MockitoAnnotations.initMocks;
import android.accounts.Account;
import androidx.fragment.app.FragmentActivity;
import org.junit.After;
......@@ -107,9 +105,8 @@ public class AccountPickerDelegateTest {
.thenReturn(mIdentityManagerMock);
when(IdentityServicesProvider.get().getSigninManager(any())).thenReturn(mSigninManagerMock);
Account account =
mCoreAccountInfo =
mAccountManagerTestRule.addAccount(AccountManagerTestRule.TEST_ACCOUNT_EMAIL);
mCoreAccountInfo = mAccountManagerTestRule.toCoreAccountInfo(account.name);
mDelegate = new AccountPickerDelegate(
mWindowAndroidMock, mTabMock, mWebSigninBridgeFactoryMock, CONTINUE_URL);
......
......@@ -4,8 +4,6 @@
package org.chromium.chrome.test;
import android.accounts.Account;
import org.junit.rules.TestRule;
import org.junit.runner.Description;
import org.junit.runners.model.Statement;
......@@ -40,7 +38,7 @@ public class ChromeBrowserTestRule implements TestRule {
/**
* Adds an account of the given accountName to the fake AccountManagerFacade.
*/
public Account addAccount(String accountName) {
public CoreAccountInfo addAccount(String accountName) {
return mAccountManagerTestRule.addAccount(accountName);
}
......
......@@ -91,21 +91,19 @@ public class AccountManagerTestRule implements TestRule {
}
/**
* TODO(https://crbug.com/1117006): Change the return type of addAccount() to CoreAccountInfo
*
* Add an account to the fake AccountManagerFacade.
* @return The account added.
* @return The CoreAccountInfo for the account added.
*/
public Account addAccount(Account account) {
public CoreAccountInfo addAccount(Account account) {
mFakeAccountManagerFacade.addAccount(account);
return account;
return toCoreAccountInfo(account.name);
}
/**
* Add an account of the given accountName to the fake AccountManagerFacade.
* @return The account added.
* @return The CoreAccountInfo for the account added.
*/
public Account addAccount(String accountName) {
public CoreAccountInfo addAccount(String accountName) {
return addAccount(AccountUtils.createAccountFromName(accountName));
}
......@@ -114,10 +112,10 @@ public class AccountManagerTestRule implements TestRule {
* ProfileDataSource of the fake AccountManagerFacade.
* @return The account added.
*/
public Account addAccount(ProfileDataSource.ProfileData profileData) {
Account account = addAccount(profileData.getAccountName());
public CoreAccountInfo addAccount(ProfileDataSource.ProfileData profileData) {
CoreAccountInfo coreAccountInfo = addAccount(profileData.getAccountName());
mFakeAccountManagerFacade.setProfileData(profileData.getAccountName(), profileData);
return account;
return coreAccountInfo;
}
/**
......@@ -133,12 +131,12 @@ public class AccountManagerTestRule implements TestRule {
*
* This method invokes native code. It shouldn't be called in a Robolectric test.
*/
public Account addAccountAndWaitForSeeding(String accountName) {
Account account = mFakeAccountManagerFacade.getProfileDataSource() == null
public CoreAccountInfo addAccountAndWaitForSeeding(String accountName) {
CoreAccountInfo coreAccountInfo = mFakeAccountManagerFacade.getProfileDataSource() == null
? addAccount(accountName)
: addAccount(createProfileDataFromName(accountName));
waitForSeeding();
return account;
return coreAccountInfo;
}
/**
......@@ -158,8 +156,7 @@ public class AccountManagerTestRule implements TestRule {
*/
public CoreAccountInfo addTestAccountThenSignin() {
assert !mIsSignedIn : "An account is already signed in!";
Account account = addAccountAndWaitForSeeding(TEST_ACCOUNT_EMAIL);
CoreAccountInfo coreAccountInfo = toCoreAccountInfo(account.name);
CoreAccountInfo coreAccountInfo = addAccountAndWaitForSeeding(TEST_ACCOUNT_EMAIL);
SigninTestUtil.signin(coreAccountInfo);
mIsSignedIn = true;
return coreAccountInfo;
......@@ -186,8 +183,7 @@ public class AccountManagerTestRule implements TestRule {
public CoreAccountInfo addTestAccountThenSigninAndEnableSync(
@Nullable ProfileSyncService profileSyncService) {
assert !mIsSignedIn : "An account is already signed in!";
Account account = addAccountAndWaitForSeeding(TEST_ACCOUNT_EMAIL);
CoreAccountInfo coreAccountInfo = toCoreAccountInfo(account.name);
CoreAccountInfo coreAccountInfo = addAccountAndWaitForSeeding(TEST_ACCOUNT_EMAIL);
SigninTestUtil.signinAndEnableSync(coreAccountInfo, profileSyncService);
mIsSignedIn = true;
return coreAccountInfo;
......@@ -209,7 +205,7 @@ public class AccountManagerTestRule implements TestRule {
*
* This method invokes native code. It shouldn't be called in a Robolectric test.
*/
public Account getCurrentSignedInAccount() {
public CoreAccountInfo getCurrentSignedInAccount() {
return SigninTestUtil.getCurrentAccount();
}
......
......@@ -4,8 +4,6 @@
package org.chromium.chrome.test.util.browser.signin;
import android.accounts.Account;
import androidx.annotation.Nullable;
import org.junit.Assert;
......@@ -31,14 +29,13 @@ import java.util.concurrent.TimeoutException;
*/
public final class SigninTestUtil {
/**
* Returns the currently signed in account.
* Returns the currently signed in coreAccountInfo.
*/
static Account getCurrentAccount() {
static CoreAccountInfo getCurrentAccount() {
return TestThreadUtils.runOnUiThreadBlockingNoException(() -> {
return CoreAccountInfo.getAndroidAccountFrom(
IdentityServicesProvider.get()
.getIdentityManager(Profile.getLastUsedRegularProfile())
.getPrimaryAccountInfo(ConsentLevel.SYNC));
return IdentityServicesProvider.get()
.getIdentityManager(Profile.getLastUsedRegularProfile())
.getPrimaryAccountInfo(ConsentLevel.SYNC);
});
}
......
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