GCM Checkin implementation with unit tests and protobufs

google_apis/gcm/engine/gcm_checkin_request_unittest.cc

Index: google_apis/gcm/engine/gcm_checkin_request_unittest.cc
diff --git a/google_apis/gcm/engine/gcm_checkin_request_unittest.cc b/google_apis/gcm/engine/gcm_checkin_request_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..49c6d42847e4ef8a9c5fff2545fc02102537f648
--- /dev/null
+++ b/google_apis/gcm/engine/gcm_checkin_request_unittest.cc
@@ -0,0 +1,179 @@
+// Copyright (c) 2013 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 <string>
+
+#include "base/values.h"

[fgorski, 2014/01/04 00:54:07]

I probably don't need that.

[fgorski, 2014/01/06 20:34:12]

Done.

+#include "google_apis/gcm/engine/gcm_checkin_request.h"
+#include "net/url_request/test_url_fetcher_factory.h"
+#include "net/url_request/url_request_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace gcm {
+
+const int64 kAndroidId = 42;
+const int64 kBlankAndroidId = -1;
+const uint64 kBlankSecurityToken = 999999;
+const char kChromeVersion[] = "Version String";
+const uint64 kSecurityToken = 77;
+const int64 kUserSerialNumber = 7;
+
+#if defined(CHROME_OS)
+const int kExpectedDeviceType = 4;

[jianli, 2014/01/04 01:34:10]

Can we use the enum defined in the proto file?

[fgorski, 2014/01/06 20:34:12]

Done.

+#else
+const int kExpectedDeviceType = 3;
+#endif
+
+class GCMCheckinRequestTest : public testing::Test {
+ public:
+  GCMCheckinRequestTest();
+  virtual ~GCMCheckinRequestTest();
+
+  GCMClient::ChromeCheckInInfo CreateCheckInInfo();
+
+  void FetcherCallback(uint64 android_id, uint64 security_token);
+
+  void SetResponse(net::TestURLFetcher* fetcher,
+                   uint64 android_id,
+                   uint64 security_token);
+
+ protected:
+  int64 android_id_;

[jianli, 2014/01/04 01:34:10]

Should we use uint64?

[fgorski, 2014/01/06 20:34:12]

No, actually I've not found a protobuf that would make that value uint64 instead
of int64. I'll gladly update all of the other usages... Thoughts?

+  uint64 security_token_;
+  int checkin_device_type_;
+  GCMClient::CheckInInfo user_checkin_info_;

[fgorski, 2014/01/04 00:54:07]

I'll remove that.

[fgorski, 2014/01/06 20:34:12]

Done.

+  GCMClient::ChromeCheckInInfo chrome_checkin_info_;
+  base::MessageLoop message_loop_;
+  net::TestURLFetcherFactory factory_;
+  scoped_refptr<net::TestURLRequestContextGetter> getter_;
+};
+
+GCMCheckinRequestTest::GCMCheckinRequestTest()
+    : android_id_(kBlankAndroidId),
+      security_token_(kBlankSecurityToken),
+      checkin_device_type_(0),
+      getter_(new net::TestURLRequestContextGetter(
+          message_loop_.message_loop_proxy())) {}
+
+GCMCheckinRequestTest::~GCMCheckinRequestTest() {}
+
+GCMClient::ChromeCheckInInfo GCMCheckinRequestTest::CreateCheckInInfo() {
+  GCMClient::ChromeCheckInInfo chrome_checkin_info;
+  chrome_checkin_info.platform =
+      checkin_proto::ChromeBuildProto::PLATFORM_LINUX;
+  chrome_checkin_info.channel = checkin_proto::ChromeBuildProto::CHANNEL_CANARY;
+  chrome_checkin_info.chrome_version = kChromeVersion;
+  return chrome_checkin_info;
+}
+
+void GCMCheckinRequestTest::FetcherCallback(uint64 android_id,
+                                            uint64 security_token) {
+  android_id_ = android_id;
+  security_token_ = security_token;
+}
+
+void GCMCheckinRequestTest::SetResponse(net::TestURLFetcher* fetcher,
+                                        uint64 android_id,
+                                        uint64 security_token) {
+  checkin_proto::AndroidCheckinResponse response;
+  response.set_stats_ok(true);
+  response.set_android_id(android_id);
+  response.set_security_token(security_token);
+
+  std::string response_string;
+  response.SerializeToString(&response_string);
+  fetcher->SetResponseString(response_string);
+}
+
+TEST_F(GCMCheckinRequestTest, EmptyResponseBody) {

[jianli, 2014/01/04 01:34:10]

Do we also need to have the test coverage for the following cases:
1) Non-200 HTTP response code, say 401.
2) Corrupted response data.
3) Empty android id or security token.

[jianli, 2014/01/06 21:00:01]

???

[fgorski, 2014/01/06 21:58:03]

Done.
Done.
Tested with both missing values and with values equal to 0, assuming that that
should not happen.

+  GCMClient::ChromeCheckInInfo chrome_checkin_info = CreateCheckInInfo();
+  GCMCheckinRequest request(base::Bind(&GCMCheckinRequestTest::FetcherCallback,
+                                       base::Unretained(this)),
+                            getter_.get(),
+                            chrome_checkin_info,
+                            kUserSerialNumber,
+                            user_checkin_info_);
+  request.Start();

[Nicolas Zea, 2014/01/04 01:40:19]

nit: newline after (to match other tests)

[fgorski, 2014/01/06 20:34:12]

Done.

+  EXPECT_EQ(kBlankAndroidId, android_id_);
+  EXPECT_EQ(kBlankSecurityToken, security_token_);
+  net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0);
+  ASSERT_TRUE(fetcher);
+  fetcher->set_response_code(net::HTTP_OK);
+  fetcher->SetResponseString(std::string());
+  fetcher->delegate()->OnURLFetchComplete(fetcher);
+  EXPECT_EQ(0u, android_id_);
+  EXPECT_EQ(0u, security_token_);
+}
+
+TEST_F(GCMCheckinRequestTest, FetcherData) {
+  GCMClient::ChromeCheckInInfo chrome_checkin_info = CreateCheckInInfo();
+  user_checkin_info_.android_id = kAndroidId;
+  user_checkin_info_.secret = kSecurityToken;
+  GCMCheckinRequest request(base::Bind(&GCMCheckinRequestTest::FetcherCallback,
+                                       base::Unretained(this)),
+                            getter_.get(),
+                            chrome_checkin_info,
+                            kUserSerialNumber,
+                            user_checkin_info_);
+  request.Start();
+
+  net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0);
+  ASSERT_TRUE(fetcher);
+  fetcher->set_response_code(net::HTTP_OK);
+  checkin_proto::AndroidCheckinRequest request_proto;
+  request_proto.ParseFromString(fetcher->upload_data());
+  EXPECT_EQ(kAndroidId, request_proto.id());
+  EXPECT_EQ(kSecurityToken, request_proto.security_token());
+  EXPECT_EQ(kUserSerialNumber, request_proto.user_serial_number());
+  EXPECT_EQ(chrome_checkin_info.platform,
+            request_proto.checkin().chrome_build().platform());
+  EXPECT_EQ(chrome_checkin_info.chrome_version,
+            request_proto.checkin().chrome_build().chrome_version());
+  EXPECT_EQ(chrome_checkin_info.channel,
+            request_proto.checkin().chrome_build().channel());
+  EXPECT_EQ(kExpectedDeviceType, request_proto.checkin().type());
+}
+
+TEST_F(GCMCheckinRequestTest, SuccessfulCheckin) {

[jianli, 2014/01/04 01:34:10]

Is this to test subsequent checkin? If so, better call it as
SuccessfulSubsequentCheckIn.

[fgorski, 2014/01/06 20:34:12]

Done.

[jianli, 2014/01/06 21:00:01]

It is still the old name in your new patch.

+  GCMClient::ChromeCheckInInfo chrome_checkin_info = CreateCheckInInfo();
+  user_checkin_info_.android_id = kAndroidId;
+  user_checkin_info_.secret = kSecurityToken;
+  GCMCheckinRequest request(base::Bind(&GCMCheckinRequestTest::FetcherCallback,
+                                       base::Unretained(this)),
+                            getter_.get(),
+                            chrome_checkin_info,
+                            kUserSerialNumber,
+                            user_checkin_info_);
+  request.Start();

[Nicolas Zea, 2014/01/04 01:40:19]

nit: newline after (to match other tests)

[fgorski, 2014/01/06 20:34:12]

Done.

+  EXPECT_EQ(kBlankAndroidId, android_id_);
+  EXPECT_EQ(kBlankSecurityToken, security_token_);
+  net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0);
+  ASSERT_TRUE(fetcher);
+  fetcher->set_response_code(net::HTTP_OK);
+  SetResponse(fetcher, kAndroidId, kSecurityToken);
+  fetcher->delegate()->OnURLFetchComplete(fetcher);
+  EXPECT_EQ(kAndroidId, android_id_);
+  EXPECT_EQ(kSecurityToken, security_token_);
+}
+
+TEST_F(GCMCheckinRequestTest, SuccessfulFirstTimeCheckin) {
+  GCMClient::ChromeCheckInInfo chrome_checkin_info = CreateCheckInInfo();
+  GCMCheckinRequest request(base::Bind(&GCMCheckinRequestTest::FetcherCallback,
+                                       base::Unretained(this)),
+                            getter_.get(),
+                            chrome_checkin_info,
+                            kUserSerialNumber,
+                            user_checkin_info_);
+  request.Start();
+
+  net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0);
+  ASSERT_TRUE(fetcher);
+  fetcher->set_response_code(net::HTTP_OK);
+  SetResponse(fetcher, kAndroidId, kSecurityToken);
+  fetcher->delegate()->OnURLFetchComplete(fetcher);
+  EXPECT_EQ(kAndroidId, android_id_);
+  EXPECT_EQ(kSecurityToken, security_token_);
+}
+
+}  // namespace gcm