New issue
Advanced search Search tips

Issue 920277 link

Starred by 2 users
Status: Fixed
Owner:
ma...@igalia.com
Closed: Jan 15
Cc:
ma...@igalia.com
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Port AccountsMutator unittest to be in the same structure as PrimaryAccountMutator unittest

Project Member Reported by blundell@chromium.org, Jan 9 
The unittest is excellent :). It's also currently on AccountsMutatorImpl and exercises lots of the internals. Let's port it to the extremely clean model of PrimaryAccountMutator:

- Test is on AccountMutator.
- Instance being tested is the one obtained from IdentityManager.
- Test driving is done via IdentityTestEnvironment via the underlying classes.

This better ensures that the unittest is testing only the public interfaces and makes it more likely that the test will be robust in the face of implementation changes.

NOTE: This work should wait for Mario's CLs finalizing the AccountsMutator API surface (and adding initial tests) to land.

Comment 1 by ma...@igalia.com, Jan 10

Owner: ma...@igalia.com
I'll take care of this one once the work on the AccountsMutator API lands

Comment 2 by ma...@igalia.com, Jan 14

Status: Started (was: Available)
CL here: https://chromium-review.googlesource.com/c/chromium/src/+/1408972

Comment 3 by ma...@igalia.com, Jan 14 

> CL here: https://chromium-review.googlesource.com/c/chromium/src/+/1408972

Better this other CL though, which should hopefully be easier to review: https://chromium-review.googlesource.com/c/chromium/src/+/1408973
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 15 

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

commit 0ca377611eb523aa3d06983162f9d4d4d1b19ab2
Author: Mario Sanchez Prada <mario@igalia.com>
Date: Tue Jan 15 13:35:20 2019

Migrated AccountsMutator tests to the IdentityTestEnvironment

Move away from ProfileOAuth2TokenService and AccountTrackerService
and rely on the AccountsMutator returned by the IdentityManager
provided by IdentityTestEnvironment instead.

Bug:  920277 
Change-Id: If25fc3efe80bc14a8a99b849c38d52802ba7b89f
Reviewed-on: https://chromium-review.googlesource.com/c/1408973
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622836}
[modify] https://crrev.com/0ca377611eb523aa3d06983162f9d4d4d1b19ab2/services/identity/BUILD.gn
[modify] https://crrev.com/0ca377611eb523aa3d06983162f9d4d4d1b19ab2/services/identity/public/cpp/BUILD.gn
[delete] https://crrev.com/317923340a7c8e3c021ba791157a48cc4c5deb61/services/identity/public/cpp/accounts_mutator_impl_unittest.cc
[add] https://crrev.com/0ca377611eb523aa3d06983162f9d4d4d1b19ab2/services/identity/public/cpp/accounts_mutator_unittest.cc

Comment 5 by ma...@igalia.com, Jan 15

Status: Fixed (was: Started)
►

Sign in to add a comment