Commit 7ccdfe5e authored by Mikel Astiz's avatar Mikel Astiz Committed by Commit Bot

Treat unknown accounts as auth errors in tests

We have multiple flaky tests that complain with:
java.lang.IllegalArgumentException: Can not find AccountHolder for account Account {name=test@gmail.com, type=com.google}
    at org.chromium.components.signin.test.util.FakeAccountManagerDelegate.getAccountHolder(FakeAccountManagerDelegate.java:341)
    at org.chromium.components.signin.test.util.FakeAccountManagerDelegate.getAuthToken(FakeAccountManagerDelegate.java:259)
    at org.chromium.components.signin.AccountManagerFacade$3.run(AccountManagerFacade.java:492)
    at org.chromium.components.signin.AccountManagerFacade$3.run(AccountManagerFacade.java:489)
    at org.chromium.components.signin.AccountManagerFacade$ConnectionRetry$1.doInBackground(AccountManagerFacade.java:829)
    at org.chromium.base.task.AsyncTask$1.call(AsyncTask.java:99)
    at java.util.concurrent.FutureTask.run(FutureTask.java:237)
    ... 3 more

One hypothesis is that GetAccountHolder() fails during teardown of the
test. Hence, let's treat it as auth error such that
AccountManagerFacade.getAuthToken() treats it as a regular failed
attempt (but doesn't necessarily make the test fail).

Bug: 879246
Change-Id: I47c69af1af2579ec70ece468e6701f67ef065992
Reviewed-on: https://chromium-review.googlesource.com/c/1264678Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597586}
parent 4b5bc563
...@@ -37,6 +37,17 @@ public class AuthException extends Exception { ...@@ -37,6 +37,17 @@ public class AuthException extends Exception {
mIsTransientError = isTransientError; mIsTransientError = isTransientError;
} }
/**
* Constructs an instance without a wrapped exception, based on transience flag and message.
* @param isTransientError Whether the error is transient and we can retry.
* Use {@link #TRANSIENT} and {@link #NONTRANSIENT} for readability.
* @param message Message describing context in which auth failure happened.
*/
public AuthException(boolean isTransientError, String message) {
super(message);
mIsTransientError = isTransientError;
}
/** /**
* @return Whether the error is transient and we can retry. * @return Whether the error is transient and we can retry.
*/ */
......
...@@ -21,6 +21,7 @@ import org.chromium.components.signin.AccountManagerDelegate; ...@@ -21,6 +21,7 @@ import org.chromium.components.signin.AccountManagerDelegate;
import org.chromium.components.signin.AccountManagerDelegateException; import org.chromium.components.signin.AccountManagerDelegateException;
import org.chromium.components.signin.AccountManagerFacade; import org.chromium.components.signin.AccountManagerFacade;
import org.chromium.components.signin.AccountsChangeObserver; import org.chromium.components.signin.AccountsChangeObserver;
import org.chromium.components.signin.AuthException;
import org.chromium.components.signin.ProfileDataSource; import org.chromium.components.signin.ProfileDataSource;
import java.lang.annotation.Retention; import java.lang.annotation.Retention;
...@@ -255,8 +256,12 @@ public class FakeAccountManagerDelegate implements AccountManagerDelegate { ...@@ -255,8 +256,12 @@ public class FakeAccountManagerDelegate implements AccountManagerDelegate {
} }
@Override @Override
public String getAuthToken(Account account, String authTokenScope) { public String getAuthToken(Account account, String authTokenScope) throws AuthException {
AccountHolder ah = getAccountHolder(account); AccountHolder ah = tryGetAccountHolder(account);
if (ah == null) {
throw new AuthException(AuthException.NONTRANSIENT,
"Cannot get auth token for unknown account '" + account + "'");
}
assert ah.hasBeenAccepted(authTokenScope); assert ah.hasBeenAccepted(authTokenScope);
synchronized (mAccounts) { synchronized (mAccounts) {
// Some tests register auth tokens with value null, and those should be preserved. // Some tests register auth tokens with value null, and those should be preserved.
...@@ -327,7 +332,7 @@ public class FakeAccountManagerDelegate implements AccountManagerDelegate { ...@@ -327,7 +332,7 @@ public class FakeAccountManagerDelegate implements AccountManagerDelegate {
ThreadUtils.postOnUiThread(() -> callback.onResult(true)); ThreadUtils.postOnUiThread(() -> callback.onResult(true));
} }
private AccountHolder getAccountHolder(Account account) { private AccountHolder tryGetAccountHolder(Account account) {
if (account == null) { if (account == null) {
throw new IllegalArgumentException("Account can not be null"); throw new IllegalArgumentException("Account can not be null");
} }
...@@ -338,6 +343,14 @@ public class FakeAccountManagerDelegate implements AccountManagerDelegate { ...@@ -338,6 +343,14 @@ public class FakeAccountManagerDelegate implements AccountManagerDelegate {
} }
} }
} }
throw new IllegalArgumentException("Can not find AccountHolder for account " + account); return null;
}
private AccountHolder getAccountHolder(Account account) {
AccountHolder ah = tryGetAccountHolder(account);
if (ah == null) {
throw new IllegalArgumentException("Can not find AccountHolder for account " + account);
}
return ah;
} }
} }
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