[Sync] Improve test to use content resolver

[Sync] Improve test to use content resolver

This more accurately simulates a user making changes through the Android
sync settings in an end-to-end way.

BUG=480604
Committed: https://crrev.com/bdfedd5af462a68b472a81257277c83c5781310e
Cr-Commit-Position: refs/heads/master@{#329905}

[maxbogue, 2015-05-12 00:19:04]

maxbogue@chromium.org changed reviewers:
+ maniscalco@chromium.org

[maxbogue, 2015-05-12 00:19:05]

Nick, PTAL. This is currently based off of http://crrev.com/1140593002, which is
why the trybots are failing.

[maniscalco, 2015-05-12 16:14:05]

https://codereview.chromium.org/1137073002/diff/1/chrome/android/sync_shell/j...
File
chrome/android/sync_shell/javatests/src/chromium/chrome/browser/sync/SyncTest.java
(right):

https://codereview.chromium.org/1137073002/diff/1/chrome/android/sync_shell/j...
chrome/android/sync_shell/javatests/src/chromium/chrome/browser/sync/SyncTest.java:142:
mSyncContentResolver.setSyncAutomatically(account, authority, false);
mSyncContentResolver is a MockSyncContentResolverDelegate, right?  I would have
expected SystemSyncContentResolverDelegate to be used in this test.  I guess
it's not clear to me which classes we're testing with this test case.  Is there
a reason we're using the mock instead of the real one?

[maxbogue, 2015-05-12 18:32:10]

https://codereview.chromium.org/1137073002/diff/1/chrome/android/sync_shell/j...
File
chrome/android/sync_shell/javatests/src/chromium/chrome/browser/sync/SyncTest.java
(right):

https://codereview.chromium.org/1137073002/diff/1/chrome/android/sync_shell/j...
chrome/android/sync_shell/javatests/src/chromium/chrome/browser/sync/SyncTest.java:142:
mSyncContentResolver.setSyncAutomatically(account, authority, false);
On 2015/05/12 16:14:05, maniscalco wrote:
> mSyncContentResolver is a MockSyncContentResolverDelegate, right?  I would
have
> expected SystemSyncContentResolverDelegate to be used in this test.  I guess
> it's not clear to me which classes we're testing with this test case.  Is
there
> a reason we're using the mock instead of the real one?

None of our tests actually add or remove accounts from Android. Doing so would
require a real account with real credentials.

This test is just doing a more end-to-end test of "Android sync switch" (aka the
content resolver; doesn't matter whether real or mocked) to "native sync state",
which effectively tests that AndroidSyncSettings and SyncController are properly
initialized and have their listeners set up.

[maniscalco, 2015-05-12 23:59:15]

lgtm

https://codereview.chromium.org/1137073002/diff/1/chrome/android/sync_shell/j...
File
chrome/android/sync_shell/javatests/src/chromium/chrome/browser/sync/SyncTest.java
(right):

https://codereview.chromium.org/1137073002/diff/1/chrome/android/sync_shell/j...
chrome/android/sync_shell/javatests/src/chromium/chrome/browser/sync/SyncTest.java:142:
mSyncContentResolver.setSyncAutomatically(account, authority, false);
On 2015/05/12 18:32:10, maxbogue wrote:
> On 2015/05/12 16:14:05, maniscalco wrote:
> > mSyncContentResolver is a MockSyncContentResolverDelegate, right?  I would
> have
> > expected SystemSyncContentResolverDelegate to be used in this test.  I guess
> > it's not clear to me which classes we're testing with this test case.  Is
> there
> > a reason we're using the mock instead of the real one?
> 
> None of our tests actually add or remove accounts from Android. Doing so would
> require a real account with real credentials.
> 
> This test is just doing a more end-to-end test of "Android sync switch" (aka
the
> content resolver; doesn't matter whether real or mocked) to "native sync
state",
> which effectively tests that AndroidSyncSettings and SyncController are
properly
> initialized and have their listeners set up.

I see.  Thanks for explaining.

[maxbogue, 2015-05-14 17:44:09]

maxbogue@chromium.org changed reviewers:
+ zea@chromium.org

[maxbogue, 2015-05-14 17:44:10]

+Nicolas for owner approval.

[Nicolas Zea, 2015-05-14 17:47:54]

lgtm

[maxbogue, 2015-05-14 19:38:15]

The CQ bit was checked by maxbogue@chromium.org

[maxbogue, 2015-05-14 19:38:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
maniscalco@chromium.org
Link to the patchset: https://codereview.chromium.org/1137073002/#ps20001
(title: "Rebase on master.")

[commit-bot: I haz the power, 2015-05-14 19:39:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1137073002/20001

[commit-bot: I haz the power, 2015-05-14 19:45:37]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-05-14 19:46:23]

Patchset 2 (id:??) landed as
https://crrev.com/bdfedd5af462a68b472a81257277c83c5781310e
Cr-Commit-Position: refs/heads/master@{#329905}