Allows client to access auth token from command line specified file.

blimp/client/session/assignment_source_unittest.cc

 // Copyright 2016 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 "blimp/client/session/assignment_source.h"
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
 #include "base/test/test_simple_task_runner.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "blimp/client/app/blimp_client_switches.h"
+#include "blimp/common/fake_commandline.h"
+#include "blimp/common/get_client_token.h"
 #include "blimp/common/protocol_version.h"
 #include "components/safe_json/testing_json_parser.h"
 #include "net/base/test_data_directory.h"
 #include "net/url_request/test_url_fetcher_factory.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
 using testing::_;
 using testing::DoAll;
@@ skipping 46 matching lines @@
     ASSERT_FALSE(src_root.empty());
     cert_path_ = src_root.Append(kCertRelativePath);
     ASSERT_TRUE(base::ReadFileToString(cert_path_, &cert_pem_));
     net::CertificateList cert_list =
         net::X509Certificate::CreateCertificateListFromBytes(
             cert_pem_.data(), cert_pem_.size(),
             net::X509Certificate::FORMAT_PEM_CERT_SEQUENCE);
     ASSERT_FALSE(cert_list.empty());
     cert_ = std::move(cert_list[0]);
     ASSERT_TRUE(cert_);
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());

[Kevin M, 2016/05/10 18:06:34]

This variable doesn't exist?

You can compile the client w/o a device, since this error would be caught by the
compiler.

[CJ, 2016/05/10 20:58:06]

It exists in fake_commandline.h. Too obscured away?
If this is supposed to break, then I'm afraid I'm doing something wrong
compiling.

[Kevin M, 2016/05/11 00:39:56]

Ah, I thought that test_dir_ was a private class field (implied by the trailing
underscore).

[CJ, 2016/05/11 21:49:30]

It got removed because it is unneeded in the preferred way to manipulate the
commandline.

+    CreateFileForSwitch(engine::kClientTokenPath, kTestClientToken);
   }
 
   // This expects the AssignmentSource::GetAssignment to return a custom
   // endpoint without having to hit the network.  This will typically be used
   // for testing that specifying an assignment via the command line works as
   // expected.
   void GetAlternateAssignment() {
     source_.GetAssignment("",
                           base::Bind(&AssignmentSourceTest::AssignmentResponse,
                                      base::Unretained(this)));
@@ skipping 94 matching lines @@
 AssignmentSourceTest::BuildAssignerResponse() {
   std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue);
   dict->SetString("clientToken", kTestClientToken);
   dict->SetString("host", kTestIpAddressString);
   dict->SetInteger("port", kTestPort);
   dict->SetString("certificate", cert_pem_);
   return dict;
 }
 
 TEST_F(AssignmentSourceTest, TestTCPAlternateEndpointSuccess) {
+  auto cmd_line = CreateCommandLine(all_filepath_switches_);

[Kevin M, 2016/05/10 18:06:34]

We should be consistent with how we're manipulating commandline switches. Add
another call to AppendSwitchASCII() instead.

(ditto for TestSSLAlternateEndpointSuccess)

[CJ, 2016/05/10 20:58:06]

Is this similar to what is done in assignment_source.cc? I was trying to
maintain consistency between this and the server tests. 

[Kevin M, 2016/05/11 00:39:56]

Check out line 210 of this patch, which mutates the current process' current
command line w/arguments.

[CJ, 2016/05/11 21:49:30]

Done.

   Assignment assignment;
   assignment.transport_protocol = Assignment::TransportProtocol::TCP;
   assignment.engine_endpoint = net::IPEndPoint(kTestIpAddress, kTestPort);
-  assignment.client_token = kDummyClientToken;
+  assignment.client_token = GetClientToken(cmd_line);

[Kevin M, 2016/05/10 18:06:34]

Set the expectations directly here; otherwise we conflate the expected value
with the actual value.

[CJ, 2016/05/10 20:58:06]

I'm not sure what you mean by this. What do you mean by setting expectations
directly.

[Kevin M, 2016/05/11 00:39:56]

In this patch, we're testing GetClientToken() by indirectly comparing the output
of GetClientToken against the output of... itself.

It'd be more effective if we compared it to a known fixed string constant.

[CJ, 2016/05/11 21:49:30]

So, would adding an assert like so work? Or are we looking for something a
little deeper than that?

   assignment.cert = scoped_refptr<net::X509Certificate>(nullptr);
 
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       switches::kEngineIP, kTestIpAddressString);
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       switches::kEnginePort, std::to_string(kTestPort));
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       switches::kEngineTransport, kTcpTransportName);
 
   EXPECT_CALL(*this, AssignmentResponse(AssignmentSource::Result::RESULT_OK,
                                         AssignmentEquals(assignment)))
       .Times(1);
 
   GetAlternateAssignment();
 }
 
 TEST_F(AssignmentSourceTest, TestSSLAlternateEndpointSuccess) {
+  auto cmd_line = CreateCommandLine(all_filepath_switches_);
   Assignment assignment;
   assignment.transport_protocol = Assignment::TransportProtocol::SSL;
   assignment.engine_endpoint = net::IPEndPoint(kTestIpAddress, kTestPort);
-  assignment.client_token = kDummyClientToken;
+  assignment.client_token = GetClientToken(cmd_line);
   assignment.cert = cert_;
 
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       switches::kEngineIP, kTestIpAddressString);
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       switches::kEnginePort, std::to_string(kTestPort));
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       switches::kEngineTransport, kSslTransportName);
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       switches::kEngineCertPath, cert_path_.value());
@@ skipping 144 matching lines @@
   EXPECT_CALL(*this, AssignmentResponse(
                          AssignmentSource::Result::RESULT_INVALID_CERT, _));
   GetNetworkAssignmentAndWaitForResponse(net::HTTP_OK, net::Error::OK,
                                          ValueToString(*response),
                                          kTestAuthToken, kEngineVersion);
 }
 
 }  // namespace
 }  // namespace client
 }  // namespace blimp