Skip to content

Commit

Permalink
Updating the Chromoting Test Driver to target the test environment
Browse files Browse the repository at this point in the history
This change updated several of the Chromoting Test Driver classes to use
URLs and settings which are needed to retrieve and connect to hosts
running in our test environment.

There are also two changes (null check and initializing the FeatureList)
which are needed for the tool to run (external dependencies changed but
the tool hasn't been updated yet).

BUG=N/A

Review-Url: https://codereview.chromium.org/2542343004
Cr-Commit-Position: refs/heads/master@{#436020}
  • Loading branch information
joedow authored and Commit bot committed Dec 2, 2016
1 parent 9979e0e commit 910989d
Show file tree
Hide file tree
Showing 16 changed files with 120 additions and 44 deletions.
11 changes: 7 additions & 4 deletions remoting/client/chromoting_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,9 @@ void ChromotingClient::Start(
host_jid_ = NormalizeJid(host_jid);
local_capabilities_ = capabilities;

if (!protocol_config_)
if (!protocol_config_) {
protocol_config_ = protocol::CandidateSessionConfig::CreateDefault();
if (!audio_consumer_)
protocol_config_->DisableAudioChannel();
}

if (!connection_) {
if (protocol_config_->webrtc_supported()) {
Expand All @@ -87,7 +86,11 @@ void ChromotingClient::Start(
connection_->set_clipboard_stub(this);
connection_->set_video_renderer(video_renderer_);

connection_->InitializeAudio(audio_decode_task_runner_, audio_consumer_);
if (audio_consumer_) {
connection_->InitializeAudio(audio_decode_task_runner_, audio_consumer_);
} else {
protocol_config_->DisableAudioChannel();
}

session_manager_.reset(new protocol::JingleSessionManager(signal_strategy));
session_manager_->set_protocol_config(std::move(protocol_config_));
Expand Down
12 changes: 6 additions & 6 deletions remoting/test/access_token_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ void AccessTokenFetcher::GetAccessTokenFromAuthCode(

// Create a new GaiaOAuthClient for each request to GAIA.
CreateNewGaiaOAuthClientInstance();
auth_client_->GetTokensFromAuthCode(
oauth_client_info_, auth_code, kMaxGetTokensRetries,
this); // GaiaOAuthClient::Delegate* delegate
auth_client_->GetTokensFromAuthCode(oauth_client_info_, auth_code,
kMaxGetTokensRetries,
/*delegate=*/this);
}

void AccessTokenFetcher::GetAccessTokenFromRefreshToken(
Expand All @@ -73,9 +73,9 @@ void AccessTokenFetcher::GetAccessTokenFromRefreshToken(
// Create a new GaiaOAuthClient for each request to GAIA.
CreateNewGaiaOAuthClientInstance();
auth_client_->RefreshToken(oauth_client_info_, refresh_token_,
std::vector<std::string>(), // scopes
/*scopes=*/std::vector<std::string>(),
kMaxGetTokensRetries,
this); // GaiaOAuthClient::Delegate* delegate
/*delegate=*/this);
}

void AccessTokenFetcher::CreateNewGaiaOAuthClientInstance() {
Expand Down Expand Up @@ -181,7 +181,7 @@ void AccessTokenFetcher::ValidateAccessToken() {
// Create a new GaiaOAuthClient for each request to GAIA.
CreateNewGaiaOAuthClientInstance();
auth_client_->GetTokenInfo(access_token_, kMaxGetTokensRetries,
this); // GaiaOAuthClient::Delegate* delegate
/*delegate=*/this);
}

} // namespace test
Expand Down
8 changes: 5 additions & 3 deletions remoting/test/app_remoting_connection_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,11 @@ bool AppRemotingConnectionHelper::StartConnection() {
timer_->Start(FROM_HERE, base::TimeDelta::FromSeconds(30),
run_loop_->QuitClosure());

client_->StartConnection(remote_host_info.GenerateConnectionSetupInfo(
AppRemotingSharedData->access_token(),
AppRemotingSharedData->user_name()));
client_->StartConnection(
/*use_test_api_settings=*/false,
remote_host_info.GenerateConnectionSetupInfo(
AppRemotingSharedData->access_token(),
AppRemotingSharedData->user_name()));

run_loop_->Run();
timer_->Stop();
Expand Down
10 changes: 10 additions & 0 deletions remoting/test/chromoting_test_driver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "base/bind.h"
#include "base/command_line.h"
#include "base/feature_list.h"
#include "base/files/file_path.h"
#include "base/logging.h"
#include "base/message_loop/message_loop.h"
Expand All @@ -25,6 +26,7 @@ const char kLoggingLevelSwitchName[] = "verbosity";
const char kPinSwitchName[] = "pin";
const char kRefreshTokenPathSwitchName[] = "refresh-token-path";
const char kSingleProcessTestsSwitchName[] = "single-process-tests";
const char kTestEnvironmentSwitchName[] = "use-test-env";
const char kUserNameSwitchName[] = "username";
}

Expand Down Expand Up @@ -74,6 +76,10 @@ void PrintUsage() {
switches::kRefreshTokenPathSwitchName);
printf(" %s: Specifies the optional logging level of the tool (0-3)."
" [default: off]\n", switches::kLoggingLevelSwitchName);
printf(
" %s: Specifies that the test environment APIs should be used."
" [default: false]\n",
switches::kTestEnvironmentSwitchName);
}

void PrintAuthCodeInfo() {
Expand Down Expand Up @@ -138,6 +144,7 @@ void PrintJsonFileInfo() {
int main(int argc, char* argv[]) {
base::TestSuite test_suite(argc, argv);
base::MessageLoopForIO message_loop;
base::FeatureList::InitializeInstance(std::string(), std::string());

if (!base::CommandLine::InitializedForCurrentProcess()) {
if (!base::CommandLine::Init(argc, argv)) {
Expand Down Expand Up @@ -221,6 +228,9 @@ int main(int argc, char* argv[]) {

options.pin = command_line->GetSwitchValueASCII(switches::kPinSwitchName);

options.use_test_environment =
command_line->HasSwitch(switches::kTestEnvironmentSwitchName);

// Create and register our global test data object. It will handle
// retrieving an access token or host list for the user. The GTest framework
// will own the lifetime of this object once it is registered below.
Expand Down
9 changes: 5 additions & 4 deletions remoting/test/chromoting_test_driver_environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ ChromotingTestDriverEnvironment::ChromotingTestDriverEnvironment(
user_name_(options.user_name),
pin_(options.pin),
refresh_token_file_path_(options.refresh_token_file_path),
test_access_token_fetcher_(nullptr),
test_refresh_token_store_(nullptr),
test_host_list_fetcher_(nullptr) {
use_test_environment_(options.use_test_environment) {
DCHECK(!user_name_.empty());
DCHECK(!host_name_.empty());
}
Expand Down Expand Up @@ -294,7 +292,10 @@ bool ChromotingTestDriverEnvironment::RetrieveHostList() {
base::Bind(&ChromotingTestDriverEnvironment::OnHostListRetrieved,
base::Unretained(this), run_loop.QuitClosure());

host_list_fetcher->RetrieveHostlist(access_token_, host_list_callback);
host_list_fetcher->RetrieveHostlist(
access_token_,
use_test_environment_ ? kHostListTestRequestUrl : kHostListProdRequestUrl,
host_list_callback);

run_loop.Run();

Expand Down
11 changes: 8 additions & 3 deletions remoting/test/chromoting_test_driver_environment.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class ChromotingTestDriverEnvironment : public testing::Environment {
std::string host_jid;
std::string pin;
base::FilePath refresh_token_file_path;
bool use_test_environment;
};

explicit ChromotingTestDriverEnvironment(const EnvironmentOptions& options);
Expand Down Expand Up @@ -72,6 +73,7 @@ class ChromotingTestDriverEnvironment : public testing::Environment {
const std::string& host_name() const { return host_name_; }
const std::string& pin() const { return pin_; }
const std::string& user_name() const { return user_name_; }
bool use_test_environment() const { return use_test_environment_; }
const std::vector<HostInfo>& host_list() const { return host_list_; }
const HostInfo& host_info() const { return host_info_; }

Expand Down Expand Up @@ -125,20 +127,23 @@ class ChromotingTestDriverEnvironment : public testing::Environment {
// Path to a JSON file containing refresh tokens.
base::FilePath refresh_token_file_path_;

// Indicates whether the test environment APIs should be used.
const bool use_test_environment_;

// List of remote hosts for the specified user/test-account.
std::vector<HostInfo> host_list_;

// Used to generate connection setup information to connect to |host_name_|.
HostInfo host_info_;

// Access token fetcher used by TestDriverEnvironment tests.
remoting::test::AccessTokenFetcher* test_access_token_fetcher_;
remoting::test::AccessTokenFetcher* test_access_token_fetcher_ = nullptr;

// RefreshTokenStore used by TestDriverEnvironment tests.
remoting::test::RefreshTokenStore* test_refresh_token_store_;
remoting::test::RefreshTokenStore* test_refresh_token_store_ = nullptr;

// HostListFetcher used by TestDriverEnvironment tests.
remoting::test::HostListFetcher* test_host_list_fetcher_;
remoting::test::HostListFetcher* test_host_list_fetcher_ = nullptr;

// Used for running network request tasks.
std::unique_ptr<base::MessageLoopForIO> message_loop_;
Expand Down
2 changes: 1 addition & 1 deletion remoting/test/chromoting_test_driver_tests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

namespace {
const base::TimeDelta kPinBasedMaxConnectionTimeInSeconds =
base::TimeDelta::FromSeconds(5);
base::TimeDelta::FromSeconds(10);
}

namespace remoting {
Expand Down
1 change: 1 addition & 0 deletions remoting/test/chromoting_test_fixture.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ bool ChromotingTestFixture::ConnectToHost(
protocol::ConnectionToHost::State::INITIALIZING,
protocol::ErrorCode::OK);
test_chromoting_client_->StartConnection(
g_chromoting_shared_data->use_test_environment(),
g_chromoting_shared_data->host_info().GenerateConnectionSetupInfo(
g_chromoting_shared_data->access_token(),
g_chromoting_shared_data->user_name(),
Expand Down
1 change: 1 addition & 0 deletions remoting/test/fake_host_list_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ FakeHostListFetcher::~FakeHostListFetcher() {
}

void FakeHostListFetcher::RetrieveHostlist(const std::string& access_token,
const std::string& target_url,
const HostlistCallback& callback) {
callback.Run(host_list_);
}
Expand Down
1 change: 1 addition & 0 deletions remoting/test/fake_host_list_fetcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class FakeHostListFetcher : public HostListFetcher {

// HostListFetcher interface.
void RetrieveHostlist(const std::string& access_token,
const std::string& target_url,
const HostlistCallback& callback) override;

void set_retrieved_host_list(const std::vector<HostInfo>& host_list) {
Expand Down
15 changes: 7 additions & 8 deletions remoting/test/host_list_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,9 @@ HostListFetcher::HostListFetcher() {
HostListFetcher::~HostListFetcher() {
}

void HostListFetcher::RetrieveHostlist(
const std::string& access_token,
const HostlistCallback& callback) {

void HostListFetcher::RetrieveHostlist(const std::string& access_token,
const std::string& target_url,
const HostlistCallback& callback) {
VLOG(2) << "HostListFetcher::RetrieveHostlist() called";

DCHECK(!access_token.empty());
Expand All @@ -36,11 +35,11 @@ void HostListFetcher::RetrieveHostlist(
hostlist_callback_ = callback;

request_context_getter_ = new remoting::URLRequestContextGetter(
base::ThreadTaskRunnerHandle::Get(), // network_runner
base::ThreadTaskRunnerHandle::Get()); // file_runner
/*network_runner=*/base::ThreadTaskRunnerHandle::Get(),
/*file_runner=*/base::ThreadTaskRunnerHandle::Get());

request_ = net::URLFetcher::Create(
GURL(kHostListProdRequestUrl), net::URLFetcher::GET, this);
request_ =
net::URLFetcher::Create(GURL(target_url), net::URLFetcher::GET, this);
request_->SetRequestContext(request_context_getter_.get());
request_->AddExtraRequestHeader("Authorization: OAuth " + access_token);
request_->Start();
Expand Down
3 changes: 3 additions & 0 deletions remoting/test/host_list_fetcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ namespace test {
// unittests for this class to set fake response data for these URLs.
const char kHostListProdRequestUrl[] = "https://www.googleapis.com/"
"chromoting/v1/@me/hosts";
const char kHostListTestRequestUrl[] =
"https://www-googleapis-test.sandbox.google.com/chromoting/v1/@me/hosts";

// Requests a host list from the directory service for an access token.
// Destroying the RemoteHostInfoFetcher while a request is outstanding
Expand All @@ -45,6 +47,7 @@ class HostListFetcher : public net::URLFetcherDelegate {
// Makes a service call to retrieve a hostlist. The
// callback will be called once the HTTP request has completed.
virtual void RetrieveHostlist(const std::string& access_token,
const std::string& target_url,
const HostlistCallback& callback);

private:
Expand Down
62 changes: 54 additions & 8 deletions remoting/test/host_list_fetcher_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,9 @@ void HostListFetcherTest::SetUp() {
SetFakeResponse(GURL(kHostListProdRequestUrl),
kHostListEmptyResponse, net::HTTP_NOT_FOUND,
net::URLRequestStatus::FAILED);

SetFakeResponse(GURL(kHostListTestRequestUrl), kHostListEmptyResponse,
net::HTTP_NOT_FOUND, net::URLRequestStatus::FAILED);
}

void HostListFetcherTest::SetFakeResponse(
Expand All @@ -228,7 +231,45 @@ TEST_F(HostListFetcherTest, RetrieveHostListFromProd) {
base::Bind(&OnHostlistRetrieved, run_loop.QuitClosure(), &hostlist);

HostListFetcher host_list_fetcher;
host_list_fetcher.RetrieveHostlist(kAccessTokenValue, host_list_callback);
host_list_fetcher.RetrieveHostlist(kAccessTokenValue, kHostListProdRequestUrl,
host_list_callback);

run_loop.Run();

EXPECT_EQ(hostlist.size(), kExpectedHostListSize);

HostInfo online_host_info = hostlist.at(0);
EXPECT_EQ(online_host_info.token_url_patterns.size(), kExpectedPatternsSize);
EXPECT_FALSE(online_host_info.host_id.empty());
EXPECT_FALSE(online_host_info.host_jid.empty());
EXPECT_FALSE(online_host_info.host_name.empty());
EXPECT_EQ(online_host_info.status, HostStatus::kHostStatusOnline);
EXPECT_TRUE(online_host_info.offline_reason.empty());
EXPECT_FALSE(online_host_info.public_key.empty());

HostInfo offline_host_info = hostlist.at(1);
EXPECT_TRUE(offline_host_info.token_url_patterns.empty());
EXPECT_FALSE(offline_host_info.host_id.empty());
EXPECT_FALSE(offline_host_info.host_jid.empty());
EXPECT_FALSE(offline_host_info.host_name.empty());
EXPECT_EQ(offline_host_info.status, HostStatus::kHostStatusOffline);
EXPECT_FALSE(offline_host_info.offline_reason.empty());
EXPECT_FALSE(offline_host_info.public_key.empty());
}

TEST_F(HostListFetcherTest, RetrieveHostListFromTest) {
SetFakeResponse(GURL(kHostListTestRequestUrl), kHostListReadyResponse,
net::HTTP_OK, net::URLRequestStatus::SUCCESS);

std::vector<HostInfo> hostlist;

base::RunLoop run_loop;
HostListFetcher::HostlistCallback host_list_callback =
base::Bind(&OnHostlistRetrieved, run_loop.QuitClosure(), &hostlist);

HostListFetcher host_list_fetcher;
host_list_fetcher.RetrieveHostlist(kAccessTokenValue, kHostListTestRequestUrl,
host_list_callback);

run_loop.Run();

Expand Down Expand Up @@ -265,7 +306,8 @@ TEST_F(HostListFetcherTest, RetrieveHostListWithEmptyPatterns) {
base::Bind(&OnHostlistRetrieved, run_loop.QuitClosure(), &hostlist);

HostListFetcher host_list_fetcher;
host_list_fetcher.RetrieveHostlist(kAccessTokenValue, host_list_callback);
host_list_fetcher.RetrieveHostlist(kAccessTokenValue, kHostListProdRequestUrl,
host_list_callback);

run_loop.Run();

Expand Down Expand Up @@ -295,7 +337,8 @@ TEST_F(HostListFetcherTest,
base::Bind(&OnHostlistRetrieved, run_loop.QuitClosure(), &hostlist);

HostListFetcher host_list_fetcher;
host_list_fetcher.RetrieveHostlist(kAccessTokenValue, host_list_callback);
host_list_fetcher.RetrieveHostlist(kAccessTokenValue, kHostListProdRequestUrl,
host_list_callback);
run_loop.Run();

EXPECT_EQ(hostlist.size(), kExpectedHostListSize);
Expand Down Expand Up @@ -329,7 +372,8 @@ TEST_F(HostListFetcherTest, RetrieveHostListNetworkError) {
base::Bind(&OnHostlistRetrieved, run_loop.QuitClosure(), &hostlist);

HostListFetcher host_list_fetcher;
host_list_fetcher.RetrieveHostlist(kAccessTokenValue, host_list_callback);
host_list_fetcher.RetrieveHostlist(kAccessTokenValue, kHostListProdRequestUrl,
host_list_callback);
run_loop.Run();

// If there was a network error retrieving the host list, then the host list
Expand All @@ -350,7 +394,8 @@ TEST_F(HostListFetcherTest, RetrieveHostListEmptyItemsResponse) {
base::Bind(&OnHostlistRetrieved, run_loop.QuitClosure(), &hostlist);

HostListFetcher host_list_fetcher;
host_list_fetcher.RetrieveHostlist(kAccessTokenValue, host_list_callback);
host_list_fetcher.RetrieveHostlist(kAccessTokenValue, kHostListProdRequestUrl,
host_list_callback);
run_loop.Run();

// If we received an empty items response, then host list should be empty.
Expand All @@ -370,7 +415,8 @@ TEST_F(HostListFetcherTest, RetrieveHostListEmptyResponse) {
base::Bind(&OnHostlistRetrieved, run_loop.QuitClosure(), &hostlist);

HostListFetcher host_list_fetcher;
host_list_fetcher.RetrieveHostlist(kAccessTokenValue, host_list_callback);
host_list_fetcher.RetrieveHostlist(kAccessTokenValue, kHostListProdRequestUrl,
host_list_callback);
run_loop.Run();

// If we received an empty response, then host list should be empty.
Expand All @@ -392,7 +438,7 @@ TEST_F(HostListFetcherTest, MultipleRetrieveHostListRequests) {
&ready_hostlist);

HostListFetcher host_list_fetcher;
host_list_fetcher.RetrieveHostlist(kAccessTokenValue,
host_list_fetcher.RetrieveHostlist(kAccessTokenValue, kHostListProdRequestUrl,
ready_host_list_callback);

ready_run_loop.Run();
Expand Down Expand Up @@ -432,7 +478,7 @@ TEST_F(HostListFetcherTest, MultipleRetrieveHostListRequests) {
&empty_items_hostlist);

// Re-use the same host_list_fetcher.
host_list_fetcher.RetrieveHostlist(kAccessTokenValue,
host_list_fetcher.RetrieveHostlist(kAccessTokenValue, kHostListProdRequestUrl,
empty_host_list_callback);

empty_items_run_loop.Run();
Expand Down
Loading

0 comments on commit 910989d

Please sign in to comment.