Flakiness in SingleClientPrintersSyncTest.EditPrinter

Flakiness in SingleClientPrintersSyncTest.EditPrinter.

The test needs to wait for sync to begin initializing before running.  This is the behavior of Chrome normally.

BUG=701999
TEST=Ran the SingleClientPrintersSyncTest on my workstation 100 times.  0 failures.

Review-Url: https://codereview.chromium.org/2758643002
Cr-Commit-Position: refs/heads/master@{#458158}
Committed: https://chromium.googlesource.com/chromium/src/+/9985aa81ecc5c3fc065c6707ef5b67b141a6394c

[skau, 2017-03-16 23:19:02]

Description was changed from

==========
Flakiness in SingleClientPrintersSyncTest.EditPrinter

BUG=
==========

to

==========
Flakiness in SingleClientPrintersSyncTest.EditPrinter.

The test needs to wait for sync to begin initializing before running.  This is
the behavior of Chrome normally.

BUG=701999
TEST=Ran the SingleClientPrintersSyncTest on my workstation 100 times.  0
failures.
==========

[skau, 2017-03-16 23:19:02]

skau@chromium.org changed reviewers:
+ stanisc@chromium.org

[skau, 2017-03-16 23:19:30]

stanisc: Please take a look.  You appear to be the package OWNER.

[skym, 2017-03-16 23:41:00]

skym@chromium.org changed reviewers:
+ skym@chromium.org

[skym, 2017-03-16 23:41:01]

lgtm

PrintersSyncBridge::StoreProxy should probably be using DCHECKs, not CHECK, as
per
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...

I'm not happy about needing this. I think this is a potential problem with all
model types that need to initialize really fast. They get accessed via a
BrowserContextKeyedServiceFactory, and are immediately attempted to be used. I'd
love it if ModelTypeStore could queue requests.

I don't want to block you, I want to keep integ tests running and coverage up.
But I think we (sync team) need to come up with a better solution than model
type owners like you peppering your integ tests with waits.

lgtm

+pavely@

https://codereview.chromium.org/2758643002/diff/1/chrome/browser/sync/test/in...
File chrome/browser/sync/test/integration/single_client_printers_sync_test.cc
(right):

https://codereview.chromium.org/2758643002/diff/1/chrome/browser/sync/test/in...
chrome/browser/sync/test/integration/single_client_printers_sync_test.cc:59: //
Wait for sync service initialization before proceeding.
It concerns me that this is needed, and that you're not adding it to any of the
other tests. Seems like an issue with all ModelTypeStore backed types, including
all of your other test cases.

[skau, 2017-03-18 00:04:55]

Thanks for the review.  The comments have been addressed.

https://codereview.chromium.org/2758643002/diff/1/chrome/browser/sync/test/in...
File chrome/browser/sync/test/integration/single_client_printers_sync_test.cc
(right):

https://codereview.chromium.org/2758643002/diff/1/chrome/browser/sync/test/in...
chrome/browser/sync/test/integration/single_client_printers_sync_test.cc:59: //
Wait for sync service initialization before proceeding.
On 2017/03/16 23:41:00, skym wrote:
> It concerns me that this is needed, and that you're not adding it to any of
the
> other tests. Seems like an issue with all ModelTypeStore backed types,
including
> all of your other test cases.

I've moved it into SetupSync so it's used in the other tests.  You are correct
that the other tests risk being flaky as well.

[skau, 2017-03-20 18:35:30]

The CQ bit was checked by skau@chromium.org

[skau, 2017-03-20 18:35:31]

The patchset sent to the CQ was uploaded after l-g-t-m from skym@chromium.org
Link to the patchset: https://codereview.chromium.org/2758643002/#ps40001
(title: "all tests")

[commit-bot: I haz the power, 2017-03-20 18:36:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-20 20:09:57]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490034930921990,
"parent_rev": "1506f35e2a82f1b50ba2aa51e61c60891dc8cbcc", "commit_rev":
"9985aa81ecc5c3fc065c6707ef5b67b141a6394c"}

[commit-bot: I haz the power, 2017-03-20 20:10:42]

Description was changed from

==========
Flakiness in SingleClientPrintersSyncTest.EditPrinter.

The test needs to wait for sync to begin initializing before running.  This is
the behavior of Chrome normally.

BUG=701999
TEST=Ran the SingleClientPrintersSyncTest on my workstation 100 times.  0
failures.
==========

to

==========
Flakiness in SingleClientPrintersSyncTest.EditPrinter.

The test needs to wait for sync to begin initializing before running.  This is
the behavior of Chrome normally.

BUG=701999
TEST=Ran the SingleClientPrintersSyncTest on my workstation 100 times.  0
failures.

Review-Url: https://codereview.chromium.org/2758643002
Cr-Commit-Position: refs/heads/master@{#458158}
Committed:
https://chromium.googlesource.com/chromium/src/+/9985aa81ecc5c3fc065c6707ef5b...
==========

[commit-bot: I haz the power, 2017-03-20 20:10:43]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/9985aa81ecc5c3fc065c6707ef5b...