Allow sync integration tests to operate on multiple datatypes: Apps, Extensions, Themes

Allow sync integration tests to operate on multiple datatypes: Apps, Extensions, Themes

The sync integration tests currently use a class hierarchy where the test classes for each datatype are subclasses of LiveSyncTest. While this design worked in the past, it allows tests to work with only one datatype at a time, and therefore doesn't allow us to test the interplay between datatypes.

This patch is another in the series of patches that will move away from an inheritance model to one where test cases can operate on more than one datatype. It updates the Apps, Extensions and Themes datatypes to the new model, and contains the following changes:

- LiveAppsSyncTest is no longer a class that inherits from LiveSyncTest, but is now a namespace called apps_helper, and contains a bunch of methods that perform various operations related to Apps. Similar changes were made to Extensions and Themes.
- SingleClientLiveAppsSyncTest is renamed to SingleClientAppsSyncTest, and is a subclass of LiveSyncTest. It uses the methods in namespace apps_helper by including its header file and attaching itself to the helper. Similar changes were made to Extensions and Themes.
- LiveSyncExtensionHelper is renamed to SyncExtensionHelper.

BUG=88510
TEST=sync_integration_tests, sync_performance_tests

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=96292

[Raghu Simha, 2011-08-09 04:23:06]

Fred, please review. Thanks.

[akalin, 2011-08-10 03:19:06]

http://codereview.chromium.org/7599019/diff/7001/chrome/test/live_sync/themes...
File chrome/test/live_sync/themes_helper.cc (right):

http://codereview.chromium.org/7599019/diff/7001/chrome/test/live_sync/themes...
chrome/test/live_sync/themes_helper.cc:34: sync_extension_helper = new
SyncExtensionHelper();
I'm not a big fan of introducing singletons. :| Do you really need the helper
methods to be free functions?  Can you have a ThemesHelper object that sets
up/tears down in constructor?

At least, you should use at base/memory/singleton.h.

[Raghu Simha, 2011-08-10 23:57:35]

Fred, please take another look. Thanks.

http://codereview.chromium.org/7599019/diff/7001/chrome/test/live_sync/themes...
File chrome/test/live_sync/themes_helper.cc (right):

http://codereview.chromium.org/7599019/diff/7001/chrome/test/live_sync/themes...
chrome/test/live_sync/themes_helper.cc:34: sync_extension_helper = new
SyncExtensionHelper();
On 2011/08/10 03:19:06, akalin wrote:
> I'm not a big fan of introducing singletons. :| Do you really need the helper
> methods to be free functions?  Can you have a ThemesHelper object that sets
> up/tears down in constructor?
Fair question. We originally considered maintaining helper object instances like
you suggest, but decided to go with the current namespace design because of how
much more verbose test cases would become. This was discussed in
http://codereview.chromium.org/7461109 and
http://codereview.chromium.org/7259005.

> 
> At least, you should use at base/memory/singleton.h.

Done.

[akalin, 2011-08-11 00:08:08]

LGTM

http://codereview.chromium.org/7599019/diff/11031/chrome/test/live_sync/sync_...
File chrome/test/live_sync/sync_extension_helper.cc (left):

http://codereview.chromium.org/7599019/diff/11031/chrome/test/live_sync/sync_...
chrome/test/live_sync/sync_extension_helper.cc:92: const std::string& name) {
indent

http://codereview.chromium.org/7599019/diff/11031/chrome/test/live_sync/sync_...
chrome/test/live_sync/sync_extension_helper.cc:97: const std::string& name) {
indent

[Raghu Simha, 2011-08-11 00:30:49]

Thanks for the review.

http://codereview.chromium.org/7599019/diff/11031/chrome/test/live_sync/sync_...
File chrome/test/live_sync/sync_extension_helper.cc (left):

http://codereview.chromium.org/7599019/diff/11031/chrome/test/live_sync/sync_...
chrome/test/live_sync/sync_extension_helper.cc:92: const std::string& name) {
On 2011/08/11 00:08:08, akalin wrote:
> indent

Good catch. Done.

http://codereview.chromium.org/7599019/diff/11031/chrome/test/live_sync/sync_...
chrome/test/live_sync/sync_extension_helper.cc:97: const std::string& name) {
On 2011/08/11 00:08:08, akalin wrote:
> indent

Good catch. Done.