Commit b1d8faa0 authored by pvalenzuela's avatar pvalenzuela Committed by Commit bot

Use Sync FakeServer in exponential backoff tests

For SyncExponentialBackoffTest.OfflineToOnline, methods were added to
simulate and reset a network failure. The old (Python server) way of
doing this has been deleted from SyncTest.

For SyncExponentialBackoffTest.TransientErrorTest, a TRANSIENT_ERROR
is now triggered on FakeServer. The old way of doing this,
SyncTest.TriggerTransientError(), has been deleted.

BUG=406545

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

Cr-Commit-Position: refs/heads/master@{#299034}
parent 0f5b22d8
...@@ -20,9 +20,7 @@ using sync_integration_test_util::AwaitCommitActivityCompletion; ...@@ -20,9 +20,7 @@ using sync_integration_test_util::AwaitCommitActivityCompletion;
class SyncExponentialBackoffTest : public SyncTest { class SyncExponentialBackoffTest : public SyncTest {
public: public:
// TODO(pvalenzuela): Switch to SINGLE_CLIENT once FakeServer SyncExponentialBackoffTest() : SyncTest(SINGLE_CLIENT) {}
// supports this scenario.
SyncExponentialBackoffTest() : SyncTest(SINGLE_CLIENT_LEGACY) {}
virtual ~SyncExponentialBackoffTest() {} virtual ~SyncExponentialBackoffTest() {}
private: private:
...@@ -70,8 +68,7 @@ IN_PROC_BROWSER_TEST_F(SyncExponentialBackoffTest, OfflineToOnline) { ...@@ -70,8 +68,7 @@ IN_PROC_BROWSER_TEST_F(SyncExponentialBackoffTest, OfflineToOnline) {
ASSERT_TRUE(AddFolder(0, 0, "folder1")); ASSERT_TRUE(AddFolder(0, 0, "folder1"));
ASSERT_TRUE(AwaitCommitActivityCompletion(GetSyncService((0)))); ASSERT_TRUE(AwaitCommitActivityCompletion(GetSyncService((0))));
// Trigger a network error at the client side. GetFakeServer()->DisableNetwork();
DisableNetwork(GetProfile(0));
// Add a new item to trigger another sync cycle. // Add a new item to trigger another sync cycle.
ASSERT_TRUE(AddFolder(0, 0, "folder2")); ASSERT_TRUE(AddFolder(0, 0, "folder2"));
...@@ -83,8 +80,7 @@ IN_PROC_BROWSER_TEST_F(SyncExponentialBackoffTest, OfflineToOnline) { ...@@ -83,8 +80,7 @@ IN_PROC_BROWSER_TEST_F(SyncExponentialBackoffTest, OfflineToOnline) {
exponential_backoff_checker.Wait(); exponential_backoff_checker.Wait();
ASSERT_FALSE(exponential_backoff_checker.TimedOut()); ASSERT_FALSE(exponential_backoff_checker.TimedOut());
// Recover from the network error. GetFakeServer()->EnableNetwork();
EnableNetwork(GetProfile(0));
// Verify that sync was able to recover. // Verify that sync was able to recover.
ASSERT_TRUE(AwaitCommitActivityCompletion(GetSyncService((0)))); ASSERT_TRUE(AwaitCommitActivityCompletion(GetSyncService((0))));
...@@ -98,8 +94,7 @@ IN_PROC_BROWSER_TEST_F(SyncExponentialBackoffTest, TransientErrorTest) { ...@@ -98,8 +94,7 @@ IN_PROC_BROWSER_TEST_F(SyncExponentialBackoffTest, TransientErrorTest) {
ASSERT_TRUE(AddFolder(0, 0, "folder1")); ASSERT_TRUE(AddFolder(0, 0, "folder1"));
ASSERT_TRUE(AwaitCommitActivityCompletion(GetSyncService((0)))); ASSERT_TRUE(AwaitCommitActivityCompletion(GetSyncService((0))));
// Trigger a transient error on the server. GetFakeServer()->TriggerError(sync_pb::SyncEnums::TRANSIENT_ERROR);
TriggerTransientError();
// Add a new item to trigger another sync cycle. // Add a new item to trigger another sync cycle.
ASSERT_TRUE(AddFolder(0, 0, "folder2")); ASSERT_TRUE(AddFolder(0, 0, "folder2"));
......
...@@ -66,9 +66,6 @@ ...@@ -66,9 +66,6 @@
#include "net/base/load_flags.h" #include "net/base/load_flags.h"
#include "net/base/network_change_notifier.h" #include "net/base/network_change_notifier.h"
#include "net/cookies/cookie_monster.h" #include "net/cookies/cookie_monster.h"
#include "net/proxy/proxy_config.h"
#include "net/proxy/proxy_config_service_fixed.h"
#include "net/proxy/proxy_service.h"
#include "net/test/spawned_test_server/spawned_test_server.h" #include "net/test/spawned_test_server/spawned_test_server.h"
#include "net/url_request/test_url_fetcher_factory.h" #include "net/url_request/test_url_fetcher_factory.h"
#include "net/url_request/url_fetcher.h" #include "net/url_request/url_fetcher.h"
...@@ -143,17 +140,6 @@ void SetupNetworkCallback( ...@@ -143,17 +140,6 @@ void SetupNetworkCallback(
done->Signal(); done->Signal();
} }
void SetProxyConfigCallback(
base::WaitableEvent* done,
net::URLRequestContextGetter* url_request_context_getter,
const net::ProxyConfig& proxy_config) {
net::ProxyService* proxy_service =
url_request_context_getter->GetURLRequestContext()->proxy_service();
proxy_service->ResetConfigService(
new net::ProxyConfigServiceFixed(proxy_config));
done->Signal();
}
KeyedService* BuildFakeServerProfileInvalidationProvider( KeyedService* BuildFakeServerProfileInvalidationProvider(
content::BrowserContext* context) { content::BrowserContext* context) {
return new invalidation::ProfileInvalidationProvider( return new invalidation::ProfileInvalidationProvider(
...@@ -799,35 +785,6 @@ bool SyncTest::IsTestServerRunning() { ...@@ -799,35 +785,6 @@ bool SyncTest::IsTestServerRunning() {
return delegate.running(); return delegate.running();
} }
void SyncTest::EnableNetwork(Profile* profile) {
// TODO(pvalenzuela): Remove this restriction when FakeServer's observers
// (namely FakeServerInvaldationService) are aware of a network disconnect.
ASSERT_NE(IN_PROCESS_FAKE_SERVER, server_type_)
<< "FakeServer does not support EnableNetwork.";
SetProxyConfig(profile->GetRequestContext(),
net::ProxyConfig::CreateDirect());
if (notifications_enabled_) {
EnableNotificationsImpl();
}
// TODO(rsimha): Remove this line once http://crbug.com/53857 is fixed.
net::NetworkChangeNotifier::NotifyObserversOfIPAddressChangeForTests();
}
void SyncTest::DisableNetwork(Profile* profile) {
// TODO(pvalenzuela): Remove this restriction when FakeServer's observers
// (namely FakeServerInvaldationService) are aware of a network disconnect.
ASSERT_NE(IN_PROCESS_FAKE_SERVER, server_type_)
<< "FakeServer does not support DisableNetwork.";
DisableNotificationsImpl();
// Set the current proxy configuration to a nonexistent proxy to effectively
// disable networking.
net::ProxyConfig config;
config.proxy_rules().ParseFromString("http=127.0.0.1:0");
SetProxyConfig(profile->GetRequestContext(), config);
// TODO(rsimha): Remove this line once http://crbug.com/53857 is fixed.
net::NetworkChangeNotifier::NotifyObserversOfIPAddressChangeForTests();
}
bool SyncTest::TestUsesSelfNotifications() { bool SyncTest::TestUsesSelfNotifications() {
return true; return true;
} }
...@@ -946,16 +903,6 @@ void SyncTest::TriggerMigrationDoneError(syncer::ModelTypeSet model_types) { ...@@ -946,16 +903,6 @@ void SyncTest::TriggerMigrationDoneError(syncer::ModelTypeSet model_types) {
GetTitle())); GetTitle()));
} }
void SyncTest::TriggerTransientError() {
ASSERT_TRUE(ServerSupportsErrorTriggering());
std::string path = "chromiumsync/transienterror";
ui_test_utils::NavigateToURL(browser(), sync_server_.GetURL(path));
ASSERT_EQ("Transient error",
base::UTF16ToASCII(
browser()->tab_strip_model()->GetActiveWebContents()->
GetTitle()));
}
void SyncTest::TriggerXmppAuthError() { void SyncTest::TriggerXmppAuthError() {
ASSERT_TRUE(ServerSupportsErrorTriggering()); ASSERT_TRUE(ServerSupportsErrorTriggering());
std::string path = "chromiumsync/xmppcred"; std::string path = "chromiumsync/xmppcred";
...@@ -1066,16 +1013,6 @@ void SyncTest::SetupNetwork(net::URLRequestContextGetter* context_getter) { ...@@ -1066,16 +1013,6 @@ void SyncTest::SetupNetwork(net::URLRequestContextGetter* context_getter) {
done.Wait(); done.Wait();
} }
void SyncTest::SetProxyConfig(net::URLRequestContextGetter* context_getter,
const net::ProxyConfig& proxy_config) {
base::WaitableEvent done(false, false);
BrowserThread::PostTask(
BrowserThread::IO, FROM_HERE,
base::Bind(&SetProxyConfigCallback, &done,
make_scoped_refptr(context_getter), proxy_config));
done.Wait();
}
fake_server::FakeServer* SyncTest::GetFakeServer() const { fake_server::FakeServer* SyncTest::GetFakeServer() const {
return fake_server_.get(); return fake_server_.get();
} }
...@@ -37,7 +37,6 @@ class FakeServerInvalidationService; ...@@ -37,7 +37,6 @@ class FakeServerInvalidationService;
namespace net { namespace net {
class FakeURLFetcherFactory; class FakeURLFetcherFactory;
class ProxyConfig;
class ScopedDefaultHostResolverProc; class ScopedDefaultHostResolverProc;
class URLFetcherImplFactory; class URLFetcherImplFactory;
class URLRequestContextGetter; class URLRequestContextGetter;
...@@ -170,12 +169,6 @@ class SyncTest : public InProcessBrowserTest { ...@@ -170,12 +169,6 @@ class SyncTest : public InProcessBrowserTest {
// Initializes sync clients and profiles if required and syncs each of them. // Initializes sync clients and profiles if required and syncs each of them.
virtual bool SetupSync() WARN_UNUSED_RESULT; virtual bool SetupSync() WARN_UNUSED_RESULT;
// Enable outgoing network connections for the given profile.
virtual void EnableNetwork(Profile* profile);
// Disable outgoing network connections for the given profile.
virtual void DisableNetwork(Profile* profile);
// Sets whether or not the sync clients in this test should respond to // Sets whether or not the sync clients in this test should respond to
// notifications of their own commits. Real sync clients do not do this, but // notifications of their own commits. Real sync clients do not do this, but
// many test assertions require this behavior. // many test assertions require this behavior.
...@@ -228,10 +221,6 @@ class SyncTest : public InProcessBrowserTest { ...@@ -228,10 +221,6 @@ class SyncTest : public InProcessBrowserTest {
// only if ServerSupportsErrorTriggering() returned true. // only if ServerSupportsErrorTriggering() returned true.
void TriggerMigrationDoneError(syncer::ModelTypeSet model_types); void TriggerMigrationDoneError(syncer::ModelTypeSet model_types);
// Triggers a transient error on the server. Note the server will stay in
// this state until shut down.
void TriggerTransientError();
// Triggers an XMPP auth error on the server. Note the server will // Triggers an XMPP auth error on the server. Note the server will
// stay in this state until shut down. // stay in this state until shut down.
void TriggerXmppAuthError(); void TriggerXmppAuthError();
...@@ -329,11 +318,6 @@ class SyncTest : public InProcessBrowserTest { ...@@ -329,11 +318,6 @@ class SyncTest : public InProcessBrowserTest {
// Helper method used to check if the test server is up and running. // Helper method used to check if the test server is up and running.
bool IsTestServerRunning(); bool IsTestServerRunning();
// Used to disable and enable network connectivity by providing and
// clearing an invalid proxy configuration.
void SetProxyConfig(net::URLRequestContextGetter* context,
const net::ProxyConfig& proxy_config);
void SetupNetwork(net::URLRequestContextGetter* context); void SetupNetwork(net::URLRequestContextGetter* context);
// Helper method used to set up fake responses for kClientLoginUrl, // Helper method used to set up fake responses for kClientLoginUrl,
......
...@@ -149,7 +149,8 @@ scoped_ptr<UpdateSieve> UpdateSieve::Create( ...@@ -149,7 +149,8 @@ scoped_ptr<UpdateSieve> UpdateSieve::Create(
FakeServer::FakeServer() : version_(0), FakeServer::FakeServer() : version_(0),
store_birthday_(kDefaultStoreBirthday), store_birthday_(kDefaultStoreBirthday),
authenticated_(true), authenticated_(true),
error_type_(sync_pb::SyncEnums::SUCCESS) { error_type_(sync_pb::SyncEnums::SUCCESS),
network_enabled_(true) {
keystore_keys_.push_back(kDefaultKeystoreKey); keystore_keys_.push_back(kDefaultKeystoreKey);
CHECK(CreateDefaultPermanentItems()); CHECK(CreateDefaultPermanentItems());
} }
...@@ -218,6 +219,11 @@ void FakeServer::SaveEntity(FakeServerEntity* entity) { ...@@ -218,6 +219,11 @@ void FakeServer::SaveEntity(FakeServerEntity* entity) {
void FakeServer::HandleCommand(const string& request, void FakeServer::HandleCommand(const string& request,
const HandleCommandCallback& callback) { const HandleCommandCallback& callback) {
if (!network_enabled_) {
callback.Run(net::ERR_FAILED, net::ERR_FAILED, string());
return;
}
if (!authenticated_) { if (!authenticated_) {
callback.Run(0, net::HTTP_UNAUTHORIZED, string()); callback.Run(0, net::HTTP_UNAUTHORIZED, string());
return; return;
...@@ -539,4 +545,12 @@ void FakeServer::RemoveObserver(Observer* observer) { ...@@ -539,4 +545,12 @@ void FakeServer::RemoveObserver(Observer* observer) {
observers_.RemoveObserver(observer); observers_.RemoveObserver(observer);
} }
void FakeServer::EnableNetwork() {
network_enabled_ = true;
}
void FakeServer::DisableNetwork() {
network_enabled_ = false;
}
} // namespace fake_server } // namespace fake_server
...@@ -94,6 +94,13 @@ class FakeServer { ...@@ -94,6 +94,13 @@ class FakeServer {
// must be called if AddObserver was ever called with |observer|. // must be called if AddObserver was ever called with |observer|.
void RemoveObserver(Observer* observer); void RemoveObserver(Observer* observer);
// Undoes the effects of DisableNetwork.
void EnableNetwork();
// Forces every request to fail in a way that simulates a network failure.
// This can be used to trigger exponential backoff in the client.
void DisableNetwork();
private: private:
typedef std::map<std::string, FakeServerEntity*> EntityMap; typedef std::map<std::string, FakeServerEntity*> EntityMap;
...@@ -168,6 +175,10 @@ class FakeServer { ...@@ -168,6 +175,10 @@ class FakeServer {
// FakeServer's observers. // FakeServer's observers.
ObserverList<Observer, true> observers_; ObserverList<Observer, true> observers_;
// When true, the server operates normally. When false, a failure is returned
// on every request. This is used to simulate a network failure on the client.
bool network_enabled_;
}; };
} // namespace fake_server } // namespace fake_server
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment