Skip to content

Commit

Permalink
sync: Prepare for integration test refactoring
Browse files Browse the repository at this point in the history
Adds an optional virtual method to sync tests to control whether or not
self-notifications should be enabled for a specific test.  Defaults to
on.

Introduces the multi-client status change checker.  This can be used to
wait for a specific condition while waiting listening to change events
emitted by several different ProfileSyncServices.

Adds some helper functions to SyncTest that can be used to fetch the
active ProfileSyncServices.  This functions are helpful for initializing
StatusChangeCheckers.

These changes should have no effect on existing tests.  They will be
used to implement a new style of integration test that does not rely on
sync-cycle concepts like "quiescence" or the self-notifications that
those concepts depend on.

BUG=97780,95742

Review URL: https://codereview.chromium.org/215583002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@260616 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rlarocque@chromium.org committed Mar 31, 2014
1 parent bf3f459 commit 1fdd4f4
Show file tree
Hide file tree
Showing 41 changed files with 344 additions and 232 deletions.
5 changes: 3 additions & 2 deletions chrome/browser/invalidation/p2p_invalidation_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ namespace invalidation {

P2PInvalidationService::P2PInvalidationService(
Profile* profile,
scoped_ptr<InvalidationAuthProvider> auth_provider)
scoped_ptr<InvalidationAuthProvider> auth_provider,
syncer::P2PNotificationTarget notification_target)
: auth_provider_(auth_provider.Pass()) {
notifier::NotifierOptions notifier_options =
ParseNotifierOptions(*CommandLine::ForCurrentProcess());
Expand All @@ -30,7 +31,7 @@ P2PInvalidationService::P2PInvalidationService(
invalidator_.reset(new syncer::P2PInvalidator(
notifier::PushClient::CreateDefault(notifier_options),
invalidator_id_,
syncer::NOTIFY_ALL));
notification_target));
}

P2PInvalidationService::~P2PInvalidationService() {
Expand Down
4 changes: 3 additions & 1 deletion chrome/browser/invalidation/p2p_invalidation_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "base/threading/non_thread_safe.h"
#include "chrome/browser/invalidation/invalidation_service.h"
#include "components/keyed_service/core/keyed_service.h"
#include "sync/notifier/p2p_invalidator.h"

#ifndef CHROME_BROWSER_INVALIDATION_P2P_INVALIDATION_SERVICE_H_
#define CHROME_BROWSER_INVALIDATION_P2P_INVALIDATION_SERVICE_H_
Expand All @@ -28,7 +29,8 @@ class P2PInvalidationService
public InvalidationService {
public:
P2PInvalidationService(Profile* profile,
scoped_ptr<InvalidationAuthProvider> auth_provider);
scoped_ptr<InvalidationAuthProvider> auth_provider,
syncer::P2PNotificationTarget notification_target);
virtual ~P2PInvalidationService();

// Overrides KeyedService method.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
// found in the LICENSE file.

#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/sync/profile_sync_service.h"
#include "chrome/browser/sync/test/integration/bookmarks_helper.h"
#include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
#include "chrome/browser/sync/test/integration/sync_integration_test_util.h"
#include "chrome/browser/sync/test/integration/sync_test.h"

Expand All @@ -28,6 +28,6 @@ class CrossPlatformSyncTest : public SyncTest {
IN_PROC_BROWSER_TEST_F(CrossPlatformSyncTest, DISABLED_AddBookmark) {
ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
ASSERT_TRUE(AddURL(0, L"Google", GURL("http://www.google.co.uk")));
ASSERT_TRUE(AwaitCommitActivityCompletion(GetClient(0)->service()));
ASSERT_TRUE(AwaitCommitActivityCompletion(GetSyncService((0))));
ASSERT_TRUE(ModelMatchesVerifier(0));
}
10 changes: 5 additions & 5 deletions chrome/browser/sync/test/integration/enable_disable_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ IN_PROC_BROWSER_TEST_F(EnableDisableSingleClientTest, EnableOneAtATime) {
DisableNotifications();

const syncer::ModelTypeSet registered_types =
GetClient(0)->service()->GetRegisteredDataTypes();
syncer::UserShare* user_share = GetClient(0)->service()->GetUserShare();
GetSyncService((0))->GetRegisteredDataTypes();
syncer::UserShare* user_share = GetSyncService((0))->GetUserShare();
for (syncer::ModelTypeSet::Iterator it = registered_types.First();
it.Good(); it.Inc()) {
ASSERT_TRUE(GetClient(0)->EnableSyncForDatatype(it.Get()));
Expand Down Expand Up @@ -90,9 +90,9 @@ IN_PROC_BROWSER_TEST_F(EnableDisableSingleClientTest, DisableOneAtATime) {
DisableNotifications();

const syncer::ModelTypeSet registered_types =
GetClient(0)->service()->GetRegisteredDataTypes();
GetSyncService((0))->GetRegisteredDataTypes();

syncer::UserShare* user_share = GetClient(0)->service()->GetUserShare();
syncer::UserShare* user_share = GetSyncService((0))->GetUserShare();

// Make sure all top-level nodes exist first.
for (syncer::ModelTypeSet::Iterator it = registered_types.First();
Expand Down Expand Up @@ -129,7 +129,7 @@ IN_PROC_BROWSER_TEST_F(EnableDisableSingleClientTest, DisableOneAtATime) {
}

syncer::UserShare* user_share =
GetClient(0)->service()->GetUserShare();
GetSyncService((0))->GetUserShare();

ASSERT_FALSE(DoesTopLevelNodeExist(user_share, it.Get()))
<< syncer::ModelTypeToString(it.Get());
Expand Down
8 changes: 4 additions & 4 deletions chrome/browser/sync/test/integration/migration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -182,14 +182,14 @@ class MigrationTest : public SyncTest {
syncer::ModelTypeSet GetPreferredDataTypes() {
// ProfileSyncService must already have been created before we can call
// GetPreferredDataTypes().
DCHECK(GetClient(0)->service());
DCHECK(GetSyncService((0)));
syncer::ModelTypeSet preferred_data_types =
GetClient(0)->service()->GetPreferredDataTypes();
GetSyncService((0))->GetPreferredDataTypes();
preferred_data_types.RemoveAll(syncer::ProxyTypes());
// Make sure all clients have the same preferred data types.
for (int i = 1; i < num_clients(); ++i) {
const syncer::ModelTypeSet other_preferred_data_types =
GetClient(i)->service()->GetPreferredDataTypes();
GetSyncService((i))->GetPreferredDataTypes();
EXPECT_TRUE(preferred_data_types.Equals(other_preferred_data_types));
}
return preferred_data_types;
Expand Down Expand Up @@ -238,7 +238,7 @@ class MigrationTest : public SyncTest {
for (int i = 0; i < num_clients(); ++i) {
MigrationChecker* checker = migration_checkers_[i];
checker->set_expected_types(migrate_types);
checker->Await();
checker->Wait();
ASSERT_FALSE(checker->TimedOut());
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// 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 "chrome/browser/sync/test/integration/multi_client_status_change_checker.h"

#include "base/logging.h"
#include "base/scoped_observer.h"
#include "chrome/browser/sync/profile_sync_service.h"

MultiClientStatusChangeChecker::MultiClientStatusChangeChecker(
std::vector<ProfileSyncService*> services)
: services_(services), timed_out_(false) {}

MultiClientStatusChangeChecker::~MultiClientStatusChangeChecker() {}

base::TimeDelta MultiClientStatusChangeChecker::GetTimeoutDuration() {
return base::TimeDelta::FromSeconds(45);
}
void MultiClientStatusChangeChecker::OnTimeout() {
DVLOG(1) << "Await -> Timed out: " << GetDebugMessage();
timed_out_ = true;
base::MessageLoop::current()->QuitWhenIdle();
}

void MultiClientStatusChangeChecker::Wait() {
DVLOG(1) << "Await: " << GetDebugMessage();

if (IsExitConditionSatisfied()) {
DVLOG(1) << "Await -> Exit before waiting: " << GetDebugMessage();
return;
}

ScopedObserver<ProfileSyncService, MultiClientStatusChangeChecker> obs(this);
for (std::vector<ProfileSyncService*>::iterator it = services_.begin();
it != services_.end(); ++it) {
obs.Add(*it);
}

base::OneShotTimer<MultiClientStatusChangeChecker> timer;
timer.Start(FROM_HERE,
GetTimeoutDuration(),
base::Bind(&MultiClientStatusChangeChecker::OnTimeout,
base::Unretained(this)));

{
base::MessageLoop* loop = base::MessageLoop::current();
base::MessageLoop::ScopedNestableTaskAllower allow(loop);
loop->Run();
}
}

void MultiClientStatusChangeChecker::OnStateChanged() {
DVLOG(1) << "Await -> Checking Condition: " << GetDebugMessage();
if (IsExitConditionSatisfied()) {
DVLOG(1) << "Await -> Condition met: " << GetDebugMessage();
base::MessageLoop::current()->QuitWhenIdle();
}
}

bool MultiClientStatusChangeChecker::TimedOut() {
return timed_out_;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// 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.

#ifndef CHROME_BROWSER_SYNC_TEST_INTEGRATION_MULTI_CLIENT_STATUS_CHANGE_CHECKER_H_
#define CHROME_BROWSER_SYNC_TEST_INTEGRATION_MULTI_CLIENT_STATUS_CHANGE_CHECKER_H_

#include <vector>

#include "base/compiler_specific.h"
#include "base/time/time.h"
#include "chrome/browser/sync/profile_sync_service_observer.h"
#include "chrome/browser/sync/test/integration/status_change_checker.h"

class ProfileSyncService;

// This class provides some common functionality for StatusChangeCheckers that
// observe many ProfileSyncServices. This class is abstract. Its descendants
// are expected to provide additional functionality.
class MultiClientStatusChangeChecker
: public StatusChangeChecker,
public ProfileSyncServiceObserver {
public:
explicit MultiClientStatusChangeChecker(
std::vector<ProfileSyncService*> services);
virtual ~MultiClientStatusChangeChecker();

// Timeout length for this operation. Default is 45s.
virtual base::TimeDelta GetTimeoutDuration();

// Called when waiting times out.
void OnTimeout();

// Blocks until the exit condition is satisfied or a timeout occurs.
void Wait();

// ProfileSyncServiceObserver implementation.
virtual void OnStateChanged() OVERRIDE;

// Returns true if the checker timed out.
bool TimedOut();

// StatusChangeChecker implementations and stubs.
virtual bool IsExitConditionSatisfied() = 0;
virtual std::string GetDebugMessage() const = 0;

protected:
const std::vector<ProfileSyncService*>& services() { return services_; }

private:
std::vector<ProfileSyncService*> services_;
bool timed_out_;
};

#endif // CHROME_BROWSER_SYNC_TEST_INTEGRATION_MULTI_CLIENT_STATUS_CHANGE_CHECKER_H_
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class SyncSetupChecker : public SingleClientStatusChangeChecker {

bool AwaitSyncSetupCompletion(ProfileSyncService* service) {
SyncSetupChecker checker(service);
checker.Await();
checker.Wait();
return !checker.TimedOut();
}

Expand Down Expand Up @@ -172,7 +172,7 @@ bool ProfileSyncServiceHarness::SetupSync(

// Wait for the OnBackendInitialized() callback.
BackendInitializeChecker checker(service());
checker.Await();
checker.Wait();

if (checker.TimedOut()) {
LOG(ERROR) << "OnBackendInitialized() timed out.";
Expand Down Expand Up @@ -266,7 +266,7 @@ bool ProfileSyncServiceHarness::AwaitQuiescence(
services.push_back((*it)->service());
}
QuiesceStatusChangeChecker checker(services);
checker.Await();
checker.Wait();
return !checker.TimedOut();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ base::TimeDelta QuiesceStatusChangeChecker::GetTimeoutDuration() {
return base::TimeDelta::FromSeconds(45);
}

void QuiesceStatusChangeChecker::Await() {
void QuiesceStatusChangeChecker::Wait() {
DVLOG(1) << "Await: " << GetDebugMessage();

if (IsExitConditionSatisfied()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class QuiesceStatusChangeChecker : public StatusChangeChecker {
virtual base::TimeDelta GetTimeoutDuration();

// Blocks until all clients have quiesced or we time out.
void Await();
void Wait();

// A callback function for some helper objects.
void OnServiceStateChanged(ProfileSyncService* service);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

#include "base/basictypes.h"
#include "base/command_line.h"
#include "chrome/browser/sync/profile_sync_service.h"
#include "chrome/browser/sync/test/integration/apps_helper.h"
#include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
#include "chrome/browser/sync/test/integration/sync_app_list_helper.h"
#include "chrome/browser/sync/test/integration/sync_integration_test_util.h"
#include "chrome/browser/sync/test/integration/sync_test.h"
Expand Down Expand Up @@ -67,7 +67,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientAppListSyncTest, AppListSomeApps) {
app_list::AppListSyncableServiceFactory::GetForProfile(verifier());
ASSERT_EQ(kNumApps + kNumDefaultApps, service->GetNumSyncItemsForTest());

ASSERT_TRUE(AwaitCommitActivityCompletion(GetClient(0)->service()));
ASSERT_TRUE(AwaitCommitActivityCompletion(GetSyncService((0))));
ASSERT_TRUE(AllProfilesHaveSameAppListAsVerifier());

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
// found in the LICENSE file.

#include "base/basictypes.h"
#include "chrome/browser/sync/profile_sync_service.h"
#include "chrome/browser/sync/test/integration/apps_helper.h"
#include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
#include "chrome/browser/sync/test/integration/sync_integration_test_util.h"
#include "chrome/browser/sync/test/integration/sync_test.h"

Expand Down Expand Up @@ -66,7 +66,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientAppsSyncTest, InstallSomeLegacyApps) {
InstallApp(verifier(), i);
}

ASSERT_TRUE(AwaitCommitActivityCompletion(GetClient(0)->service()));
ASSERT_TRUE(AwaitCommitActivityCompletion(GetSyncService((0))));

ASSERT_TRUE(AllProfilesHaveSameAppsAsVerifier());
}
Expand All @@ -80,7 +80,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientAppsSyncTest, InstallSomePlatformApps) {
InstallPlatformApp(verifier(), i);
}

ASSERT_TRUE(AwaitCommitActivityCompletion(GetClient(0)->service()));
ASSERT_TRUE(AwaitCommitActivityCompletion(GetSyncService((0))));

ASSERT_TRUE(AllProfilesHaveSameAppsAsVerifier());
}
Expand All @@ -102,7 +102,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientAppsSyncTest, InstallSomeApps) {
InstallPlatformApp(verifier(), i);
}

ASSERT_TRUE(AwaitCommitActivityCompletion(GetClient(0)->service()));
ASSERT_TRUE(AwaitCommitActivityCompletion(GetSyncService((0))));

ASSERT_TRUE(AllProfilesHaveSameAppsAsVerifier());
}
Loading

0 comments on commit 1fdd4f4

Please sign in to comment.