Refactor CryptAuth component to be more testable.

components/proximity_auth/cryptauth/cryptauth_client_unittest.cc

 // 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 "components/proximity_auth/cryptauth/cryptauth_client.h"
-
 #include "base/command_line.h"
 #include "base/test/null_task_runner.h"
 #include "components/proximity_auth/cryptauth/cryptauth_access_token_fetcher.h"
 #include "components/proximity_auth/cryptauth/cryptauth_api_call_flow.h"
-#include "components/proximity_auth/cryptauth/cryptauth_client_factory.h"
+#include "components/proximity_auth/cryptauth/cryptauth_client_impl.h"

[Ilya Sherman, 2015/04/06 23:48:02]

Here, too, this include should be at the top of the file.

[Tim Song, 2015/04/07 02:22:57]

Done.

However, this gave presubmit warnings since the name of the *_unittest.cc class
isn't the same as the *.h class.

[Ilya Sherman, 2015/04/07 22:04:50]

Hmm.  I think it's reasonable to update the presubmit check to allow "foo_impl"
to be tested by "foo_unittest".  WDYT?

 #include "components/proximity_auth/cryptauth/proto/cryptauth_api.pb.h"
 #include "components/proximity_auth/switches.h"
 #include "google_apis/gaia/fake_oauth2_token_service.h"
 #include "net/url_request/test_url_fetcher_factory.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
 
 using testing::_;
 using testing::DoAll;
 using testing::Return;
 using testing::SaveArg;
 using testing::StrictMock;
 
 namespace proximity_auth {
 
 namespace {
@@ skipping 25 matching lines @@
     access_token_ = access_token;
   };
 
  private:
   std::string access_token_;
 };
 
 // Mock CryptAuthApiCallFlow, which handles the HTTP requests to CryptAuth.
 class MockCryptAuthApiCallFlow : public CryptAuthApiCallFlow {
  public:
-  MockCryptAuthApiCallFlow() : CryptAuthApiCallFlow(GURL(std::string())) {}
+  MockCryptAuthApiCallFlow() : CryptAuthApiCallFlow() {}
   virtual ~MockCryptAuthApiCallFlow() {}
 
-  MOCK_METHOD5(Start,
-               void(net::URLRequestContextGetter* context,
+  MOCK_METHOD6(Start,
+               void(const GURL&,
+                    net::URLRequestContextGetter* context,
                     const std::string& access_token,
                     const std::string& serialized_request,
                     const ResultCallback& result_callback,
                     const ErrorCallback& error_callback));
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockCryptAuthApiCallFlow);
 };
 
-// Subclass of CryptAuthClient to use as test harness.
-class MockCryptAuthClient : public CryptAuthClient {
- public:
-  // Ownership of |access_token_fetcher| is passed to the superclass. Due to the
-  // limitations of gmock, we need to use a raw pointer argument rather than a
-  // scoped_ptr.
-  MockCryptAuthClient(
-      CryptAuthAccessTokenFetcher* access_token_fetcher,
-      scoped_refptr<net::URLRequestContextGetter> url_request_context,
-      const cryptauth::DeviceClassifier& device_classifier)
-      : CryptAuthClient(make_scoped_ptr(access_token_fetcher),
-                        url_request_context,
-                        device_classifier) {}
-  virtual ~MockCryptAuthClient() {}
-
-  MOCK_METHOD1(CreateFlowProxy, CryptAuthApiCallFlow*(const GURL& request_url));
-
-  scoped_ptr<CryptAuthApiCallFlow> CreateFlow(
-      const GURL& request_url) override {
-    return make_scoped_ptr(CreateFlowProxy(request_url));
-  };
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(MockCryptAuthClient);
-};
-
 // Callback that should never be invoked.
 template <class T>
 void NotCalled(const T& type) {
   EXPECT_TRUE(false);
 }
 
 // Callback that saves the result returned by CryptAuthClient.
 template <class T>
 void SaveResult(T* out, const T& result) {
   *out = result;
 }
 
 }  // namespace
 
 class ProximityAuthCryptAuthClientTest : public testing::Test {
  protected:
   ProximityAuthCryptAuthClientTest()
       : access_token_fetcher_(new FakeCryptAuthAccessTokenFetcher()),
+        api_call_flow_(new StrictMock<MockCryptAuthApiCallFlow>()),
         url_request_context_(
             new net::TestURLRequestContextGetter(new base::NullTaskRunner())),
         serialized_request_(std::string()) {}
 
   void SetUp() override {
     base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
         switches::kCryptAuthHTTPHost, kTestGoogleApisUrl);
 
     cryptauth::DeviceClassifier device_classifier;
     device_classifier.set_device_os_version_code(kDeviceOsVersionCode);
     device_classifier.set_device_software_version_code(
         kDeviceSoftwareVersionCode);
     device_classifier.set_device_software_package(kDeviceSoftwarePackage);
     device_classifier.set_device_type(kDeviceType);
 
-    client_.reset(new StrictMock<MockCryptAuthClient>(
-        access_token_fetcher_, url_request_context_, device_classifier));
+    client_.reset(new CryptAuthClientImpl(
+        make_scoped_ptr(api_call_flow_), make_scoped_ptr(access_token_fetcher_),
+        url_request_context_, device_classifier));
   }
 
   // Sets up an expectation and captures a CryptAuth API request to
   // |request_url|.
   void ExpectRequest(const std::string& request_url) {
-    StrictMock<MockCryptAuthApiCallFlow>* api_call_flow =
-        new StrictMock<MockCryptAuthApiCallFlow>();
-
-    EXPECT_CALL(*client_, CreateFlowProxy(GURL(request_url)))
-        .WillOnce(Return(api_call_flow));
-
-    EXPECT_CALL(*api_call_flow,
-                Start(url_request_context_.get(), kAccessToken, _, _, _))
-        .WillOnce(DoAll(SaveArg<2>(&serialized_request_),
-                        SaveArg<3>(&flow_result_callback_),
-                        SaveArg<4>(&flow_error_callback_)));
+    GURL url(request_url);
+    EXPECT_CALL(*api_call_flow_,
+                Start(url, url_request_context_.get(), kAccessToken, _, _, _))
+        .WillOnce(DoAll(SaveArg<3>(&serialized_request_),
+                        SaveArg<4>(&flow_result_callback_),
+                        SaveArg<5>(&flow_error_callback_)));
   }
 
   // Returns |response_proto| as the result to the current API request.
   // ExpectResult() must have been called first.
   void FinishApiCallFlow(const google::protobuf::MessageLite* response_proto) {
     flow_result_callback_.Run(response_proto->SerializeAsString());
   }
 
   // Ends the current API request with |error_message|. ExpectResult() must have
   // been called first.
   void FailApiCallFlow(const std::string& error_message) {
     flow_error_callback_.Run(error_message);
   }
 
  protected:
   // Owned by |client_|.
   FakeCryptAuthAccessTokenFetcher* access_token_fetcher_;
+  // Owned by |client_|.
+  StrictMock<MockCryptAuthApiCallFlow>* api_call_flow_;
 
   scoped_refptr<net::URLRequestContextGetter> url_request_context_;
-  scoped_ptr<StrictMock<MockCryptAuthClient>> client_;
+  scoped_ptr<CryptAuthClient> client_;
 
   std::string serialized_request_;
   CryptAuthApiCallFlow::ResultCallback flow_result_callback_;
   CryptAuthApiCallFlow::ErrorCallback flow_error_callback_;
 };
 
 TEST_F(ProximityAuthCryptAuthClientTest, GetMyDevicesSuccess) {
   ExpectRequest(
       "https://www.testgoogleapis.com/cryptauth/v1/deviceSync/"
       "getmydevices?alt=proto");
@@ skipping 375 matching lines @@
       expected_request.device_classifier();
   EXPECT_EQ(kDeviceOsVersionCode, device_classifier.device_os_version_code());
   EXPECT_EQ(kDeviceSoftwareVersionCode,
             device_classifier.device_software_version_code());
   EXPECT_EQ(kDeviceSoftwarePackage,
             device_classifier.device_software_package());
   EXPECT_EQ(kDeviceType, device_classifier.device_type());
 }
 
 }  // namespace proximity_auth