Convert account_reconcilor_unittest.cc to IdentityManager |
|||
Issue descriptionProduction code was migrated [1], and unit tests were minimally updated to keep running. Now it is time to fully migrate the account_reconcilor_unittest.cc code away from SigninManager and PO2TS. [1] https://chromium-review.googlesource.com/c/1330223
,
Dec 4
,
Dec 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0d46e46ee383877ad62974bf266d5bbc96c2f056 commit 0d46e46ee383877ad62974bf266d5bbc96c2f056 Author: Antonio Gomes <tonikitoo@igalia.com> Date: Thu Dec 06 13:09:06 2018 [s13n] Convert account_reconcilor_unittest.cc to IdentityManager This CL migrates (most of) account_reconcilor_unittest.cc code away from SigninManager and PO2TS. Production code was migrated and unit tests were minimally updated to keep running in [1]. Remarks: - The CL can completely remove SigninManager direct API calls. - The CL could not completely remove direct PO2TS API calls, due to two missing API: set_all_credentials_loaded_for_testing and LoadCredentials. - Given how the test uses gaia_id to manipulate reconciles and how IdentityTestEnvironment hides setting the gaia_id from the tests, there were some adaptations needed so that the functionalities could continue fully tested. For instance, the following methods were added/ used: * GaiaIdForAccountKey: Added. * identity::GetTestGaiaIdForEmail: Referenced. [1] https://chromium-review.googlesource.com/c/1330223 BUG=910581 Change-Id: I58db6044caa22b6072cb6d22dd7c97c668c46cd3 Reviewed-on: https://chromium-review.googlesource.com/c/1357059 Reviewed-by: David Roger <droger@chromium.org> Reviewed-by: Colin Blundell <blundell@chromium.org> Commit-Queue: Antonio Gomes <tonikitoo@igalia.com> Cr-Commit-Position: refs/heads/master@{#614327} [modify] https://crrev.com/0d46e46ee383877ad62974bf266d5bbc96c2f056/components/signin/core/browser/account_reconcilor_unittest.cc
,
Dec 10
Hi Antonio, Do you know if there is a bug for migrating AccountReconcilor away from ProfileOAuth2TokenService? Is this a task that you already had in your queue? Thanks!
,
Dec 11
Hi Colin. There used to be bug 903907 (which I have merged a patch for, https://crrev.com/c/1330223), but as you said, it only moved it away from SigninManager. We still need to migrate it away from PO2TS. I have reopened it and retitled it accordingly.
,
Dec 12
Thank you!
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/745381ba737bfdc1a6f678fbbdb066a1f8045693 commit 745381ba737bfdc1a6f678fbbdb066a1f8045693 Author: Antonio Gomes <tonikitoo@igalia.com> Date: Thu Dec 13 13:45:48 2018 [s13n] Provide replacement API for PO2TS::set_all_credentials_loaded_for_testing CL is another step forward eliminating all direct calls to PO2TS APIs from AccountReconcilor unittests. Particularly, it adds a new API to our testing infrastructure, IdentityTestEnvironment. BUG= 911641 ,910581 Change-Id: I1ff3a8f287fff0f5b4bfbffe2e2d6943ba36ec52 Reviewed-on: https://chromium-review.googlesource.com/c/1372426 Reviewed-by: David Roger <droger@chromium.org> Reviewed-by: Colin Blundell <blundell@chromium.org> Commit-Queue: Antonio Gomes <tonikitoo@igalia.com> Cr-Commit-Position: refs/heads/master@{#616298} [modify] https://crrev.com/745381ba737bfdc1a6f678fbbdb066a1f8045693/components/signin/core/browser/account_reconcilor_unittest.cc [modify] https://crrev.com/745381ba737bfdc1a6f678fbbdb066a1f8045693/services/identity/public/cpp/identity_test_environment.cc [modify] https://crrev.com/745381ba737bfdc1a6f678fbbdb066a1f8045693/services/identity/public/cpp/identity_test_environment.h
,
Jan 4
Update: what is missing here in a moving away from a direct call to PO2TS::LoadCredentials:
TEST_P(AccountReconcilorMirrorEndpointParamTest, TokensNotLoaded) {
const std::string account_id =
ConnectProfileToAccount("user@gmail.com").account_id;
cookie_manager_service()->SetListAccountsResponseNoAccounts();
identity_test_env()->ResetToAccountsNotYetLoadedFromDiskState();
AccountReconcilor* reconcilor = GetMockReconcilor();
reconcilor->StartReconcile();
#if !defined(OS_CHROMEOS)
// No reconcile when tokens are not loaded, except on ChromeOS where reconcile
// can start as long as the token service is not empty.
ASSERT_FALSE(reconcilor->is_reconcile_started_);
// When tokens are loaded, reconcile starts automatically.
token_service()->LoadCredentials(account_id);
,
Jan 15
|
|||
►
Sign in to add a comment |
|||
Comment 1 by blundell@chromium.org
, Nov 30Labels: -Pri-2 Pri-1