Add Statistics for SupervisedUserSettingService during merging and syncing data.

chrome/browser/supervised_user/legacy/supervised_user_shared_settings_service_unittest.cc

Index: chrome/browser/supervised_user/legacy/supervised_user_shared_settings_service_unittest.cc
diff --git a/chrome/browser/supervised_user/legacy/supervised_user_shared_settings_service_unittest.cc b/chrome/browser/supervised_user/legacy/supervised_user_shared_settings_service_unittest.cc
index e7b0675550bc3431854fd4d9eb927d68564d4b38..ea11fbb2ff907b3f628058db6c9dca13776a897a 100644
--- a/chrome/browser/supervised_user/legacy/supervised_user_shared_settings_service_unittest.cc
+++ b/chrome/browser/supervised_user/legacy/supervised_user_shared_settings_service_unittest.cc
@@ -87,7 +87,7 @@ class SupervisedUserSharedSettingsServiceTest : public ::testing::Test {
       : settings_service_(profile_.GetPrefs()) {}
   ~SupervisedUserSharedSettingsServiceTest() override {}
 
-  void StartSyncing(const syncer::SyncDataList& initial_sync_data) {
+  SyncMergeResult StartSyncing(const syncer::SyncDataList& initial_sync_data) {
     sync_processor_.reset(new syncer::FakeSyncChangeProcessor);
     scoped_ptr<syncer::SyncErrorFactory> error_handler(
         new MockSyncErrorFactory(SUPERVISED_USER_SHARED_SETTINGS));
@@ -97,6 +97,7 @@ class SupervisedUserSharedSettingsServiceTest : public ::testing::Test {
             new SyncChangeProcessorWrapperForTest(sync_processor_.get())),
         std::move(error_handler));
     EXPECT_FALSE(result.error().IsSet());
+    return result;
   }
 
   const base::DictionaryValue* GetAllSettings() {
@@ -190,7 +191,12 @@ TEST_F(SupervisedUserSharedSettingsServiceTest, SetAndGet) {
 
 TEST_F(SupervisedUserSharedSettingsServiceTest, Merge) {
   // Set initial values, then stop syncing so we can restart.
-  StartSyncing(SyncDataList());
+  SyncMergeResult result = StartSyncing(SyncDataList());
+  EXPECT_EQ(0, result.num_items_added());
+  EXPECT_EQ(0, result.num_items_deleted());
+  EXPECT_EQ(0, result.num_items_modified());
+  EXPECT_EQ(0, result.num_items_before_association());
+  EXPECT_EQ(0, result.num_items_after_association());
 
   const char kIdA[] = "aaaaaa";
   const char kIdB[] = "bbbbbb";
@@ -214,7 +220,14 @@ TEST_F(SupervisedUserSharedSettingsServiceTest, Merge) {
       SupervisedUserSharedSettingsService::CreateSyncDataForSetting(
           kIdC, "baz", blurp, true));
 
-  StartSyncing(sync_data);
+  result = StartSyncing(sync_data);
+
+  EXPECT_EQ(2, result.num_items_added());
+  EXPECT_EQ(0, result.num_items_deleted());
+  EXPECT_EQ(0, result.num_items_modified());
+  EXPECT_EQ(2, result.num_items_before_association());
+  EXPECT_EQ(3, result.num_items_after_association());

[Bernhard Bauer, 2016/01/06 16:41:50]

Is this correct? I would have expected 3 values before association (lines
207-209), 1 modification (the name for kid A changes from Jack to Jill), and 1
addition ("baz" for kid C), for a total of 4 values after association.

...and after looking at this for a while, I can see why you get these results,
but it's because the logic you've added is indeed wrong (so, good thing we added
unit tests!) You count the number of users before and after association instead
of the total number of settings for all users (see my comment above), and what
you calculate for the number of items added and modified is... something
altogether different.

This also goes to show why writing tests doesn't mean you should just run your
code and then define whatever the output is to be correct. So, I've told you
above what the output should be; now please update the expectations
correspondingly and then fix the code so the test passes.

[Deepak, 2016/01/07 09:52:31]

Done.

+
   EXPECT_EQ(2u, sync_processor_->changes().size());
   VerifySyncChangesAndClear();
   EXPECT_EQ(2u, changed_settings_.size());