Add Statistics for SupervisedUserSettingService during merging and syncing data.

chrome/browser/supervised_user/legacy/supervised_user_shared_settings_service_unittest.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/supervised_user/legacy/supervised_user_shared_settings_service.h"
 
 #include <string>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 69 matching lines @@
 
 class SupervisedUserSharedSettingsServiceTest : public ::testing::Test {
  protected:
   typedef base::CallbackList<void(const std::string&, const std::string&)>
       CallbackList;
 
   SupervisedUserSharedSettingsServiceTest()
       : 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));
     SyncMergeResult result = settings_service_.MergeDataAndStartSyncing(
         SUPERVISED_USER_SHARED_SETTINGS, initial_sync_data,
         scoped_ptr<SyncChangeProcessor>(
             new SyncChangeProcessorWrapperForTest(sync_processor_.get())),
         std::move(error_handler));
     EXPECT_FALSE(result.error().IsSet());
+    return result;
   }
 
   const base::DictionaryValue* GetAllSettings() {
     return profile_.GetPrefs()->GetDictionary(
         prefs::kSupervisedUserSharedSettings);
   }
 
   void VerifySyncChangesAndClear() {
     SyncChangeList& changes = sync_processor_->changes();
     for (const SyncChange& sync_change : changes) {
@@ skipping 73 matching lines @@
   EXPECT_EQ(ToJson(&name), ToJson(settings_service_.GetValue(kIdA, "name")));
   EXPECT_EQ(ToJson(&age), ToJson(settings_service_.GetValue(kIdA, "age")));
   EXPECT_EQ(ToJson(&bar), ToJson(settings_service_.GetValue(kIdB, "foo")));
   EXPECT_FALSE(settings_service_.GetValue(kIdA, "foo"));
   EXPECT_FALSE(settings_service_.GetValue(kIdB, "name"));
   EXPECT_FALSE(settings_service_.GetValue(kIdC, "name"));
 }
 
 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";
   const char kIdC[] = "cccccc";
 
   FundamentalValue age(8);
   StringValue bar("bar");
   settings_service_.SetValue(kIdA, "name", StringValue("Jack"));
   settings_service_.SetValue(kIdA, "age", age);
   settings_service_.SetValue(kIdB, "foo", bar);
 
   settings_service_.StopSyncing(SUPERVISED_USER_SHARED_SETTINGS);
 
   StringValue name("Jill");
   StringValue blurp("blurp");
   SyncDataList sync_data;
   sync_data.push_back(
       SupervisedUserSharedSettingsService::CreateSyncDataForSetting(
           kIdA, "name", name, true));
   sync_data.push_back(
       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());
 
   EXPECT_EQ(
       4u,
       settings_service_.GetAllSyncData(SUPERVISED_USER_SHARED_SETTINGS).size());
   EXPECT_EQ(ToJson(&name),
             ToJson(settings_service_.GetValue(kIdA, "name")));
   EXPECT_EQ(ToJson(&age), ToJson(settings_service_.GetValue(kIdA, "age")));
@@ skipping 39 matching lines @@
       settings_service_.GetAllSyncData(SUPERVISED_USER_SHARED_SETTINGS).size());
   EXPECT_EQ(ToJson(&name),
             ToJson(settings_service_.GetValue(kIdA, "name")));
   EXPECT_EQ(ToJson(&age), ToJson(settings_service_.GetValue(kIdA, "age")));
   EXPECT_EQ(ToJson(&bar), ToJson(settings_service_.GetValue(kIdB, "foo")));
   EXPECT_EQ(ToJson(&blurp), ToJson(settings_service_.GetValue(kIdC, "baz")));
   EXPECT_FALSE(settings_service_.GetValue(kIdA, "foo"));
   EXPECT_FALSE(settings_service_.GetValue(kIdB, "name"));
   EXPECT_FALSE(settings_service_.GetValue(kIdC, "name"));
 }