Consolidate BraveSyncServiceTest

components/brave_sync/brave_profile_sync_service.cc

@@ -350,9 +350,9 @@ void BraveProfileSyncService::OnSetSyncBookmarks(const bool sync_bookmarks) {
     type_set.Put(syncer::UserSelectableType::kBookmarks);
   else
     type_set.Remove(syncer::UserSelectableType::kBookmarks);
-  ProfileSyncService::GetUserSettings()->SetSelectedTypes(false,
-                                                                type_set);
-  brave_sync_prefs_->SetSyncBookmarksEnabled(sync_bookmarks);
+  ProfileSyncService::GetUserSettings()->SetSelectedTypes(false, type_set);
+  if (brave_sync_prefs_->GetSyncBookmarksEnabled() != sync_bookmarks)
+    brave_sync_prefs_->SetSyncBookmarksEnabled(sync_bookmarks);
 }
 
 void BraveProfileSyncService::OnSetSyncBrowsingHistory(

components/brave_sync/brave_profile_sync_service.h

@@ -25,7 +25,7 @@ FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnGetInitData);
 FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnSaveBookmarksBaseOrder);
 FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnSyncPrefsChanged);
 FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnSyncDebug);
-FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnSyncReadyAlreadyWithSync);
+FORWARD_DECLARE_TEST(BraveSyncServiceTest, StartSyncNonDeviceRecords);
 FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnSyncReadyNewToSync);
 FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnGetExistingObjects);
 
@@ -113,7 +113,7 @@ class BraveProfileSyncService : public syncer::ProfileSyncService,
   FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnGetInitData);
   FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnSyncPrefsChanged);
   FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnSyncDebug);
-  FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnSyncReadyAlreadyWithSync);
+  FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, StartSyncNonDeviceRecords);
   FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnSyncReadyNewToSync);
   FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnGetExistingObjects);
   friend class ::BraveSyncServiceTest;

components/brave_sync/brave_sync_service_unittest.cc

@@ -24,6 +24,7 @@
 #include "components/bookmarks/browser/bookmark_utils.h"
 #include "components/bookmarks/test/test_bookmark_client.h"
 #include "components/prefs/pref_service.h"
+#include "components/sync/base/pref_names.h"
 #include "content/public/browser/network_service_instance.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "net/base/network_interfaces.h"
@@ -146,6 +147,8 @@ class BraveSyncServiceTest : public testing::Test {
     profile_ = brave_sync::CreateBraveSyncProfile(temp_dir_.GetPath());
     EXPECT_TRUE(profile_.get() != NULL);
 
+    sync_prefs_ = std::make_unique<syncer::SyncPrefs>(profile_->GetPrefs());
+
     // TODO(bridiver) - this is temporary until some changes are made to
     // to bookmark_change_processor to allow `set_for_testing` like
     // BraveSyncClient
@@ -176,6 +179,8 @@ class BraveSyncServiceTest : public testing::Test {
   void TearDown() override {
     sync_service_->BraveSyncService::RemoveObserver(observer_.get());
     // this will also trigger a shutdown of the brave sync service
+    sync_service_->Shutdown();
+    sync_prefs_.reset();
     profile_.reset();
   }
 
@@ -185,7 +190,11 @@ class BraveSyncServiceTest : public testing::Test {
   BookmarkModel* model() { return model_; }
   MockBraveSyncServiceObserver* observer() { return observer_.get(); }
   brave_sync::prefs::Prefs* brave_sync_prefs() {
-    return sync_service_->brave_sync_prefs_.get(); }
+    return sync_service_->brave_sync_prefs_.get();
+  }
+  syncer::SyncPrefs* sync_prefs() {
+    return sync_prefs_.get();
+  }
 
  private:
   // Need this as a very first member to run tests in UI thread
@@ -197,6 +206,7 @@ class BraveSyncServiceTest : public testing::Test {
   BraveProfileSyncService* sync_service_;
   MockBraveSyncClient* sync_client_;
   BookmarkModel* model_;  // Not owns
+  std::unique_ptr<syncer::SyncPrefs> sync_prefs_;
   std::unique_ptr<MockBraveSyncServiceObserver> observer_;
 
   base::ScopedTempDir temp_dir_;
@@ -548,19 +558,29 @@ TEST_F(BraveSyncServiceTest, OnResetSync) {
 
   EXPECT_FALSE(sync_service()->IsBraveSyncInitialized());
   EXPECT_FALSE(sync_service()->IsBraveSyncConfigured());
+
+  EXPECT_FALSE(sync_prefs()->IsSyncRequested());
+  EXPECT_FALSE(profile()->GetPrefs()
+               ->GetBoolean(syncer::prefs::kSyncBookmarks));
 }
 
 TEST_F(BraveSyncServiceTest, OnSetSyncBookmarks) {
   EXPECT_FALSE(profile()->GetPrefs()->GetBoolean(
        brave_sync::prefs::kSyncBookmarksEnabled));
   EXPECT_CALL(*observer(), OnSyncStateChanged).Times(1);
   sync_service()->OnSetSyncBookmarks(true);
+  EXPECT_TRUE(profile()->GetPrefs()
+              ->GetBoolean(syncer::prefs::kSyncBookmarks));
   EXPECT_TRUE(profile()->GetPrefs()->GetBoolean(
        brave_sync::prefs::kSyncBookmarksEnabled));
   EXPECT_CALL(*observer(), OnSyncStateChanged).Times(1);
   sync_service()->OnSetSyncBookmarks(false);
   EXPECT_FALSE(profile()->GetPrefs()->GetBoolean(
       brave_sync::prefs::kSyncBookmarksEnabled));
+  EXPECT_FALSE(profile()->GetPrefs()
+              ->GetBoolean(syncer::prefs::kSyncBookmarks));
+  EXPECT_CALL(*observer(), OnSyncStateChanged).Times(0);
+  sync_service()->OnSetSyncBookmarks(false);
 }
 
 TEST_F(BraveSyncServiceTest, OnSetSyncBrowsingHistory) {
@@ -609,6 +629,7 @@ TEST_F(BraveSyncServiceTest, OnSaveBookmarksBaseOrder) {
   order.clear();
   model()->other_node()->GetMetaInfo("order", &order);
   EXPECT_EQ(order, "1.1.2");
+  EXPECT_EQ(brave_sync_prefs()->GetMigratedBookmarksVersion(), 2);
 }
 
 TEST_F(BraveSyncServiceTest, OnBraveSyncPrefsChanged) {
@@ -617,51 +638,39 @@ TEST_F(BraveSyncServiceTest, OnBraveSyncPrefsChanged) {
   sync_service()->OnBraveSyncPrefsChanged(brave_sync::prefs::kSyncEnabled);
 }
 
-TEST_F(BraveSyncServiceTest, DISABLED_OnSyncReadyAlreadyWithSync) {
+TEST_F(BraveSyncServiceTest, StartSyncNonDeviceRecords) {
   EXPECT_FALSE(sync_service()->IsBraveSyncInitialized());
   sync_service()->OnSetSyncEnabled(true);
   profile()->GetPrefs()->SetString(
                            brave_sync::prefs::kSyncBookmarksBaseOrder, "1.1.");
-  // OnBraveSyncPrefsChanged => OnSyncStateChanged for
-  // kSyncSiteSettingsEnabled (1)  and kSyncDeviceList (2)
-  EXPECT_CALL(*observer(), OnSyncStateChanged).Times(2);
   profile()->GetPrefs()->SetBoolean(
-                            brave_sync::prefs::kSyncSiteSettingsEnabled, true);
-  profile()->GetPrefs()->SetTime(
-                     brave_sync::prefs::kSyncLastFetchTime, base::Time::Now());
-  const char* devices_json = R"(
-    {
-       "devices":[
-          {
-             "device_id":"0",
-             "last_active":1552993896717.0,
-             "name":"Device1",
-             "object_id":"186, 247, 230, 75, 57, 111, 76, 166, 51, 142, 217, 221, 219, 237, 229, 235"
-          },
-          {
-             "device_id":"1",
-             "last_active":1552993909257.0,
-             "name":"Device2",
-             "object_id":"36, 138, 200, 221, 191, 81, 214, 65, 134, 48, 55, 119, 162, 93, 33, 226"
-          }
-       ]
-    }
-  )";
-
-  profile()->GetPrefs()->SetString(
-                    brave_sync::prefs::kSyncDeviceList, devices_json);
-  EXPECT_CALL(*sync_client(), SendFetchSyncRecords).Times(1);
-  EXPECT_CALL(*sync_client(), SendFetchSyncDevices).Times(1);
-  sync_service()->OnSyncReady();
-  EXPECT_TRUE(sync_service()->IsBraveSyncInitialized());
+                            brave_sync::prefs::kSyncBookmarksEnabled, true);
+  EXPECT_FALSE(profile()->GetPrefs()->GetBoolean(syncer::prefs::kSyncBookmarks));
+  brave_sync_prefs()->SetThisDeviceId("1");
+  RecordsList records;
+  records.push_back(SimpleDeviceRecord(
+      SyncRecord::Action::A_CREATE,
+      "1", "device1"));
+  records.push_back(SimpleDeviceRecord(
+      SyncRecord::Action::A_CREATE,
+      "2", "device2"));
+  EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1);
+  sync_service()->OnResolvedPreferences(records);
+  EXPECT_TRUE(profile()->GetPrefs()->GetBoolean(syncer::prefs::kSyncBookmarks));
 }
 
 TEST_F(BraveSyncServiceTest, OnSyncReadyNewToSync) {
   EXPECT_CALL(*observer(), OnSyncStateChanged);
-  profile()->GetPrefs()->SetBoolean(
-                            brave_sync::prefs::kSyncSiteSettingsEnabled, true);
   EXPECT_CALL(*sync_client(), SendGetBookmarksBaseOrder).Times(1);
   sync_service()->OnSyncReady();
+
+  // simulate OnSaveBookmarksBaseOrder
+  profile()->GetPrefs()->SetString(
+                           brave_sync::prefs::kSyncBookmarksBaseOrder, "1.1.");
+  sync_service()->OnSyncReady();
+  EXPECT_TRUE(sync_prefs()->IsSyncRequested());
+  EXPECT_FALSE(profile()->GetPrefs()
+               ->GetBoolean(syncer::prefs::kSyncBookmarks));
 }
 
 TEST_F(BraveSyncServiceTest, OnGetExistingObjects) {
@@ -740,3 +749,20 @@ TEST_F(BraveSyncServiceTest, OnSetupSyncHaveCode_Reset_SetupAgain) {
   EXPECT_TRUE(profile()->GetPrefs()->GetBoolean(
        brave_sync::prefs::kSyncEnabled));
 }
+
+TEST_F(BraveSyncServiceTest, GetPreferredDataTypes) {
+  // Make sure GetPreferredDataTypes contains DEVICE_INFO which is needed for
+  // brave device record polling
+  EXPECT_TRUE(sync_service()->GetPreferredDataTypes().Has(syncer::DEVICE_INFO));
+}
+
+TEST_F(BraveSyncServiceTest, GetDisableReasons) {
+  EXPECT_NE(sync_service()->GetDisableReasons(),
+            syncer::SyncService::DISABLE_REASON_NONE);
+  sync_service()->OnSetSyncEnabled(true);
+  EXPECT_NE(sync_service()->GetDisableReasons(),
+            syncer::SyncService::DISABLE_REASON_NONE);
+  brave_sync_prefs()->SetMigratedBookmarksVersion(2);
+  EXPECT_EQ(sync_service()->GetDisableReasons(),
+            syncer::SyncService::DISABLE_REASON_NONE);
+}