Add test that checks function UpdateServerFieldsFromUpdate is working

sync/engine/syncer_util_unittest.cc

Index: sync/engine/syncer_util_unittest.cc
diff --git a/sync/engine/syncer_util_unittest.cc b/sync/engine/syncer_util_unittest.cc
index eec72eaad313746ed07dff7830a6755c61ee35ae..2e8fb43ab8c7a1b7305548eb3a97caf80a52bcd6 100644
--- a/sync/engine/syncer_util_unittest.cc
+++ b/sync/engine/syncer_util_unittest.cc
@@ -6,13 +6,33 @@
 
 #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();
 
@@ -49,6 +69,9 @@ class GetUpdatePositionTest : public ::testing::Test {
 
   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
@@ -124,4 +147,54 @@ TEST_F(GetUpdatePositionTest, FromNothing) {
   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