Add more integration tests for backup/rollback.

Add more integration tests for backup/rollback.

BUG=387862
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=281782

[pval...(no longer on Chromium), 2014-06-28 00:12:00]

These tests are great and should prove valuable as we enable this feature. Just
a few comments and questions.

https://codereview.chromium.org/344193005/diff/1/chrome/browser/sync/test/int...
File chrome/browser/sync/test/integration/single_client_backup_rollback_test.cc
(right):

https://codereview.chromium.org/344193005/diff/1/chrome/browser/sync/test/int...
chrome/browser/sync/test/integration/single_client_backup_rollback_test.cc:98:
ASSERT_FALSE(checker.Wait());
does Wait() have to wait the entire timeout to eventually return false?

https://codereview.chromium.org/344193005/diff/1/chrome/browser/sync/test/int...
chrome/browser/sync/test/integration/single_client_backup_rollback_test.cc:141:
Remove(0, GetOtherNode(0), 0);
Is there another way we can trigger a sync that wouldn't mess with the bookmark
model?

I'm thinking that we should maybe trigger an invalidation on calls to
TriggerError (this would be fairly easy within FakeServer). Thoughts?

https://codereview.chromium.org/344193005/diff/1/chrome/browser/sync/test/int...
chrome/browser/sync/test/integration/single_client_backup_rollback_test.cc:143:
// Wait for sync to switch to backup mode after finishing rollback.
Update comment to reflect that a rollback isn't actually happening

[haitaol1, 2014-07-01 16:49:37]

https://codereview.chromium.org/344193005/diff/1/chrome/browser/sync/test/int...
File chrome/browser/sync/test/integration/single_client_backup_rollback_test.cc
(right):

https://codereview.chromium.org/344193005/diff/1/chrome/browser/sync/test/int...
chrome/browser/sync/test/integration/single_client_backup_rollback_test.cc:98:
ASSERT_FALSE(checker.Wait());
Unfortunately yes because backup controller won't do anything if disabled.

On 2014/06/28 00:12:00, pvalenzuela wrote:
> does Wait() have to wait the entire timeout to eventually return false?

https://codereview.chromium.org/344193005/diff/1/chrome/browser/sync/test/int...
chrome/browser/sync/test/integration/single_client_backup_rollback_test.cc:141:
Remove(0, GetOtherNode(0), 0);
Can you point me to a sample to trigger invalidation?

On 2014/06/28 00:12:00, pvalenzuela wrote:
> Is there another way we can trigger a sync that wouldn't mess with the
bookmark
> model?
> 
> I'm thinking that we should maybe trigger an invalidation on calls to
> TriggerError (this would be fairly easy within FakeServer). Thoughts?

https://codereview.chromium.org/344193005/diff/1/chrome/browser/sync/test/int...
chrome/browser/sync/test/integration/single_client_backup_rollback_test.cc:143:
// Wait for sync to switch to backup mode after finishing rollback.
On 2014/06/28 00:12:00, pvalenzuela wrote:
> Update comment to reflect that a rollback isn't actually happening

Done.

[pval...(no longer on Chromium), 2014-07-02 18:10:59]

lgtm

https://codereview.chromium.org/344193005/diff/1/chrome/browser/sync/test/int...
File chrome/browser/sync/test/integration/single_client_backup_rollback_test.cc
(right):

https://codereview.chromium.org/344193005/diff/1/chrome/browser/sync/test/int...
chrome/browser/sync/test/integration/single_client_backup_rollback_test.cc:141:
Remove(0, GetOtherNode(0), 0);
On 2014/07/01 16:49:36, haitaol1 wrote:
> Can you point me to a sample to trigger invalidation?
> 
> On 2014/06/28 00:12:00, pvalenzuela wrote:
> > Is there another way we can trigger a sync that wouldn't mess with the
> bookmark
> > model?
> > 
> > I'm thinking that we should maybe trigger an invalidation on calls to
> > TriggerError (this would be fairly easy within FakeServer). Thoughts?
> 

Let's save it for another CL. Add a TODO here (in my name) to trigger an
invaldiation as part of the rollback triggering.

[pval...(no longer on Chromium), 2014-07-02 18:11:23]

BTW I believe you need approval from a committer as well

[haitaol1, 2014-07-02 20:54:27]

+zea for owner approval.

[haitaol1, 2014-07-05 15:02:41]

The CQ bit was checked by haitaol@chromium.org

[commit-bot: I haz the power, 2014-07-05 15:03:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/haitaol@chromium.org/344193005/20001

[commit-bot: I haz the power, 2014-07-05 15:03:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-05 15:03:20]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Nicolas Zea, 2014-07-07 18:00:23]

lgtm

[haitaol1, 2014-07-07 20:05:18]

The CQ bit was checked by haitaol@chromium.org

[commit-bot: I haz the power, 2014-07-07 20:06:12]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/haitaol@chromium.org/344193005/40001

[commit-bot: I haz the power, 2014-07-07 20:14:50]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/9...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[commit-bot: I haz the power, 2014-07-07 20:20:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-07 20:20:48]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[haitaol1, 2014-07-07 23:10:27]

The CQ bit was checked by haitaol@chromium.org

[commit-bot: I haz the power, 2014-07-07 23:11:33]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/haitaol@chromium.org/344193005/40001

[commit-bot: I haz the power, 2014-07-08 00:28:31]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/9...)

[commit-bot: I haz the power, 2014-07-08 01:03:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-08 01:03:32]

Try jobs failed on following builders:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/9...)

[haitaol1, 2014-07-08 18:08:04]

The CQ bit was checked by haitaol@chromium.org

[commit-bot: I haz the power, 2014-07-08 18:09:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/haitaol@chromium.org/344193005/40001

[commit-bot: I haz the power, 2014-07-08 19:02:05]

Change committed as 281782