Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(267)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: sync/engine/sync_scheduler_unittest.cc

Issue 10483015: [Sync] Refactor sync configuration logic. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Use mock sync scheduler instead of DoConfigureSyncer Created 8 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « sync/engine/sync_scheduler.cc ('k') | sync/engine/sync_scheduler_whitebox_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: sync/engine/sync_scheduler_unittest.cc
diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc
index a6d805913454de38dd18337afb019e0707d00a91..7aa30ab550b9bc15968e6deda0e710809e6e1f2e 100644
--- a/sync/engine/sync_scheduler_unittest.cc
+++ b/sync/engine/sync_scheduler_unittest.cc
@@ -12,6 +12,7 @@
#include "sync/engine/syncer.h"
#include "sync/engine/throttled_data_type_tracker.h"
#include "sync/sessions/test_util.h"
+#include "sync/test/callback_counter.h"
#include "sync/test/engine/fake_model_worker.h"
#include "sync/test/engine/mock_connection_manager.h"
#include "sync/test/engine/test_directory_setter_upper.h"
@@ -27,6 +28,7 @@ using testing::DoAll;
using testing::Eq;
using testing::Invoke;
using testing::Mock;
+using testing::Not;
using testing::Return;
using testing::WithArg;
@@ -69,6 +71,14 @@ void PumpLoop() {
RunLoop();
}
+ModelSafeRoutingInfo TypesToRoutingInfo(const ModelTypeSet& types) {
+ ModelSafeRoutingInfo routes;
+ for (ModelTypeSet::Iterator iter = types.First(); iter.Good(); iter.Inc()) {
+ routes[iter.Get()] = GROUP_PASSIVE;
+ }
+ return routes;
+}
+
// Convenient to use in tests wishing to analyze SyncShare calls over time.
static const size_t kMinNumSamples = 5;
class SyncSchedulerTest : public testing::Test {
@@ -152,7 +162,7 @@ class SyncSchedulerTest : public testing::Test {
}
void StartSyncScheduler(SyncScheduler::Mode mode) {
- scheduler()->Start(mode, base::Closure());
+ scheduler()->Start(mode);
}
// This stops the scheduler synchronously.
@@ -291,14 +301,23 @@ TEST_F(SyncSchedulerTest, Config) {
SyncShareRecords records;
const ModelTypeSet model_types(syncable::BOOKMARKS);
- EXPECT_CALL(*syncer(), SyncShare(_,_,_))
+ EXPECT_CALL(*syncer(),
+ SyncShare(_,_,_))
+ .WillOnce(Invoke(sessions::test_util::SimulateSuccess))
.WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
WithArg<0>(RecordSyncShare(&records))));
StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
- scheduler()->ScheduleConfiguration(
- model_types, GetUpdatesCallerInfo::RECONFIGURATION);
+ CallbackCounter counter;
+ ConfigurationParams params(
+ GetUpdatesCallerInfo::RECONFIGURATION,
+ model_types,
+ TypesToRoutingInfo(model_types),
+ ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
+ base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
+ ASSERT_TRUE(scheduler()->ScheduleConfiguration(params));
+ ASSERT_EQ(1, counter.times_called());
ASSERT_EQ(1U, records.snapshots.size());
EXPECT_TRUE(CompareModelTypeSetToModelTypePayloadMap(model_types,
@@ -315,7 +334,9 @@ TEST_F(SyncSchedulerTest, ConfigWithBackingOff) {
SyncShareRecords records;
const ModelTypeSet model_types(syncable::BOOKMARKS);
- EXPECT_CALL(*syncer(), SyncShare(_,_,_))
+ EXPECT_CALL(*syncer(),
+ SyncShare(_,_,_))
+ .WillOnce(Invoke(sessions::test_util::SimulateSuccess))
.WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
WithArg<0>(RecordSyncShare(&records))))
.WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
@@ -324,61 +345,27 @@ TEST_F(SyncSchedulerTest, ConfigWithBackingOff) {
StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
ASSERT_EQ(0U, records.snapshots.size());
- scheduler()->ScheduleConfiguration(
- model_types, GetUpdatesCallerInfo::RECONFIGURATION);
+ CallbackCounter counter;
+ ConfigurationParams params(
+ GetUpdatesCallerInfo::RECONFIGURATION,
+ model_types,
+ TypesToRoutingInfo(model_types),
+ ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
+ base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
+ ASSERT_FALSE(scheduler()->ScheduleConfiguration(params));
+ ASSERT_EQ(0, counter.times_called());
ASSERT_EQ(1U, records.snapshots.size());
RunLoop();
ASSERT_EQ(2U, records.snapshots.size());
+ ASSERT_EQ(1, counter.times_called());
EXPECT_TRUE(CompareModelTypeSetToModelTypePayloadMap(model_types,
records.snapshots[1].source().types));
EXPECT_EQ(GetUpdatesCallerInfo::RECONFIGURATION,
records.snapshots[1].source().updates_source);
}
-// Issue 2 config commands. Second one right after the first has failed
-// and make sure LATEST is executed.
-TEST_F(SyncSchedulerTest, MultipleConfigWithBackingOff) {
- const ModelTypeSet
- model_types1(syncable::BOOKMARKS),
- model_types2(syncable::AUTOFILL);
- UseMockDelayProvider();
- EXPECT_CALL(*delay(), GetDelay(_))
- .WillRepeatedly(Return(TimeDelta::FromMilliseconds(30)));
- SyncShareRecords records;
-
- EXPECT_CALL(*syncer(), SyncShare(_,_,_))
- .WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
- WithArg<0>(RecordSyncShare(&records))))
- .WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
- WithArg<0>(RecordSyncShare(&records))))
- .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
- WithArg<0>(RecordSyncShare(&records))));
-
- StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
-
- ASSERT_EQ(0U, records.snapshots.size());
- scheduler()->ScheduleConfiguration(
- model_types1, GetUpdatesCallerInfo::RECONFIGURATION);
-
- ASSERT_EQ(1U, records.snapshots.size());
- scheduler()->ScheduleConfiguration(
- model_types2, GetUpdatesCallerInfo::RECONFIGURATION);
-
- // A canary job gets posted when we go into exponential backoff.
- RunLoop();
-
- ASSERT_EQ(2U, records.snapshots.size());
- RunLoop();
-
- ASSERT_EQ(3U, records.snapshots.size());
- EXPECT_TRUE(CompareModelTypeSetToModelTypePayloadMap(model_types2,
- records.snapshots[2].source().types));
- EXPECT_EQ(GetUpdatesCallerInfo::RECONFIGURATION,
- records.snapshots[2].source().updates_source);
-}
-
// Issue a nudge when the config has failed. Make sure both the config and
// nudge are executed.
TEST_F(SyncSchedulerTest, NudgeWithConfigWithBackingOff) {
@@ -401,19 +388,28 @@ TEST_F(SyncSchedulerTest, NudgeWithConfigWithBackingOff) {
StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
ASSERT_EQ(0U, records.snapshots.size());
- scheduler()->ScheduleConfiguration(
- model_types, GetUpdatesCallerInfo::RECONFIGURATION);
-
+ CallbackCounter counter;
+ ConfigurationParams params(
+ GetUpdatesCallerInfo::RECONFIGURATION,
+ model_types,
+ TypesToRoutingInfo(model_types),
+ ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
+ base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
+ ASSERT_FALSE(scheduler()->ScheduleConfiguration(params));
+ ASSERT_EQ(0, counter.times_called());
ASSERT_EQ(1U, records.snapshots.size());
+
scheduler()->ScheduleNudgeAsync(
zero(), NUDGE_SOURCE_LOCAL, model_types, FROM_HERE);
RunLoop();
-
ASSERT_EQ(2U, records.snapshots.size());
+ ASSERT_EQ(0, counter.times_called());
+
RunLoop();
+ ASSERT_EQ(3U, records.snapshots.size());
+ ASSERT_EQ(1, counter.times_called());
// Now change the mode so nudge can execute.
- ASSERT_EQ(3U, records.snapshots.size());
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
ASSERT_EQ(4U, records.snapshots.size());
@@ -718,13 +714,19 @@ TEST_F(SyncSchedulerTest, HasMoreToSyncThenFails) {
EXPECT_TRUE(RunAndGetBackoff());
}
-// Test that no syncing occurs when throttled.
+// Test that no syncing occurs when throttled (although CleanupDisabledTypes
+// is allowed).
TEST_F(SyncSchedulerTest, ThrottlingDoesThrottle) {
const ModelTypeSet types(syncable::BOOKMARKS);
TimeDelta poll(TimeDelta::FromMilliseconds(5));
TimeDelta throttle(TimeDelta::FromMinutes(10));
scheduler()->OnReceivedLongPollIntervalUpdate(poll);
- EXPECT_CALL(*syncer(), SyncShare(_,_,_))
+
+ EXPECT_CALL(*syncer(),
+ SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES))
+ .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
+ EXPECT_CALL(*syncer(), SyncShare(_,Not(CLEANUP_DISABLED_TYPES),
+ Not(CLEANUP_DISABLED_TYPES)))
.WillOnce(WithArg<0>(sessions::test_util::SimulateThrottled(throttle)))
.WillRepeatedly(AddFailureAndQuitLoopNow());
@@ -736,8 +738,15 @@ TEST_F(SyncSchedulerTest, ThrottlingDoesThrottle) {
StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
- scheduler()->ScheduleConfiguration(
- types, GetUpdatesCallerInfo::RECONFIGURATION);
+ CallbackCounter counter;
+ ConfigurationParams params(
+ GetUpdatesCallerInfo::RECONFIGURATION,
+ types,
+ TypesToRoutingInfo(types),
+ ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
+ base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
+ ASSERT_FALSE(scheduler()->ScheduleConfiguration(params));
+ ASSERT_EQ(0, counter.times_called());
}
TEST_F(SyncSchedulerTest, ThrottlingExpires) {
@@ -770,6 +779,7 @@ TEST_F(SyncSchedulerTest, ConfigurationMode) {
SyncShareRecords records;
scheduler()->OnReceivedLongPollIntervalUpdate(poll);
EXPECT_CALL(*syncer(), SyncShare(_,_,_))
+ .WillOnce(Invoke(sessions::test_util::SimulateSuccess))
.WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
WithArg<0>(RecordSyncShare(&records))));
@@ -783,8 +793,15 @@ TEST_F(SyncSchedulerTest, ConfigurationMode) {
const ModelTypeSet config_types(syncable::BOOKMARKS);
- scheduler()->ScheduleConfiguration(
- config_types, GetUpdatesCallerInfo::RECONFIGURATION);
+ CallbackCounter counter;
+ ConfigurationParams params(
+ GetUpdatesCallerInfo::RECONFIGURATION,
+ config_types,
+ TypesToRoutingInfo(config_types),
+ ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
+ base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
+ ASSERT_TRUE(scheduler()->ScheduleConfiguration(params));
+ ASSERT_EQ(1, counter.times_called());
ASSERT_EQ(1U, records.snapshots.size());
EXPECT_TRUE(CompareModelTypeSetToModelTypePayloadMap(config_types,
@@ -853,7 +870,7 @@ TEST_F(SyncSchedulerTest, BackoffDropsJobs) {
scheduler()->OnReceivedLongPollIntervalUpdate(poll);
UseMockDelayProvider();
- EXPECT_CALL(*syncer(), SyncShare(_,_,_)).Times(1)
+ EXPECT_CALL(*syncer(), SyncShare(_,_,_))
.WillRepeatedly(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
RecordSyncShareMultiple(&r, 1U)));
EXPECT_CALL(*delay(), GetDelay(_)).
@@ -895,9 +912,15 @@ TEST_F(SyncSchedulerTest, BackoffDropsJobs) {
StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
- scheduler()->CleanupDisabledTypes();
- scheduler()->ScheduleConfiguration(
- types, GetUpdatesCallerInfo::RECONFIGURATION);
+ CallbackCounter counter;
+ ConfigurationParams params(
+ GetUpdatesCallerInfo::RECONFIGURATION,
+ types,
+ TypesToRoutingInfo(types),
+ ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
+ base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
+ ASSERT_FALSE(scheduler()->ScheduleConfiguration(params));
+ ASSERT_EQ(0, counter.times_called());
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
@@ -1066,28 +1089,49 @@ TEST_F(SyncSchedulerTest, SyncerSteps) {
StopSyncScheduler();
Mock::VerifyAndClearExpectations(syncer());
- // Configuration.
+ // Configuration (always includes a cleanup disabled types).
+ EXPECT_CALL(*syncer(),
+ SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES))
+ .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
EXPECT_CALL(*syncer(), SyncShare(_, DOWNLOAD_UPDATES, APPLY_UPDATES))
.WillOnce(Invoke(sessions::test_util::SimulateSuccess));
StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
- scheduler()->ScheduleConfiguration(
- ModelTypeSet(), GetUpdatesCallerInfo::RECONFIGURATION);
-
+ syncable::ModelTypeSet model_types(syncable::BOOKMARKS);
+ CallbackCounter counter;
+ ConfigurationParams params(
+ GetUpdatesCallerInfo::RECONFIGURATION,
+ model_types,
+ TypesToRoutingInfo(model_types),
+ ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
+ base::Bind(&CallbackCounter::Callback, base::Unretained(&counter)));
+ ASSERT_TRUE(scheduler()->ScheduleConfiguration(params));
+ ASSERT_EQ(1, counter.times_called());
+ // Runs directly so no need to pump the loop.
StopSyncScheduler();
Mock::VerifyAndClearExpectations(syncer());
- // Cleanup disabled types.
+ // Cleanup disabled types. Because no types are being configured, we just
+ // perform the cleanup.
EXPECT_CALL(*syncer(),
- SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES))
- .WillOnce(Invoke(sessions::test_util::SimulateSuccess));
- StartSyncScheduler(SyncScheduler::NORMAL_MODE);
-
- scheduler()->CleanupDisabledTypes();
+ SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES)).
+ WillOnce(Invoke(sessions::test_util::SimulateSuccess));
+ StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
+ CallbackCounter counter2;
+ ConfigurationParams params2(
+ GetUpdatesCallerInfo::RECONFIGURATION,
+ ModelTypeSet(),
+ ModelSafeRoutingInfo(),
+ ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
+ base::Bind(&CallbackCounter::Callback, base::Unretained(&counter2)));
+ ASSERT_TRUE(scheduler()->ScheduleConfiguration(params2));
+ ASSERT_EQ(1, counter2.times_called());
StopSyncScheduler();
Mock::VerifyAndClearExpectations(syncer());
+ StartSyncScheduler(SyncScheduler::NORMAL_MODE);
+
// Poll.
EXPECT_CALL(*syncer(), SyncShare(_, SYNCER_BEGIN, SYNCER_END))
.Times(AtLeast(1))
« no previous file with comments | « sync/engine/sync_scheduler.cc ('k') | sync/engine/sync_scheduler_whitebox_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698