sync: Introduce control data types

chrome/browser/sync/glue/data_type_manager_impl_unittest.cc

Index: chrome/browser/sync/glue/data_type_manager_impl_unittest.cc
diff --git a/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc b/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc
index 2044e5ef3109fee604e8e52dc0ba1dd07fafbfce..0cf92023af50dc153d7429a310ba14ae409044e2 100644
--- a/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc
+++ b/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc
@@ -33,11 +33,6 @@ using testing::_;
 using testing::Mock;
 using testing::ResultOf;
 
-enum NigoriState {
-  WITH_NIGORI,
-  WITHOUT_NIGORI
-};
-
 // Fake BackendDataTypeConfigurer implementation that simply stores away the
 // callback passed into ConfigureDataTypes.
 class FakeBackendDataTypeConfigurer : public BackendDataTypeConfigurer {
@@ -73,8 +68,7 @@ DataTypeManager::ConfigureStatus GetStatus(
 
 // The actual test harness class, parametrized on nigori state (i.e., tests are
 // run both configuring with nigori, and configuring without).
-class SyncDataTypeManagerImplTest
-    : public testing::TestWithParam<NigoriState> {
+class SyncDataTypeManagerImplTest : public testing::Test {

[tim (not reviewing), 2012/08/22 22:42:28]

I think this removes all coverage for the Purge flow... it'd be useful to have
some tests that call ConfigureImpl without nigori in the set of datatypes,
possibly calls the real Purge (not the mock, which is what BackendMigratorTest
uses).

[rlarocque, 2012/08/23 20:11:23]

I've added a few tests.

You're right that function should at least be invoked at least once during the
tests.

However, from a whitebox perspective, there's not a lot there to test.  The
function is three lines long.  I wrote only a few tests, rather than trying to
revive all the WITHOUT_NIGORI test variants.

  public:
   SyncDataTypeManagerImplTest()
       : ui_thread_(content::BrowserThread::UI, &ui_loop_) {}
@@ -92,11 +86,6 @@ class SyncDataTypeManagerImplTest
                    content::NotificationService::AllSources());
   }
 
-  // A clearer name for the param accessor.
-  NigoriState GetNigoriState() {
-    return GetParam();
-  }
-
   void SetConfigureStartExpectation() {
     EXPECT_CALL(
         observer_,
@@ -114,13 +103,7 @@ class SyncDataTypeManagerImplTest
   // Configure the given DTM with the given desired types.
   void Configure(DataTypeManagerImpl* dtm,
                  const DataTypeManager::TypeSet& desired_types) {
-    const syncer::ConfigureReason kReason =
-        syncer::CONFIGURE_REASON_RECONFIGURATION;
-    if (GetNigoriState() == WITH_NIGORI) {
-      dtm->Configure(desired_types, kReason);
-    } else {
-      dtm->ConfigureWithoutNigori(desired_types, kReason);
-    }
+    dtm->Configure(desired_types, syncer::CONFIGURE_REASON_RECONFIGURATION);
   }
 
   // Finish downloading for the given DTM.  Should be done only after
@@ -161,7 +144,7 @@ class SyncDataTypeManagerImplTest
 
 // Set up a DTM with no controllers, configure it, finish downloading,
 // and then stop it.
-TEST_P(SyncDataTypeManagerImplTest, NoControllers) {
+TEST_F(SyncDataTypeManagerImplTest, NoControllers) {
   DataTypeManagerImpl dtm(&configurer_, &controllers_);
   SetConfigureStartExpectation();
   SetConfigureDoneExpectation(DataTypeManager::OK);
@@ -178,7 +161,7 @@ TEST_P(SyncDataTypeManagerImplTest, NoControllers) {
 
 // Set up a DTM with a single controller, configure it, finish
 // downloading, finish starting the controller, and then stop the DTM.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureOne) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureOne) {
   AddController(BOOKMARKS);
 
   DataTypeManagerImpl dtm(&configurer_, &controllers_);
@@ -200,7 +183,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureOne) {
 
 // Set up a DTM with 2 controllers. configure it. One of them finishes loading
 // after the  timeout. Make sure eventually all are configured.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureSlowLoadingType) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureSlowLoadingType) {
   AddController(BOOKMARKS);
   AddController(APPS);
 
@@ -247,7 +230,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureSlowLoadingType) {
 // Set up a DTM with a single controller, configure it, but stop it
 // before finishing the download.  It should still be safe to run the
 // download callback even after the DTM is stopped and destroyed.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureOneStopWhileDownloadPending) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureOneStopWhileDownloadPending) {
   AddController(BOOKMARKS);
 
   {
@@ -269,7 +252,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureOneStopWhileDownloadPending) {
 // downloading, but stop the DTM before the controller finishes
 // starting up.  It should still be safe to finish starting up the
 // controller even after the DTM is stopped and destroyed.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureOneStopWhileStartingModel) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureOneStopWhileStartingModel) {
   AddController(BOOKMARKS);
 
   {
@@ -295,7 +278,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureOneStopWhileStartingModel) {
 // the controller finishes starting up.  It should still be safe to
 // finish starting up the controller even after the DTM is stopped and
 // destroyed.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureOneStopWhileAssociating) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureOneStopWhileAssociating) {
   AddController(BOOKMARKS);
   {
     DataTypeManagerImpl dtm(&configurer_, &controllers_);
@@ -324,7 +307,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureOneStopWhileAssociating) {
 //   5) Finish the download for step 4.
 //   6) Finish starting the controller successfully.
 //   7) Stop the DTM.
-TEST_P(SyncDataTypeManagerImplTest, OneWaitingForCrypto) {
+TEST_F(SyncDataTypeManagerImplTest, OneWaitingForCrypto) {
   AddController(PASSWORDS);
 
   DataTypeManagerImpl dtm(&configurer_, &controllers_);
@@ -373,7 +356,7 @@ TEST_P(SyncDataTypeManagerImplTest, OneWaitingForCrypto) {
 //   5) Finish the download for step 4.
 //   6) Finish starting the second controller.
 //   7) Stop the DTM.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureOneThenBoth) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureOneThenBoth) {
   AddController(BOOKMARKS);
   AddController(PREFERENCES);
 
@@ -423,7 +406,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureOneThenBoth) {
 //   5) Finish the download for step 4.
 //   6) Finish starting the second controller.
 //   7) Stop the DTM.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureOneThenSwitch) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureOneThenSwitch) {
   AddController(BOOKMARKS);
   AddController(PREFERENCES);
 
@@ -473,7 +456,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureOneThenSwitch) {
 //   5) Finish the download for step 3.
 //   6) Finish starting the second controller.
 //   7) Stop the DTM.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureWhileOneInFlight) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureWhileOneInFlight) {
   AddController(BOOKMARKS);
   AddController(PREFERENCES);
 
@@ -518,7 +501,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureWhileOneInFlight) {
 // Set up a DTM with one controller.  Then configure, finish
 // downloading, and start the controller with an unrecoverable error.
 // The unrecoverable error should cause the DTM to stop.
-TEST_P(SyncDataTypeManagerImplTest, OneFailingController) {
+TEST_F(SyncDataTypeManagerImplTest, OneFailingController) {
   AddController(BOOKMARKS);
 
   DataTypeManagerImpl dtm(&configurer_, &controllers_);
@@ -544,7 +527,7 @@ TEST_P(SyncDataTypeManagerImplTest, OneFailingController) {
 //   4) Finish starting the second controller with an unrecoverable error.
 //
 // The failure from step 4 should cause the DTM to stop.
-TEST_P(SyncDataTypeManagerImplTest, SecondControllerFails) {
+TEST_F(SyncDataTypeManagerImplTest, SecondControllerFails) {
   AddController(BOOKMARKS);
   AddController(PREFERENCES);
 
@@ -582,7 +565,7 @@ TEST_P(SyncDataTypeManagerImplTest, SecondControllerFails) {
 //
 // TODO(akalin): Check that the data type that failed association is
 // recorded in the CONFIGURE_DONE notification.
-TEST_P(SyncDataTypeManagerImplTest, OneControllerFailsAssociation) {
+TEST_F(SyncDataTypeManagerImplTest, OneControllerFailsAssociation) {
   AddController(BOOKMARKS);
   AddController(PREFERENCES);
 
@@ -620,7 +603,7 @@ TEST_P(SyncDataTypeManagerImplTest, OneControllerFailsAssociation) {
 //   4) Finish the download for step 2.
 //   5) Finish starting both controllers.
 //   6) Stop the DTM.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureWhileDownloadPending) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureWhileDownloadPending) {
   AddController(BOOKMARKS);
   AddController(PREFERENCES);
 
@@ -671,7 +654,7 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureWhileDownloadPending) {
 //
 // The failure from step 3 should be ignored since there's a
 // reconfigure pending from step 2.
-TEST_P(SyncDataTypeManagerImplTest, ConfigureWhileDownloadPendingWithFailure) {
+TEST_F(SyncDataTypeManagerImplTest, ConfigureWhileDownloadPendingWithFailure) {
   AddController(BOOKMARKS);
   AddController(PREFERENCES);
 
@@ -711,12 +694,4 @@ TEST_P(SyncDataTypeManagerImplTest, ConfigureWhileDownloadPendingWithFailure) {
   EXPECT_EQ(DataTypeManager::STOPPED, dtm.state());
 }
 
-INSTANTIATE_TEST_CASE_P(
-    WithoutNigori, SyncDataTypeManagerImplTest,
-    ::testing::Values(WITHOUT_NIGORI));
-
-INSTANTIATE_TEST_CASE_P(
-    WithNigori, SyncDataTypeManagerImplTest,
-    ::testing::Values(WITH_NIGORI));
-
 }  // namespace browser_sync