Add test that checks function UpdateServerFieldsFromUpdate is working

sync/engine/syncer_util_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 "sync/engine/syncer_util.h"
 
 #include "base/rand_util.h"
 #include "sync/internal_api/public/base/unique_position.h"
+#include "sync/internal_api/public/test/test_entry_factory.h"
 #include "sync/protocol/sync.pb.h"
+#include "sync/syncable/mutable_entry.h"
+#include "sync/syncable/syncable_write_transaction.h"
+#include "sync/test/engine/test_directory_setter_upper.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace syncer {
 
 class GetUpdatePositionTest : public ::testing::Test {
  public:
+  virtual void SetUp() {
+    dir_maker_.SetUp();
+    entry_factory_.reset(new TestEntryFactory(directory()));
+  }
+  virtual void TearDown() {
+    dir_maker_.TearDown();
+  }
+
+  syncable::Directory* directory() {
+    return dir_maker_.directory();
+  }
+
+  TestEntryFactory* entry_factory() {
+    return entry_factory_.get();
+  }
+
   GetUpdatePositionTest() {
     InitUpdate();
 
     // Init test_position to some valid position value, but don't assign
     // it to the update just yet.
     std::string pos_suffix = UniquePosition::RandomSuffix();
     test_position = UniquePosition::InitialPosition(pos_suffix);
   }
 
   void InitUpdate() {
@@ skipping 16 matching lines @@
   void InitProtoPosition() {
     test_position.ToProto(update.mutable_unique_position());
   }
 
   void InitInt64Position(int64 pos_value) {
     update.set_position_in_parent(pos_value);
   }
 
   sync_pb::SyncEntity update;
   UniquePosition test_position;
+  base::MessageLoop message_loop_;
+  TestDirectorySetterUpper dir_maker_;
+  scoped_ptr<TestEntryFactory> entry_factory_;
 };
 
 // Generate a suffix from originator client GUID and client-assigned ID.  These
 // values should always be present in updates sent down to the client, and
 // combine to create a globally unique value.
 TEST_F(GetUpdatePositionTest, SuffixFromUpdate) {
   InitSuffixIngredients();
 
   // Expect suffix is valid and consistent.
   std::string suffix1 = GetUniqueBookmarkTagFromUpdate(update);
@@ skipping 55 matching lines @@
 
 TEST_F(GetUpdatePositionTest, FromNothing) {
   // Init none of the ingredients necessary to make a position.
   // Verify we still generate a valid position locally.
 
   std::string suffix = GetUniqueBookmarkTagFromUpdate(update);
   UniquePosition pos = GetUpdatePosition(update, suffix);
   EXPECT_TRUE(pos.IsValid());
 }
 
+namespace {
+
+sync_pb::EntitySpecifics DefaultBookmarkSpecifics() {
+  sync_pb::EntitySpecifics result;
+  AddDefaultFieldValue(BOOKMARKS, &result);
+  return result;
+}
+
+}  // namespace
+
+TEST_F(GetUpdatePositionTest, UpdateServerFieldsFromUpdateTest) {

[Nicolas Zea, 2014/09/03 20:58:52]

Comment what the purpose of this test is, here and below. It seems like these
tests are mainly just to verify we don't crash? Especially in the second test,
where the suffix is actually going to wind up being random.

[reat, 2014/09/04 04:32:20]

Yes, exactly so.
The commit https://codereview.chromium.org/283143002 fix crashes with invalid
data from server and add unittest for both GetUniqueBookmarkTagFromUpdate and
GetUpdatePosition functions. But it checks this function separately, and my
approach is to check whole cycle for receiving and updating unique_position from
server

+  // Initialize update with valid data

[Nicolas Zea, 2014/09/03 20:58:52]

nit: periods after comments (here and below)

+  InitSuffixIngredients();
+
+  std::string root_server_id = syncable::GetNullId().GetServerId();
+  int64 handle =
+      entry_factory()->CreateUnappliedNewBookmarkItemWithParent(
+          "I", DefaultBookmarkSpecifics(), root_server_id);
+
+  syncable::WriteTransaction trans(FROM_HERE, syncable::UNITTEST, directory());
+  syncable::MutableEntry target(&trans, syncable::GET_BY_HANDLE, handle);
+
+  // Before update, target has invalid bookmark tag and unique position
+  EXPECT_FALSE(UniquePosition::IsValidSuffix(target.GetUniqueBookmarkTag()));
+  EXPECT_FALSE(target.GetServerUniquePosition().IsValid());
+  UpdateServerFieldsFromUpdate(&target, update, "name");
+  // After update, target has valid bookmark tag and unique position
+  EXPECT_TRUE(UniquePosition::IsValidSuffix(target.GetUniqueBookmarkTag()));
+  EXPECT_TRUE(target.GetServerUniquePosition().IsValid());
+}
+
+TEST_F(GetUpdatePositionTest, UpdateServerFieldsFromInvalidUpdateTest) {
+  // Do not initialize data in update
+  std::string root_server_id = syncable::GetNullId().GetServerId();
+  int64 handle =
+      entry_factory()->CreateUnappliedNewBookmarkItemWithParent(
+          "I", DefaultBookmarkSpecifics(), root_server_id);
+
+  syncable::WriteTransaction trans(FROM_HERE, syncable::UNITTEST, directory());
+  syncable::MutableEntry target(&trans, syncable::GET_BY_HANDLE, handle);
+
+  // Before update, target has invalid bookmark tag and unique position
+  EXPECT_FALSE(UniquePosition::IsValidSuffix(target.GetUniqueBookmarkTag()));
+  EXPECT_FALSE(target.GetServerUniquePosition().IsValid());
+  UpdateServerFieldsFromUpdate(&target, update, "name");
+  // After update, target has valid bookmark tag and unique position
+  EXPECT_TRUE(UniquePosition::IsValidSuffix(target.GetUniqueBookmarkTag()));
+  EXPECT_TRUE(target.GetServerUniquePosition().IsValid());
+}
+
 }  // namespace syncer