Initial RemoteCommandService

components/policy/core/common/cloud/cloud_policy_client_unittest.cc

Index: components/policy/core/common/cloud/cloud_policy_client_unittest.cc
diff --git a/components/policy/core/common/cloud/cloud_policy_client_unittest.cc b/components/policy/core/common/cloud/cloud_policy_client_unittest.cc
index dc9faf0b5d09063131891c5c606e383495d5e8ed..474f40dfd4096da08a305c2be3a73272ae579cf9 100644
--- a/components/policy/core/common/cloud/cloud_policy_client_unittest.cc
+++ b/components/policy/core/common/cloud/cloud_policy_client_unittest.cc
@@ -4,6 +4,8 @@
 
 #include "components/policy/core/common/cloud/cloud_policy_client.h"
 
+#include <stdint.h>
+
 #include <map>
 #include <set>
 
@@ -21,6 +23,7 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using testing::ElementsAre;
 using testing::Mock;
 using testing::Return;
 using testing::SaveArg;
@@ -41,6 +44,11 @@ const char kDMToken[] = "fake-dm-token";
 const char kDeviceCertificate[] = "fake-device-certificate";
 const char kRequisition[] = "fake-requisition";
 const char kStateKey[] = "fake-state-key";
+const char kPayload[] = "input_payload";
+const char kResultPayload[] = "output_payload";
+
+const int64_t kLastCommandId = 123456789;
+const int64_t kTimestamp = 987654321;
 
 MATCHER_P(MatchProto, expected, "matches protobuf") {
   return arg.SerializePartialAsString() == expected.SerializePartialAsString();
@@ -50,11 +58,20 @@ MATCHER_P(MatchProto, expected, "matches protobuf") {
 class MockUploadObserver {
  public:
   MockUploadObserver() {}
-  virtual ~MockUploadObserver() {}
 
   MOCK_METHOD1(OnUploadComplete, void(bool));
 };
 
+// A mock class to allow us to set expectations on remote commands fetch

[bartfab (slow), 2015/02/28 00:32:42]

Nit: s/commands/command/

[binjin, 2015/02/28 02:18:05]

Done.

+// callbacks.
+class MockRemoteCommandsObserver {
+ public:
+  MockRemoteCommandsObserver() {}
+
+  MOCK_METHOD1(OnRemoteCommandsFetched,

[bartfab (slow), 2015/02/28 00:32:42]

You could just add this method to CloudPolicyClientTest instead of defining a
helper class.

[binjin, 2015/02/28 02:18:05]

I actually need to track calling record of this method, and with helper class
it's much simpler. Moving this method into CloudPolicyClientTest doesn't help
setting expectations.

[bartfab (slow), 2015/03/10 16:09:20]

CloudPolicyClientTest can also have mock methods. You do not need a separate
class just to define mock methods.

[binjin, 2015/03/12 11:03:15]

Making this in seperate class will make lifetime management of mock instance
easier. And there are also other class with single mock method existing in same
test, it's not worth moving all of these into CloudPolicyClientTest in my
opinion.

+               void(const std::vector<em::RemoteCommand>&));
+};
+
 }  // namespace
 
 class CloudPolicyClientTest : public testing::Test {
@@ -88,13 +105,32 @@ class CloudPolicyClientTest : public testing::Test {
 
     upload_status_request_.mutable_device_status_report_request();
     upload_status_request_.mutable_session_status_report_request();
+
+    remote_command_request_.mutable_remote_command_request()
+        ->set_last_command_unique_id(kLastCommandId);
+    em::RemoteCommandResult* command_result =
+        remote_command_request_.mutable_remote_command_request()
+            ->add_command_results();
+    command_result->set_unique_id(kLastCommandId);
+    command_result->set_result(
+        em::RemoteCommandResult_ResultType_RESULT_SUCCESS);
+    command_result->set_payload(kResultPayload);
+    command_result->set_timestamp(kTimestamp);
+
+    em::RemoteCommand* command =
+        remote_command_response_.mutable_remote_command_response()
+            ->add_commands();
+    command->set_timestamp(kTimestamp + 1);
+    command->set_payload(kPayload);
+    command->set_unique_id(kLastCommandId + 1);
+    command->set_type(em::RemoteCommand_Type_COMMAND_ECHO_TEST);
   }
 
-  virtual void SetUp() override {
+  void SetUp() override {
     CreateClient(USER_AFFILIATION_NONE);
   }
 
-  virtual void TearDown() override {
+  void TearDown() override {
     client_->RemoveObserver(&observer_);
   }
 
@@ -175,6 +211,17 @@ class CloudPolicyClientTest : public testing::Test {
                          client_id_, MatchProto(upload_status_request_)));
   }
 
+  void ExpectFetchRemoteCommands() {
+    EXPECT_CALL(service_,
+                CreateJob(DeviceManagementRequestJob::TYPE_REMOTE_COMMANDS,
+                          request_context_))
+        .WillOnce(service_.SucceedJob(remote_command_response_));
+    EXPECT_CALL(service_,
+                StartJob(dm_protocol::kValueRequestRemoteCommands,
+                         std::string(), std::string(), kDMToken, std::string(),
+                         client_id_, MatchProto(remote_command_request_)));
+  }
+
   void CheckPolicyResponse() {
     ASSERT_TRUE(client_->GetPolicyFor(policy_type_, std::string()));
     EXPECT_THAT(*client_->GetPolicyFor(policy_type_, std::string()),
@@ -194,6 +241,7 @@ class CloudPolicyClientTest : public testing::Test {
   em::DeviceManagementRequest unregistration_request_;
   em::DeviceManagementRequest upload_certificate_request_;
   em::DeviceManagementRequest upload_status_request_;
+  em::DeviceManagementRequest remote_command_request_;
 
   // Protobufs used in successful responses.
   em::DeviceManagementResponse registration_response_;
@@ -201,6 +249,7 @@ class CloudPolicyClientTest : public testing::Test {
   em::DeviceManagementResponse unregistration_response_;
   em::DeviceManagementResponse upload_certificate_response_;
   em::DeviceManagementResponse upload_status_response_;
+  em::DeviceManagementResponse remote_command_response_;
 
   base::MessageLoop loop_;
   std::string client_id_;
@@ -208,6 +257,7 @@ class CloudPolicyClientTest : public testing::Test {
   MockDeviceManagementService service_;
   StrictMock<MockCloudPolicyClientObserver> observer_;
   StrictMock<MockUploadObserver> upload_observer_;
+  StrictMock<MockRemoteCommandsObserver> remote_commands_observer_;

[bartfab (slow), 2015/02/28 00:32:42]

Nit: If this is used in a single test, there is no need for it to be a member of
the test fixture.

[binjin, 2015/02/28 02:18:05]

Done.

   scoped_ptr<CloudPolicyClient> client_;
   // Pointer to the client's request context.
   scoped_refptr<net::URLRequestContextGetter> request_context_;
@@ -717,4 +767,24 @@ TEST_F(CloudPolicyClientTest, RequestCancelOnUnregister) {
   EXPECT_EQ(0, client_->GetActiveRequestCountForTest());
 }
 
+TEST_F(CloudPolicyClientTest, FetchRemoteCommands) {
+  Register();
+
+  ExpectFetchRemoteCommands();
+  EXPECT_CALL(
+      remote_commands_observer_,
+      OnRemoteCommandsFetched(ElementsAre(MatchProto(
+          remote_command_response_.remote_command_response().commands(0)))))
+      .Times(1);
+  const CloudPolicyClient::RemoteCommandCallback callback =
+      base::Bind(&MockRemoteCommandsObserver::OnRemoteCommandsFetched,
+                 base::Unretained(&remote_commands_observer_));
+
+  const std::vector<enterprise_management::RemoteCommandResult> command_results(

[bartfab (slow), 2015/02/28 00:32:42]

Nit: s/enterprise_management/em/

[binjin, 2015/02/28 02:18:04]

Done.

+      1, remote_command_request_.remote_command_request().command_results(0));

[bartfab (slow), 2015/02/28 00:32:42]

This is a clever way to initialize the vector. I think everyone else would have
created an empty vector first and then push_back()ed an entry.

[binjin, 2015/02/28 02:18:05]

Acknowledged.

+  client_->FetchRemoteCommands(kLastCommandId, command_results, callback);
+
+  EXPECT_EQ(DM_STATUS_SUCCESS, client_->status());
+}
+
 }  // namespace policy