Sync: Print FakeServer contents on test failure

Sync: Print FakeServer contents on test failure

This CL adds additional logging to the FakeServer verification methods
of FakeServerVerifier.

BUG=NONE

Committed: https://crrev.com/a2ae52554de792f33e6818883d9212bdaf6fb54b
Cr-Commit-Position: refs/heads/master@{#321914}

[pval...(no longer on Chromium), 2015-03-13 22:23:08]

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

[pval...(no longer on Chromium), 2015-03-13 22:23:09]

Here's a little debugging info I've been wanting to add to FakeServer for a
while.

FWIW, this is what a failure will now look like:

../../chrome/browser/sync/test/integration/single_client_bookmarks_sync_test.cc:67:
Failure
Value of: fake_server_verifier.VerifyEntityCountByTypeAndName( 2,
syncer::BOOKMARKS, base::UTF16ToUTF8(it->title))
  Actual: false (Actual count: 1; Expected count: 2; Name: Toronto Maple Leafs

FakeServer contents:
{
   "App List": [ "mgndgikekgjfcpckkfioiadnlibdjbkf",
"ahfgeienlihckogmohjhadlkjgocpleb" ],
   "App Notifications": [  ],
   "App settings": [  ],
   "Apps": [ "Web Store", "Chromium" ],
   "Articles": [  ],
   "Autofill": [  ],
   "Autofill Profiles": [  ],
   "Autofill Wallet": [  ],
   "Bookmarks": [ "Seattle Bubble", "Bank of America", "Wynn", "tier1_a_url1",
"News Wired", "CNN", "ICanHazPorsche?", "tier1_a_url2", "Toronto Maple Leafs",
"tier1_b_url0" ],
   "Device Info": [ "pat.sea.corp.google.com" ],
   "Dictionary": [  ],
   "Encryption keys": [  ],
   "Experiments": [  ],
   "Extension settings": [  ],
   "Extensions": [  ],
   "Favicon Images": [  ],
   "Favicon Tracking": [  ],
   "History Delete Directives": [  ],
   "Managed User Settings": [  ],
   "Managed User Shared Settings": [  ],
   "Managed User Whitelists": [  ],
   "Managed Users": [  ],
   "Passwords": [  ],
   "Preferences": [ "translate_blocked_languages", "profile.name",
"translate_whitelists", "profile.content_settings.pattern_pairs",
"intl.accept_languages", "profile.avatar_index" ],
   "Priority Preferences": [  ],
   "Search Engines": [ "bing.com", "aol.com", "ask.com", "yahoo.com",
"google.com" ],
   "Sessions": [ "pat.sea.corp.google.com" ],
   "Synced Notification App Info": [  ],
   "Synced Notifications": [  ],
   "Tabs": [  ],
   "Themes": [ "Current Theme" ],
   "Typed URLs": [  ],
   "WiFi Credentials": [  ]
}
)
Expected: true

[maniscalco, 2015-03-18 15:33:56]

LGTM modulo two nits

https://codereview.chromium.org/1008983002/diff/1/sync/test/fake_server/fake_...
File sync/test/fake_server/fake_server_verifier.cc (right):

https://codereview.chromium.org/1008983002/diff/1/sync/test/fake_server/fake_...
sync/test/fake_server/fake_server_verifier.cc:14: using base::JSONWriter;
Is the using statement necessary?  I ask because it look like you're using the
JSONWriter:: namespace prefix below.

https://codereview.chromium.org/1008983002/diff/1/sync/test/fake_server/fake_...
sync/test/fake_server/fake_server_verifier.cc:39: string
ConvertFakeServerContentsToString(base::DictionaryValue* entities) {
Does entities need to be a non-const pointer?  Could you pass by const ref
instead?  That way the caller knows there's no way this function will modify
entities.

[pval...(no longer on Chromium), 2015-03-23 23:35:39]

https://codereview.chromium.org/1008983002/diff/1/sync/test/fake_server/fake_...
File sync/test/fake_server/fake_server_verifier.cc (right):

https://codereview.chromium.org/1008983002/diff/1/sync/test/fake_server/fake_...
sync/test/fake_server/fake_server_verifier.cc:14: using base::JSONWriter;
On 2015/03/18 15:33:55, maniscalco wrote:
> Is the using statement necessary?  I ask because it look like you're using the
> JSONWriter:: namespace prefix below.

It's not necessary, but I think the code below reads better (and fits decently
in 80 chars) without the base:: prefix.

https://codereview.chromium.org/1008983002/diff/1/sync/test/fake_server/fake_...
sync/test/fake_server/fake_server_verifier.cc:39: string
ConvertFakeServerContentsToString(base::DictionaryValue* entities) {
On 2015/03/18 15:33:56, maniscalco wrote:
> Does entities need to be a non-const pointer?  Could you pass by const ref
> instead?  That way the caller knows there's no way this function will modify
> entities.

It does not. Thanks for pointing this out; I've made the change and really like
how this looks now.

[maniscalco, 2015-03-24 00:11:19]

LGTM

https://codereview.chromium.org/1008983002/diff/1/sync/test/fake_server/fake_...
File sync/test/fake_server/fake_server_verifier.cc (right):

https://codereview.chromium.org/1008983002/diff/1/sync/test/fake_server/fake_...
sync/test/fake_server/fake_server_verifier.cc:39: string
ConvertFakeServerContentsToString(base::DictionaryValue* entities) {
On 2015/03/23 23:35:39, pvalenzuela wrote:
> On 2015/03/18 15:33:56, maniscalco wrote:
> > Does entities need to be a non-const pointer?  Could you pass by const ref
> > instead?  That way the caller knows there's no way this function will modify
> > entities.
> 
> It does not. Thanks for pointing this out; I've made the change and really
like
> how this looks now.

Great.

[pval...(no longer on Chromium), 2015-03-24 00:19:32]

The CQ bit was checked by pvalenzuela@chromium.org

[commit-bot: I haz the power, 2015-03-24 00:20:06]

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

[commit-bot: I haz the power, 2015-03-24 01:04:19]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-24 01:05:22]

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