[Sync] Add TestUserShare class meant to be used by syncapi-related unit tests

BUG=
TEST=


Review URL: http://codereview.chromium.org/6759041

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@79902 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 184 insertions, 57 deletions

diff --git a/chrome/browser/sync/engine/syncapi_unittest.cc b/chrome/browser/sync/engine/syncapi_unittest.cc
index 28d8c98..1cc5032 100644
--- a/chrome/browser/sync/engine/syncapi_unittest.cc
+++ b/chrome/browser/sync/engine/syncapi_unittest.cc
@@ -34,7 +34,7 @@
 #include "chrome/browser/sync/syncable/syncable.h"
 #include "chrome/browser/sync/syncable/syncable_id.h"
 #include "chrome/browser/sync/util/cryptographer.h"
-#include "chrome/test/sync/engine/test_directory_setter_upper.h"
+#include "chrome/test/sync/engine/test_user_share.h"
 #include "chrome/test/values_test_util.h"
 #include "content/browser/browser_thread.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -166,34 +166,29 @@ int64 MakeServerNodeForType(UserShare* share,
 class SyncApiTest : public testing::Test {
  public:
   virtual void SetUp() {
-    setter_upper_.SetUp();
-    share_.dir_manager.reset(setter_upper_.manager());
-    share_.name = setter_upper_.name();
+    test_user_share_.SetUp();
   }
 
   virtual void TearDown() {
-    // |share_.dir_manager| does not actually own its value.
-    ignore_result(share_.dir_manager.release());
-    setter_upper_.TearDown();
+    test_user_share_.TearDown();
   }
 
  protected:
-  UserShare share_;
-  browser_sync::TestDirectorySetterUpper setter_upper_;
+  browser_sync::TestUserShare test_user_share_;
 };
 
 TEST_F(SyncApiTest, SanityCheckTest) {
   {
-    ReadTransaction trans(&share_);
+    ReadTransaction trans(test_user_share_.user_share());
     EXPECT_TRUE(trans.GetWrappedTrans() != NULL);
   }
   {
-    WriteTransaction trans(&share_);
+    WriteTransaction trans(test_user_share_.user_share());
     EXPECT_TRUE(trans.GetWrappedTrans() != NULL);
   }
   {
     // No entries but root should exist
-    ReadTransaction trans(&share_);
+    ReadTransaction trans(test_user_share_.user_share());
     ReadNode node(&trans);
     // Metahandle 1 can be root, sanity check 2
     EXPECT_FALSE(node.InitByIdLookup(2));
@@ -202,16 +197,17 @@ TEST_F(SyncApiTest, SanityCheckTest) {
 
 TEST_F(SyncApiTest, BasicTagWrite) {
   {
-    ReadTransaction trans(&share_);
+    ReadTransaction trans(test_user_share_.user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
     EXPECT_EQ(root_node.GetFirstChildId(), 0);
   }
 
-  ignore_result(MakeNode(&share_, syncable::BOOKMARKS, "testtag"));
+  ignore_result(MakeNode(test_user_share_.user_share(),
+                         syncable::BOOKMARKS, "testtag"));
 
   {
-    ReadTransaction trans(&share_);
+    ReadTransaction trans(test_user_share_.user_share());
     ReadNode node(&trans);
     EXPECT_TRUE(node.InitByClientTagLookup(syncable::BOOKMARKS,
         "testtag"));
@@ -241,18 +237,21 @@ TEST_F(SyncApiTest, GenerateSyncableHash) {
 
 TEST_F(SyncApiTest, ModelTypesSiloed) {
   {
-    WriteTransaction trans(&share_);
+    WriteTransaction trans(test_user_share_.user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
     EXPECT_EQ(root_node.GetFirstChildId(), 0);
   }
 
-  ignore_result(MakeNode(&share_, syncable::BOOKMARKS, "collideme"));
-  ignore_result(MakeNode(&share_, syncable::PREFERENCES, "collideme"));
-  ignore_result(MakeNode(&share_, syncable::AUTOFILL, "collideme"));
+  ignore_result(MakeNode(test_user_share_.user_share(),
+                         syncable::BOOKMARKS, "collideme"));
+  ignore_result(MakeNode(test_user_share_.user_share(),
+                         syncable::PREFERENCES, "collideme"));
+  ignore_result(MakeNode(test_user_share_.user_share(),
+                         syncable::AUTOFILL, "collideme"));
 
   {
-    ReadTransaction trans(&share_);
+    ReadTransaction trans(test_user_share_.user_share());
 
     ReadNode bookmarknode(&trans);
     EXPECT_TRUE(bookmarknode.InitByClientTagLookup(syncable::BOOKMARKS,
@@ -274,13 +273,13 @@ TEST_F(SyncApiTest, ModelTypesSiloed) {
 
 TEST_F(SyncApiTest, ReadMissingTagsFails) {
   {
-    ReadTransaction trans(&share_);
+    ReadTransaction trans(test_user_share_.user_share());
     ReadNode node(&trans);
     EXPECT_FALSE(node.InitByClientTagLookup(syncable::BOOKMARKS,
         "testtag"));
   }
   {
-    WriteTransaction trans(&share_);
+    WriteTransaction trans(test_user_share_.user_share());
     WriteNode node(&trans);
     EXPECT_FALSE(node.InitByClientTagLookup(syncable::BOOKMARKS,
         "testtag"));
@@ -295,7 +294,7 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
   std::wstring test_title(L"test1");
 
   {
-    WriteTransaction trans(&share_);
+    WriteTransaction trans(test_user_share_.user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
 
@@ -316,7 +315,7 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
 
   // Ensure we can delete something with a tag.
   {
-    WriteTransaction trans(&share_);
+    WriteTransaction trans(test_user_share_.user_share());
     WriteNode wnode(&trans);
     EXPECT_TRUE(wnode.InitByClientTagLookup(syncable::BOOKMARKS,
         "testtag"));
@@ -329,7 +328,7 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
   // Lookup of a node which was deleted should return failure,
   // but have found some data about the node.
   {
-    ReadTransaction trans(&share_);
+    ReadTransaction trans(test_user_share_.user_share());
     ReadNode node(&trans);
     EXPECT_FALSE(node.InitByClientTagLookup(syncable::BOOKMARKS,
         "testtag"));
@@ -339,7 +338,7 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
   }
 
   {
-    WriteTransaction trans(&share_);
+    WriteTransaction trans(test_user_share_.user_share());
     ReadNode folder_node(&trans);
     EXPECT_TRUE(folder_node.InitByIdLookup(folder_id));
 
@@ -356,7 +355,7 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
 
   // Now look up should work.
   {
-    ReadTransaction trans(&share_);
+    ReadTransaction trans(test_user_share_.user_share());
     ReadNode node(&trans);
     EXPECT_TRUE(node.InitByClientTagLookup(syncable::BOOKMARKS,
         "testtag"));
@@ -368,11 +367,11 @@ TEST_F(SyncApiTest, TestDeleteBehavior) {
 TEST_F(SyncApiTest, WriteAndReadPassword) {
   KeyParams params = {"localhost", "username", "passphrase"};
   {
-    ReadTransaction trans(&share_);
+    ReadTransaction trans(test_user_share_.user_share());
     trans.GetCryptographer()->AddKey(params);
   }
   {
-    WriteTransaction trans(&share_);
+    WriteTransaction trans(test_user_share_.user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
 
@@ -384,7 +383,7 @@ TEST_F(SyncApiTest, WriteAndReadPassword) {
     password_node.SetPasswordSpecifics(data);
   }
   {
-    ReadTransaction trans(&share_);
+    ReadTransaction trans(test_user_share_.user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
 
@@ -441,7 +440,7 @@ void CheckNodeValue(const BaseNode& node, const DictionaryValue& value) {
 }  // namespace
 
 TEST_F(SyncApiTest, BaseNodeToValue) {
-  ReadTransaction trans(&share_);
+  ReadTransaction trans(test_user_share_.user_share());
   ReadNode node(&trans);
   node.InitByRootLookup();
   scoped_ptr<DictionaryValue> value(node.ToValue());
@@ -521,10 +520,11 @@ class MockExtraChangeRecordData
 }  // namespace
 
 TEST_F(SyncApiTest, ChangeRecordToValue) {
-  int64 child_id = MakeNode(&share_, syncable::BOOKMARKS, "testtag");
+  int64 child_id = MakeNode(test_user_share_.user_share(),
+                            syncable::BOOKMARKS, "testtag");
   sync_pb::EntitySpecifics child_specifics;
   {
-    ReadTransaction trans(&share_);
+    ReadTransaction trans(test_user_share_.user_share());
     ReadNode node(&trans);
     EXPECT_TRUE(node.InitByIdLookup(child_id));
     child_specifics = node.GetEntry()->Get(syncable::SPECIFICS);
@@ -532,7 +532,7 @@ TEST_F(SyncApiTest, ChangeRecordToValue) {
 
   // Add
   {
-    ReadTransaction trans(&share_);
+    ReadTransaction trans(test_user_share_.user_share());
     SyncManager::ChangeRecord record;
     record.action = SyncManager::ChangeRecord::ACTION_ADD;
     record.id = 1;
@@ -544,7 +544,7 @@ TEST_F(SyncApiTest, ChangeRecordToValue) {
 
   // Update
   {
-    ReadTransaction trans(&share_);
+    ReadTransaction trans(test_user_share_.user_share());
     SyncManager::ChangeRecord record;
     record.action = SyncManager::ChangeRecord::ACTION_UPDATE;
     record.id = child_id;
@@ -556,7 +556,7 @@ TEST_F(SyncApiTest, ChangeRecordToValue) {
 
   // Delete (no extra)
   {
-    ReadTransaction trans(&share_);
+    ReadTransaction trans(test_user_share_.user_share());
     SyncManager::ChangeRecord record;
     record.action = SyncManager::ChangeRecord::ACTION_DELETE;
     record.id = child_id + 1;
@@ -567,7 +567,7 @@ TEST_F(SyncApiTest, ChangeRecordToValue) {
 
   // Delete (with extra)
   {
-    ReadTransaction trans(&share_);
+    ReadTransaction trans(test_user_share_.user_share());
     SyncManager::ChangeRecord record;
     record.action = SyncManager::ChangeRecord::ACTION_DELETE;
     record.id = child_id + 1;
diff --git a/chrome/browser/sync/js_sync_manager_observer_unittest.cc b/chrome/browser/sync/js_sync_manager_observer_unittest.cc
index f5ed961..b9c0c33 100644
--- a/chrome/browser/sync/js_sync_manager_observer_unittest.cc
+++ b/chrome/browser/sync/js_sync_manager_observer_unittest.cc
@@ -13,7 +13,7 @@
 #include "chrome/browser/sync/js_test_util.h"
 #include "chrome/browser/sync/sessions/session_state.h"
 #include "chrome/browser/sync/syncable/model_type.h"
-#include "chrome/test/sync/engine/test_directory_setter_upper.h"
+#include "chrome/test/sync/engine/test_user_share.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace browser_sync {
@@ -194,11 +194,8 @@ int64 MakeNode(sync_api::UserShare* share, syncable::ModelType model_type) {
 TEST_F(JsSyncManagerObserverTest, OnChangesApplied) {
   InSequence dummy;
 
-  TestDirectorySetterUpper setter_upper;
-  sync_api::UserShare share;
-  setter_upper.SetUp();
-  share.dir_manager.reset(setter_upper.manager());
-  share.name = setter_upper.name();
+  TestUserShare test_user_share;
+  test_user_share.SetUp();
 
   // We don't test with passwords as that requires additional setup.
 
@@ -206,7 +203,8 @@ TEST_F(JsSyncManagerObserverTest, OnChangesApplied) {
   sync_api::SyncManager::ChangeRecord changes[syncable::MODEL_TYPE_COUNT];
   for (int i = syncable::AUTOFILL_PROFILE;
        i < syncable::MODEL_TYPE_COUNT; ++i) {
-    changes[i].id = MakeNode(&share, syncable::ModelTypeFromInt(i));
+    changes[i].id =
+        MakeNode(test_user_share.user_share(), syncable::ModelTypeFromInt(i));
     switch (i % 3) {
       case 0:
         changes[i].action =
@@ -222,7 +220,7 @@ TEST_F(JsSyncManagerObserverTest, OnChangesApplied) {
         break;
     }
     {
-      sync_api::ReadTransaction trans(&share);
+      sync_api::ReadTransaction trans(test_user_share.user_share());
       sync_api::ReadNode node(&trans);
       EXPECT_TRUE(node.InitByIdLookup(changes[i].id));
       changes[i].specifics = node.GetEntry()->Get(syncable::SPECIFICS);
@@ -243,7 +241,7 @@ TEST_F(JsSyncManagerObserverTest, OnChangesApplied) {
     ListValue* expected_changes = new ListValue();
     expected_args.Append(expected_changes);
     for (int j = i; j < syncable::MODEL_TYPE_COUNT; ++j) {
-      sync_api::ReadTransaction trans(&share);
+      sync_api::ReadTransaction trans(test_user_share.user_share());
       expected_changes->Append(changes[j].ToValue(&trans));
     }
     EXPECT_CALL(mock_router_,
@@ -254,15 +252,13 @@ TEST_F(JsSyncManagerObserverTest, OnChangesApplied) {
   // Fire OnChangesApplied() for each data type.
   for (int i = syncable::AUTOFILL_PROFILE;
        i < syncable::MODEL_TYPE_COUNT; ++i) {
-    sync_api::ReadTransaction trans(&share);
+    sync_api::ReadTransaction trans(test_user_share.user_share());
     sync_manager_observer_.OnChangesApplied(syncable::ModelTypeFromInt(i),
                                             &trans, &changes[i],
                                             syncable::MODEL_TYPE_COUNT - i);
   }
 
-  // |share.dir_manager| does not actually own its value.
-  ignore_result(share.dir_manager.release());
-  setter_upper.TearDown();
+  test_user_share.TearDown();
 }
 
 }  // namespace
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 0dfd105..3df2967 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -254,12 +254,38 @@
         '../testing/gtest.gyp:gtest',
         'sync',
       ],
+      'export_dependent_settings': [
+        '../testing/gmock.gyp:gmock',
+        '../testing/gtest.gyp:gtest',
+        'sync',
+      ],
       'include_dirs': [
         '..',
       ],
       'sources': [
         'browser/sync/js_test_util.cc',
         'browser/sync/js_test_util.h',
+        'test/sync/engine/test_directory_setter_upper.cc',
+        'test/sync/engine/test_directory_setter_upper.h',
+      ],
+    },
+    {
+      'target_name': 'test_support_syncapi',
+      'type': '<(library)',
+      'dependencies': [
+        'syncapi',
+        'test_support_sync',
+      ],
+      'export_dependent_settings': [
+        'syncapi',
+        'test_support_sync',
+      ],
+      'include_dirs': [
+        '..',
+      ],
+      'sources': [
+        'test/sync/engine/test_user_share.cc',
+        'test/sync/engine/test_user_share.h',
       ],
     },
     {
@@ -1116,6 +1142,7 @@
         'service',
         'test_support_common',
         'test_support_sync',
+        'test_support_syncapi',
         'test_support_unit',
         'utility',
         '../app/app.gyp:app_base',
@@ -2912,8 +2939,6 @@
         'test/sync/engine/mock_gaia_authenticator.h',
         'test/sync/engine/mock_gaia_authenticator_unittest.cc',
         'test/sync/engine/syncer_command_test.h',
-        'test/sync/engine/test_directory_setter_upper.cc',
-        'test/sync/engine/test_directory_setter_upper.h',
         'test/sync/engine/test_id_factory.h',
         'test/sync/engine/test_syncable_utils.cc',
         'test/sync/engine/test_syncable_utils.h',
@@ -2943,6 +2968,7 @@
         'sync_notifier',
         'test_support_common',
         'test_support_sync',
+        'test_support_syncapi',
         'test_support_sync_notifier',
         'test_support_unit',
       ],
diff --git a/chrome/test/sync/engine/test_directory_setter_upper.cc b/chrome/test/sync/engine/test_directory_setter_upper.cc
index a0e4431..c124d52 100644
--- a/chrome/test/sync/engine/test_directory_setter_upper.cc
+++ b/chrome/test/sync/engine/test_directory_setter_upper.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/test/sync/engine/test_directory_setter_upper.h"
 
+#include "base/compiler_specific.h"
 #include "base/file_util.h"
 #include "base/string_util.h"
 #include "chrome/browser/sync/syncable/directory_manager.h"
@@ -26,8 +27,9 @@ TestDirectorySetterUpper::~TestDirectorySetterUpper() {}
 void TestDirectorySetterUpper::Init() {
   ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
   reset_directory_manager(new DirectoryManager(temp_dir_.path()));
-  file_path_ = manager_->GetSyncDataDatabasePath();
-  file_util::Delete(file_path_, false);
+  // There shouldn't be any existing database in the newly-created
+  // temp dir.
+  ASSERT_FALSE(file_util::PathExists(manager_->GetSyncDataDatabasePath()));
 }
 
 void TestDirectorySetterUpper::reset_directory_manager(DirectoryManager* d) {
@@ -57,8 +59,7 @@ void TestDirectorySetterUpper::TearDown() {
   manager()->FinalSaveChangesForAll();
   manager()->Close(name());
   manager_.reset();
-  EXPECT_TRUE(file_util::Delete(file_path_, false));
-  file_path_ = FilePath(FILE_PATH_LITERAL(""));
+  ASSERT_TRUE(temp_dir_.Delete());
 }
 
 void TestDirectorySetterUpper::RunInvariantCheck(const ScopedDirLookup& dir) {
diff --git a/chrome/test/sync/engine/test_directory_setter_upper.h b/chrome/test/sync/engine/test_directory_setter_upper.h
index 0621301..75506c0 100644
--- a/chrome/test/sync/engine/test_directory_setter_upper.h
+++ b/chrome/test/sync/engine/test_directory_setter_upper.h
@@ -33,6 +33,7 @@
 
 #include <string>
 
+#include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_temp_dir.h"
 #include "chrome/browser/sync/syncable/directory_manager.h"
@@ -74,8 +75,9 @@ class TestDirectorySetterUpper {
 
   scoped_ptr<syncable::DirectoryManager> manager_;
   const std::string name_;
-  FilePath file_path_;
   ScopedTempDir temp_dir_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestDirectorySetterUpper);
 };
 
 // A variant of the above where SetUp does not actually open the directory.
diff --git a/chrome/test/sync/engine/test_user_share.cc b/chrome/test/sync/engine/test_user_share.cc
new file mode 100644
index 0000000..2fdd36f
--- /dev/null
+++ b/chrome/test/sync/engine/test_user_share.cc
@@ -0,0 +1,35 @@
+// Copyright (c) 2011 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 "chrome/test/sync/engine/test_user_share.h"
+
+#include "base/compiler_specific.h"
+
+namespace browser_sync {
+
+TestUserShare::TestUserShare() {}
+
+TestUserShare::~TestUserShare() {
+  CHECK(!user_share_.dir_manager.get());
+}
+
+void TestUserShare::SetUp() {
+  setter_upper_.SetUp();
+  // HACK: We have two scoped_ptrs to the same object.  But we make
+  // sure to release one in TearDown.
+  user_share_.dir_manager.reset(setter_upper_.manager());
+  user_share_.name = setter_upper_.name();
+}
+
+void TestUserShare::TearDown() {
+  // Make sure we don't free the manager twice.
+  ignore_result(user_share_.dir_manager.release());
+  setter_upper_.TearDown();
+}
+
+sync_api::UserShare* TestUserShare::user_share() {
+  return &user_share_;
+}
+
+}  // namespace browser_sync
diff --git a/chrome/test/sync/engine/test_user_share.h b/chrome/test/sync/engine/test_user_share.h
new file mode 100644
index 0000000..00d73d1
--- /dev/null
+++ b/chrome/test/sync/engine/test_user_share.h
@@ -0,0 +1,67 @@
+// Copyright (c) 2011 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.
+//
+// A handy class that takes care of setting up and destroying a
+// sync_api::UserShare instance for unit tests that require one.
+//
+// The expected usage is to make this a component of your test fixture:
+//
+//   class AwesomenessTest : public testing::Test {
+//    public:
+//     virtual void SetUp() {
+//       test_user_share_.SetUp();
+//     }
+//     virtual void TearDown() {
+//       test_user_share_.TearDown();
+//     }
+//    protected:
+//     TestUserShare user_share_;
+//   };
+//
+// Then, in your tests:
+//
+//   TEST_F(AwesomenessTest, IsMaximal) {
+//     sync_api::ReadTransaction trans(test_user_share_.user_share());
+//     ...
+//   }
+//
+
+#ifndef CHROME_TEST_SYNC_ENGINE_TEST_USER_SHARE_H_
+#define CHROME_TEST_SYNC_ENGINE_TEST_USER_SHARE_H_
+#pragma once
+
+#include "base/basictypes.h"
+#include "chrome/browser/sync/engine/syncapi.h"
+#include "chrome/test/sync/engine/test_directory_setter_upper.h"
+
+namespace browser_sync {
+
+class TestUserShare {
+ public:
+  TestUserShare();
+  ~TestUserShare();
+
+  // Sets up the UserShare instance.  Clears any existing database
+  // backing files that might exist on disk.
+  void SetUp();
+
+  // Undo everything done by SetUp(): closes the UserShare and deletes
+  // the backing files. Before closing the directory, this will run
+  // the directory invariant checks and perform the SaveChanges action
+  // on the user share's directory.
+  void TearDown();
+
+  // Non-NULL iff called between a call to SetUp() and TearDown().
+  sync_api::UserShare* user_share();
+
+ private:
+  TestDirectorySetterUpper setter_upper_;
+  sync_api::UserShare user_share_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestUserShare);
+};
+
+}  // namespace browser_sync
+
+#endif  // CHROME_TEST_SYNC_ENGINE_TEST_USER_SHARE_H_