Allow sync integration tests to operate on multiple datatypes: Preferences + Bookmarks

chrome/test/live_sync/preferences_helper.h

Index: chrome/test/live_sync/preferences_helper.h
diff --git a/chrome/test/live_sync/preferences_helper.h b/chrome/test/live_sync/preferences_helper.h
new file mode 100644
index 0000000000000000000000000000000000000000..26b5b82dfc28879a687a46bbde54b62c44b9f386
--- /dev/null
+++ b/chrome/test/live_sync/preferences_helper.h
@@ -0,0 +1,126 @@
+// 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.
+
+#ifndef CHROME_TEST_LIVE_SYNC_PREFERENCES_HELPER_H_
+#define CHROME_TEST_LIVE_SYNC_PREFERENCES_HELPER_H_
+#pragma once
+
+#include "base/file_path.h"
+#include "base/values.h"
+
+#include <string>
+
+class PrefService;
+class LiveSyncTest;
+
+class PreferencesHelper {
+ public:
+  PreferencesHelper();
+  ~PreferencesHelper();

[ncarter (slow), 2011/07/06 17:48:52]

I'm doing pros/cons of whether this should be a non-static class with |int
index| captured by the ctor.  I think it could lead to slightly more readable
code.  E.g. the following:

// If |index| is static.
PreferencesHelper alice(0);
PreferencesHelper bob(1);
alice.changeListPref("pref1", "a");
bob.changeListPref("pref2", "b");

// If |index| is dynamic.
for (int i = 0; i < num_clients(); ++i) {
  PreferencesHelper(i).changeListPref(i, "pref1", "value");
}

[C++ is increasingly foreign to me, and the above suggestion may have a tinge of
Java in it.  Evaluate its merits.]

[Raghu Simha, 2011/07/07 02:14:04]

Maybe I'm missing something, but I don't see the benefit of using a separate
Helper object per profile. Test cases already use the profile index in numerous
other places, so this is a logical extension of that design.

The goal I'm working towards is that test cases can use more than one
DatatypeHelper class (for example, MigrationErrorTest can use PreferencesHelper
and BookmarksHelper).

One thing your comment made me realise is that in the model I'm currently using,
tests should never try to create instances of PreferencesHelper, and so its
constructor should probably be private.

[ncarter (slow), 2011/07/07 02:30:34]

The only difference is in readability; it allows you to drop the
"PreferencesHelper::" everywhere.  More personally, I find it easier to follow
code where boilerplate parameters that have to be passed to a method with every
invocation are kept to a minimum.  In particular, I've found it particularly
hard to keep things correct when doing cut/paste/edit of code that uses
hardcoded |int index|.

Having said that, you're the expert here, not me.  You have a valid point about
the precedent, but it also seems that this changelist is about setting a new
precedent, so I think it's worthwhile making readability improvements to the API
if possible.

+
+  // Associates an instance of LiveSyncTest with PreferencesHelper. Must be
+  // called before any of the helper methods below can be used.
+  static void AssociateWithTest(LiveSyncTest* test);
+
+  // Returns |test_| after making sure it is a valid pointer.
+  static LiveSyncTest* test();
+
+  // Used to access the preferences within a particular sync profile.
+  static PrefService* GetPrefs(int index);
+
+  // Used to access the preferences within the verifier sync profile.
+  static PrefService* GetVerifierPrefs();
+
+  // Inverts the value of the boolean preference with name |pref_name| in the
+  // profile with index |index|. Also inverts its value in |verifier| if
+  // DisableVerifier() hasn't been called.
+  static void ChangeBooleanPref(int index, const char* pref_name);
+
+  // Changes the value of the integer preference with name |pref_name| in the
+  // profile with index |index| to |new_value|. Also changes its value in
+  // |verifier| if DisableVerifier() hasn't been called.
+  static void ChangeIntegerPref(int index,
+                                const char* pref_name,
+                                int new_value);
+
+  // Changes the value of the double preference with name |pref_name| in the
+  // profile with index |index| to |new_value|. Also changes its value in
+  // |verifier| if DisableVerifier() hasn't been called.
+  static void ChangeDoublePref(int index,
+                               const char* pref_name,
+                               double new_value);
+
+  // Changes the value of the string preference with name |pref_name| in the
+  // profile with index |index| to |new_value|. Also changes its value in
+  // |verifier| if DisableVerifier() hasn't been called.
+  static void ChangeStringPref(int index,
+                               const char* pref_name,
+                               const std::string& new_value);
+
+  // Modifies the value of the string preference with name |pref_name| in the
+  // profile with index |index| by appending |append_value| to its current
+  // value. Also changes its value in |verifier| if DisableVerifier() hasn't
+  // been called.
+  static void AppendStringPref(int index,
+                               const char* pref_name,
+                               const std::string& append_value);
+
+  // Changes the value of the file path preference with name |pref_name| in the
+  // profile with index |index| to |new_value|. Also changes its value in
+  // |verifier| if DisableVerifier() hasn't been called.
+  static void ChangeFilePathPref(int index,
+                                 const char* pref_name,
+                                 const FilePath& new_value);
+
+  // Changes the value of the list preference with name |pref_name| in the
+  // profile with index |index| to |new_value|. Also changes its value in
+  // |verifier| if DisableVerifier() hasn't been called.
+  static void ChangeListPref(int index,
+                             const char* pref_name,
+                             const ListValue& new_value);
+
+  // Used to verify that the boolean preference with name |pref_name| has the
+  // same value across all profiles. Also checks |verifier| if DisableVerifier()
+  // hasn't been called.
+  static bool BooleanPrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
+
+  // Used to verify that the integer preference with name |pref_name| has the
+  // same value across all profiles. Also checks |verifier| if DisableVerifier()
+  // hasn't been called.
+  static bool IntegerPrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
+
+  // Used to verify that the double preference with name |pref_name| has the
+  // same value across all profiles. Also checks |verifier| if DisableVerifier()
+  // hasn't been called.
+  static bool DoublePrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
+
+  // Used to verify that the string preference with name |pref_name| has the
+  // same value across all profiles. Also checks |verifier| if DisableVerifier()
+  // hasn't been called.
+  static bool StringPrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
+
+  // Used to verify that the file path preference with name |pref_name| has the
+  // same value across all profiles. Also checks |verifier| if DisableVerifier()
+  // hasn't been called.
+  static bool FilePathPrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
+
+  // Used to verify that the list preference with name |pref_name| has the
+  // same value across all profiles. Also checks |verifier| if DisableVerifier()
+  // hasn't been called.
+  static bool ListPrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
+
+  // Encrypts the Preferences datatype.
+  static bool EnableEncryption(int index);
+
+  // Checks if Preferences are encrypted.
+  static bool IsEncrypted(int index);
+
+ private:
+  // The LiveSyncTest instance associated with this helper object.
+  static LiveSyncTest* test_;
+
+  DISALLOW_COPY_AND_ASSIGN(PreferencesHelper);
+};
+
+#endif  // CHROME_TEST_LIVE_SYNC_PREFERENCES_HELPER_H_