New issue
Advanced search Search tips

Issue 892077 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 5
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Rename the IdentityManager unittests that reference "Before"

Project Member Reported by blundell@chromium.org, Oct 4

Issue description

The names of these tests are now outdated and should be changed. They were named from an earlier implementation where IdentityManager cached its state internally and updated that state (as well as notifying its observers) via custom callbacks from SigninManager/ProfileOAuth2TokenService. However, we've since simplified the implementation, and IdentityManager now acts as a straight passthrough to the backing classes. The tests are still relevant, as during the incremental conversion it's important that IdentityManager return consistent values from within a SigninManagerBase::Observer or OAuth2TokenService::Observer callback. However, they're not about IdentityManager getting "an event" before those. They should be renamed to things like "IdentityManagerGivesConsistentValuesFromSigninManagerObserverNotificationOfSignin" and etc. 
 
Owner: svil...@igalia.com
Status: Assigned (was: Available)
Taking this
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/95e8813336d594a582af7426f39129912dd343f6

commit 95e8813336d594a582af7426f39129912dd343f6
Author: Sergio Villar Senin <svillar@igalia.com>
Date: Fri Oct 05 10:50:54 2018

Rename some IdentityManager unit tests referencing "EventBefore"

Early implementations of IdentityManager used to cache its internal
state and update it (notifying its observers) via custom
callbacks. That's no longer the case as IdentityManager acts now as a
passthrough to the backing classes. There is no guarantee that the
IdentityManager is going to receive the "events" before any other
observer.

The tests are still valid though, as they can be used to verify
that we get consistent values from IdentityManager from within
SigninManagerBase::Observer and OAuth2TokenService::Observer while
we transition all of these observers (and the flows that start from
them) incrementally to IdentityManager.

Bug:  892077 
Change-Id: I4bfea3f5114c5ad21444af43e6bfd119ffce65ba
Reviewed-on: https://chromium-review.googlesource.com/c/1264199
Commit-Queue: Sergio Villar <svillar@igalia.com>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597068}
[modify] https://crrev.com/95e8813336d594a582af7426f39129912dd343f6/services/identity/public/cpp/identity_manager_unittest.cc

Status: Fixed (was: Assigned)
Closing...

Sign in to add a comment