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

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: sync/engine/syncer_unittest.cc

Issue 10038041: sync: Loop committing items without downloading updates (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 8 years, 8 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
« sync/engine/syncer.cc ('K') | « sync/engine/syncer.cc ('k') | sync/sessions/ordered_commit_set.h » ('j') | sync/sessions/ordered_commit_set.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: sync/engine/syncer_unittest.cc
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index 1d8cbb0ea9355f6f7715419bf04edc07032e5786..1865cbaaf450255c61110ac4d8789028cef266be 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -371,13 +371,13 @@ class SyncerTest : public testing::Test,
void DoTruncationTest(const vector<int64>& unsynced_handle_view,
const vector<syncable::Id>& expected_id_order) {
for (size_t limit = expected_id_order.size() + 2; limit > 0; --limit) {
- StatusController* status = session_->mutable_status_controller();
WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
ScopedSetSessionWriteTransaction set_trans(session_.get(), &wtrans);
ModelSafeRoutingInfo routes;
GetModelSafeRoutingInfo(&routes);
- GetCommitIdsCommand command(limit);
+ sessions::OrderedCommitSet output_set(routes);
+ GetCommitIdsCommand command(limit, &output_set);
std::set<int64> ready_unsynced_set;
command.FilterUnreadyEntries(&wtrans, syncable::ModelTypeSet(),
syncable::ModelTypeSet(), false,
@@ -386,22 +386,18 @@ class SyncerTest : public testing::Test,
ready_unsynced_set);
syncable::Directory::UnsyncedMetaHandles ready_unsynced_vector(
ready_unsynced_set.begin(), ready_unsynced_set.end());
- status->set_unsynced_handles(ready_unsynced_vector);
- vector<syncable::Id> output =
- command.ordered_commit_set_->GetAllCommitIds();
size_t truncated_size = std::min(limit, expected_id_order.size());
- ASSERT_EQ(truncated_size, output.size());
+ ASSERT_EQ(truncated_size, output_set.Size());
for (size_t i = 0; i < truncated_size; ++i) {
- ASSERT_EQ(expected_id_order[i], output[i])
+ ASSERT_EQ(expected_id_order[i], output_set.GetCommitIdAt(i))
<< "At index " << i << " with batch size limited to " << limit;
}
sessions::OrderedCommitSet::Projection proj;
- proj = command.ordered_commit_set_->GetCommitIdProjection(GROUP_PASSIVE);
+ proj = output_set.GetCommitIdProjection(GROUP_PASSIVE);
ASSERT_EQ(truncated_size, proj.size());
for (size_t i = 0; i < truncated_size; ++i) {
SCOPED_TRACE(::testing::Message("Projection mismatch with i = ") << i);
- syncable::Id projected =
- command.ordered_commit_set_->GetCommitIdAt(proj[i]);
+ syncable::Id projected = output_set.GetCommitIdAt(proj[i]);
ASSERT_EQ(expected_id_order[proj[i]], projected);
// Since this projection is the identity, the following holds.
ASSERT_EQ(expected_id_order[i], projected);
@@ -710,9 +706,6 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) {
}
SyncShareAsDelegate();
{
- // We remove any unready entries from the status controller's unsynced
- // handles, so this should remain 0 even though the entries didn't commit.
- EXPECT_EQ(0U, session_->status_controller().unsynced_handles().size());
// Nothing should have commited due to bookmarks being encrypted and
// the cryptographer having pending keys. A would have been resolved
// as a simple conflict, but still be unsynced until the next sync cycle.
@@ -727,8 +720,6 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) {
}
SyncShareAsDelegate();
{
- // 2 unsynced handles to reflect the items that committed succesfully.
- EXPECT_EQ(2U, session_->status_controller().unsynced_handles().size());
// All properly encrypted and non-conflicting items should commit. "A" was
// conflicting, but last sync cycle resolved it as simple conflict, so on
// this sync cycle it committed succesfullly.
@@ -756,9 +747,9 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) {
}
SyncShareAsDelegate();
{
- // We attempted to commit two items.
- EXPECT_EQ(2U, session_->status_controller().unsynced_handles().size());
- EXPECT_TRUE(session_->status_controller().did_commit_items());
+ const StatusController& status_controller = session_->status_controller();
+ // Expect success.
+ EXPECT_EQ(status_controller.last_post_commit_result(), SYNCER_OK);
// None should be unsynced anymore.
ReadTransaction rtrans(FROM_HERE, directory());
VERIFY_ENTRY(1, false, false, false, 0, 21, 21, ids_, &rtrans);
@@ -807,7 +798,6 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
mock_server_->AddUpdateSpecifics(4, 0, "D", 10, 10, false, 0, pref);
SyncShareAsDelegate();
{
- EXPECT_EQ(0U, session_->status_controller().unsynced_handles().size());
// Initial state. Everything is normal.
ReadTransaction rtrans(FROM_HERE, directory());
VERIFY_ENTRY(1, false, false, false, 0, 10, 10, ids_, &rtrans);
@@ -828,7 +818,6 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
encrypted_pref);
SyncShareAsDelegate();
{
- EXPECT_EQ(0U, session_->status_controller().unsynced_handles().size());
// All should be unapplied due to being undecryptable and have a valid
// BASE_SERVER_SPECIFICS.
ReadTransaction rtrans(FROM_HERE, directory());
@@ -849,7 +838,6 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
encrypted_pref);
SyncShareAsDelegate();
{
- EXPECT_EQ(0U, session_->status_controller().unsynced_handles().size());
// Items 1, 2, and 4 should have newer server versions, 3 remains the same.
// All should remain unapplied due to be undecryptable.
ReadTransaction rtrans(FROM_HERE, directory());
@@ -868,7 +856,6 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
encrypted_bookmark);
SyncShareAsDelegate();
{
- EXPECT_EQ(0U, session_->status_controller().unsynced_handles().size());
// Items 2 and 4 should be the only ones with BASE_SERVER_SPECIFICS set.
// Items 1 is now unencrypted, so should have applied normally.
ReadTransaction rtrans(FROM_HERE, directory());
@@ -904,7 +891,6 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
}
SyncShareAsDelegate();
{
- EXPECT_EQ(0U, session_->status_controller().unsynced_handles().size());
// Item 1 remains unsynced due to there being pending keys.
// Items 2, 3, 4 should remain unsynced since they were not up to date.
ReadTransaction rtrans(FROM_HERE, directory());
@@ -921,25 +907,22 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
}
// First cycle resolves conflicts, second cycle commits changes.
SyncShareAsDelegate();
- EXPECT_EQ(2, session_->status_controller().syncer_status().
- num_server_overwrites);
- EXPECT_EQ(1, session_->status_controller().syncer_status().
- num_local_overwrites);
- // We attempted to commit item 1.
- EXPECT_EQ(1U, session_->status_controller().unsynced_handles().size());
- EXPECT_TRUE(session_->status_controller().did_commit_items());
+ {
+ const StatusController& status_controller = session_->status_controller();
+ EXPECT_EQ(2, status_controller.syncer_status().num_server_overwrites);
+ EXPECT_EQ(1, status_controller.syncer_status().num_local_overwrites);
+ EXPECT_EQ(status_controller.last_post_commit_result(), SYNCER_OK);
+ }
+
SyncShareAsDelegate();
{
// Everything should be resolved now. The local changes should have
// overwritten the server changes for 2 and 4, while the server changes
// overwrote the local for entry 3.
- // We attempted to commit two handles.
- EXPECT_EQ(0, session_->status_controller().syncer_status().
- num_server_overwrites);
- EXPECT_EQ(0, session_->status_controller().syncer_status().
- num_local_overwrites);
- EXPECT_EQ(2U, session_->status_controller().unsynced_handles().size());
- EXPECT_TRUE(session_->status_controller().did_commit_items());
+ const StatusController& status_controller = session_->status_controller();
+ EXPECT_EQ(0, status_controller.syncer_status().num_server_overwrites);
+ EXPECT_EQ(0, status_controller.syncer_status().num_local_overwrites);
+ EXPECT_EQ(status_controller.last_post_commit_result(), SYNCER_OK);
ReadTransaction rtrans(FROM_HERE, directory());
VERIFY_ENTRY(1, false, false, false, 0, 41, 41, ids_, &rtrans);
VERIFY_ENTRY(2, false, false, false, 1, 31, 31, ids_, &rtrans);
@@ -1173,9 +1156,7 @@ TEST_F(SyncerTest, TestGetUnsyncedAndSimpleCommit) {
WriteTestDataToEntry(&wtrans, &child);
}
- const StatusController& status = session_->status_controller();
syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END);
- EXPECT_EQ(2u, status.unsynced_handles().size());
ASSERT_EQ(2u, mock_server_->committed_ids().size());
// If this test starts failing, be aware other sort orders could be valid.
EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]);
@@ -1218,9 +1199,7 @@ TEST_F(SyncerTest, TestPurgeWhileUnsynced) {
directory()->PurgeEntriesWithTypeIn(
syncable::ModelTypeSet(syncable::PREFERENCES));
- const StatusController& status = session_->status_controller();
syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END);
- EXPECT_EQ(2U, status.unsynced_handles().size());
ASSERT_EQ(2U, mock_server_->committed_ids().size());
// If this test starts failing, be aware other sort orders could be valid.
EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]);
@@ -1459,7 +1438,6 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNesting) {
}
syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END);
- EXPECT_EQ(6u, session_->status_controller().unsynced_handles().size());
ASSERT_EQ(6u, mock_server_->committed_ids().size());
// This test will NOT unroll deletes because SERVER_PARENT_ID is not set.
// It will treat these like moves.
@@ -1527,7 +1505,6 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNewItems) {
}
syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END);
- EXPECT_EQ(6u, session_->status_controller().unsynced_handles().size());
ASSERT_EQ(6u, mock_server_->committed_ids().size());
// If this test starts failing, be aware other sort orders could be valid.
EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]);
@@ -1566,7 +1543,6 @@ TEST_F(SyncerTest, TestCommitListOrderingCounterexample) {
}
syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END);
- EXPECT_EQ(3u, session_->status_controller().unsynced_handles().size());
ASSERT_EQ(3u, mock_server_->committed_ids().size());
// If this test starts failing, be aware other sort orders could be valid.
EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]);
@@ -1612,7 +1588,6 @@ TEST_F(SyncerTest, TestCommitListOrderingAndNewParent) {
}
syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END);
- EXPECT_EQ(3u, session_->status_controller().unsynced_handles().size());
ASSERT_EQ(3u, mock_server_->committed_ids().size());
// If this test starts failing, be aware other sort orders could be valid.
EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]);
@@ -1683,7 +1658,6 @@ TEST_F(SyncerTest, TestCommitListOrderingAndNewParentAndChild) {
}
syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END);
- EXPECT_EQ(3u, session_->status_controller().unsynced_handles().size());
ASSERT_EQ(3u, mock_server_->committed_ids().size());
// If this test starts failing, be aware other sort orders could be valid.
EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]);
@@ -2308,8 +2282,9 @@ TEST_F(EntryCreatedInNewFolderTest, EntryCreatedInNewFolderMidSync) {
mock_server_->SetMidCommitCallback(
base::Bind(&EntryCreatedInNewFolderTest::CreateFolderInBob,
base::Unretained(this)));
- syncer_->SyncShare(session_.get(), BUILD_COMMIT_REQUEST, SYNCER_END);
- EXPECT_EQ(1u, mock_server_->committed_ids().size());
+ syncer_->SyncShare(session_.get(), COMMIT, SYNCER_END);
+ // We loop until no unsynced handles remain, so we will commit both ids.
+ EXPECT_EQ(2u, mock_server_->committed_ids().size());
{
ReadTransaction trans(FROM_HERE, directory());
Entry parent_entry(&trans, syncable::GET_BY_ID,
@@ -2321,7 +2296,7 @@ TEST_F(EntryCreatedInNewFolderTest, EntryCreatedInNewFolderMidSync) {
Entry child(&trans, syncable::GET_BY_ID, child_id);
ASSERT_TRUE(child.good());
EXPECT_EQ(parent_entry.Get(ID), child.Get(PARENT_ID));
-}
+ }
}
TEST_F(SyncerTest, NegativeIDInUpdate) {
@@ -4115,18 +4090,27 @@ TEST_F(SyncerUndeletionTest, UndeleteDuringCommit) {
base::Bind(&SyncerUndeletionTest::Undelete, base::Unretained(this)));
SyncShareAsDelegate();
- // The item ought to exist as an unsynced undeletion (meaning,
- // we think that the next commit ought to be a recreation commit).
+ // We will continue to commit until all nodes are synced, so we expect
+ // that both the delete and following undelete were committed. We haven't
+ // downloaded any updates, though, so the SERVER fields will be the same
+ // as they were at the start of the cycle.
EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
- ExpectUnsyncedUndeletion();
+
+ // Server fields lag behind.
+ EXPECT_FALSE(Get(metahandle_, SERVER_IS_DEL));
+
+ // We have committed the second (undelete) update.
+ EXPECT_FALSE(Get(metahandle_, IS_DEL));
+ EXPECT_FALSE(Get(metahandle_, IS_UNSYNCED));
+ EXPECT_FALSE(Get(metahandle_, IS_UNAPPLIED_UPDATE));
// Now, encounter a GetUpdates corresponding to the deletion from
// the server. The undeletion should prevail again and be committed.
// None of this should trigger any conflict detection -- it is perfectly
// normal to recieve updates from our own commits.
mock_server_->SetMidCommitCallback(base::Closure());
- mock_server_->AddUpdateTombstone(Get(metahandle_, ID));
+ mock_server_->AddUpdateFromLastCommit();
SyncShareAsDelegate();
EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests());
« sync/engine/syncer.cc ('K') | « sync/engine/syncer.cc ('k') | sync/sessions/ordered_commit_set.h » ('j') | sync/sessions/ordered_commit_set.h » ('J')

Powered by Google App Engine
This is Rietveld 408576698