Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(24)

Unified Diff: google_apis/gcm/engine/connection_factory_impl_unittest.cc

Issue 2481873002: Added ClientEvent proto and structure for storing events in the factory. (Closed)
Patch Set: Rebase and address code review comments. Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: google_apis/gcm/engine/connection_factory_impl_unittest.cc
diff --git a/google_apis/gcm/engine/connection_factory_impl_unittest.cc b/google_apis/gcm/engine/connection_factory_impl_unittest.cc
index a4cc8d72e868c67da9066471fe62e42e95cc8118..e66e3429d48f5ec09a57c115d71405cccc079a88 100644
--- a/google_apis/gcm/engine/connection_factory_impl_unittest.cc
+++ b/google_apis/gcm/engine/connection_factory_impl_unittest.cc
@@ -90,7 +90,7 @@ class TestConnectionFactoryImpl : public ConnectionFactoryImpl {
void InitializeFactory();
// Overridden stubs.
- void ConnectImpl() override;
+ void StartConnection() override;
void InitHandler() override;
std::unique_ptr<net::BackoffEntry> CreateBackoffEntry(
const net::BackoffEntry::Policy* const policy) override;
@@ -113,6 +113,14 @@ class TestConnectionFactoryImpl : public ConnectionFactoryImpl {
// Simulate a socket error.
void SetSocketError();
+ // Get the client events recorded by the event tracker.
+ const google::protobuf::RepeatedPtrField<mcs_proto::ClientEvent>
+ GetClientEventsForTest() {
Peter Beverloo 2016/12/05 16:29:35 nit: I'd drop "ForTest" since you're in a test. Up
harkness 2016/12/06 12:53:35 Done.
+ mcs_proto::LoginRequest login_request;
+ GetEventTrackerForTesting()->WriteToLoginRequest(&login_request);
+ return login_request.client_event();
+ }
+
base::SimpleTestTickClock* tick_clock() { return &tick_clock_; }
private:
@@ -163,7 +171,7 @@ TestConnectionFactoryImpl::~TestConnectionFactoryImpl() {
EXPECT_EQ(0, num_expected_attempts_);
}
-void TestConnectionFactoryImpl::ConnectImpl() {
+void TestConnectionFactoryImpl::StartConnection() {
ASSERT_GT(num_expected_attempts_, 0);
ASSERT_FALSE(GetConnectionHandler()->CanSendMessage());
std::unique_ptr<mcs_proto::LoginRequest> request(BuildLoginRequest(0, 0, ""));
@@ -368,11 +376,24 @@ TEST_F(ConnectionFactoryImplTest, MultipleFailuresThenSucceed) {
EXPECT_GE((retry_time - connect_time).InMilliseconds(),
CalculateBackoff(kNumAttempts));
+ // There should be one failed client event for each failed connection.
+ const auto& client_events = factory()->GetClientEventsForTest();
Peter Beverloo 2016/12/05 16:29:35 nit (and on line 395, 603 and 617): GetClientEvent
harkness 2016/12/06 12:53:35 Done.
+ ASSERT_EQ(kNumAttempts, client_events.size());
+
+ for (const auto client_event : client_events) {
Peter Beverloo 2016/12/05 16:29:35 nit (and on line 606): No need to copy the individ
harkness 2016/12/06 12:53:35 Done.
+ EXPECT_EQ(mcs_proto::ClientEvent::FAILED_CONNECTION, client_event.type());
+ EXPECT_EQ(net::ERR_CONNECTION_FAILED, client_event.error_code());
+ }
+
factory()->SetConnectResult(net::OK);
WaitForConnections();
EXPECT_TRUE(factory()->NextRetryAttempt().is_null());
EXPECT_TRUE(factory()->IsEndpointReachable());
EXPECT_TRUE(connected_server().is_valid());
+
+ // Old client events should have been reset after the successful connection.
+ const auto& new_client_events = factory()->GetClientEventsForTest();
+ ASSERT_EQ(0, new_client_events.size());
}
// Network change events should trigger canary connections.
@@ -569,4 +590,32 @@ TEST_F(ConnectionFactoryImplTest, ConnectionResetRace) {
EXPECT_TRUE(factory()->IsEndpointReachable());
}
+TEST_F(ConnectionFactoryImplTest, MultipleFailuresWrapClientEvents) {
+ const int kNumAttempts = 50;
+ factory()->SetMultipleConnectResults(net::ERR_CONNECTION_FAILED,
+ kNumAttempts);
+
+ factory()->Connect();
+ WaitForConnections();
+
+ // There should be one failed client event for each failed connection, but
+ // there is a maximum cap of kMaxClientEvents, which is 30.
+ const auto& client_events = factory()->GetClientEventsForTest();
+ ASSERT_EQ(30, client_events.size());
+
+ for (const auto client_event : client_events) {
+ EXPECT_EQ(mcs_proto::ClientEvent::FAILED_CONNECTION, client_event.type());
+ EXPECT_EQ(net::ERR_CONNECTION_FAILED, client_event.error_code());
+ }
+
+ factory()->SetConnectResult(net::OK);
+ WaitForConnections();
+ EXPECT_TRUE(factory()->IsEndpointReachable());
+ EXPECT_TRUE(connected_server().is_valid());
+
+ // Old client events should have been reset after the successful connection.
+ const auto& new_client_events = factory()->GetClientEventsForTest();
+ ASSERT_EQ(0, new_client_events.size());
+}
+
} // namespace gcm

Powered by Google App Engine
This is Rietveld 408576698