Skip to content
This repository has been archived by the owner on Aug 19, 2019. It is now read-only.

Add unit tests for environment with fake HTTP server. #139

Merged
merged 7 commits into from
Jul 17, 2018
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,14 @@ constexpr const char kGceInstanceResourceType[] = "gce_instance";
}

Environment::Environment(const Configuration& config)
: config_(config), application_default_credentials_read_(false) {}
: config_(config),
metadata_server_url_(kGceMetadataServerAddress),
application_default_credentials_read_(false) {}

std::string Environment::GetMetadataString(const std::string& path) const {
http::client::options options;
http::client client(options.timeout(2));
http::client::request request(kGceMetadataServerAddress + path);
http::client::request request(metadata_server_url_ + path);
request << boost::network::header("Metadata-Flavor", "Google");
try {
http::client::response response = client.get(request);
Expand All @@ -98,7 +100,7 @@ std::string Environment::GetMetadataString(const std::string& path) const {
}
} catch (const boost::system::system_error& e) {
LOG(ERROR) << "Exception: " << e.what()
<< ": '" << kGceMetadataServerAddress << path << "'";
<< ": '" << metadata_server_url_ << path << "'";
return "";
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/environment.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ class Environment {

void ReadApplicationDefaultCredentials() const;

// The url must end in a '/'.
void SetMetadataServerUrlForTest(const std::string& url) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could add a comment saying that the url must end in a '/'...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this only "ForTest"? It seems like a reasonable enough setter function to remove that suffix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make it clear that the method only exists because we need to override the URL in test code. If we ever need to use it in production code, we can rename the method.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

metadata_server_url_ = url;
}

const Configuration& config_;

// Cached data.
Expand All @@ -60,6 +65,7 @@ class Environment {
mutable std::string kubernetes_cluster_location_;
mutable std::string client_email_;
mutable std::string private_key_;
mutable std::string metadata_server_url_;
mutable bool application_default_credentials_read_;
};

Expand Down
2 changes: 1 addition & 1 deletion test/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ base64_unittest: base64_unittest.o $(SRC_DIR)/base64.o
$(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@
configuration_unittest: configuration_unittest.o $(SRC_DIR)/configuration.o
$(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@
environment_unittest: environment_unittest.o $(SRC_DIR)/environment.o $(SRC_DIR)/configuration.o $(SRC_DIR)/format.o $(SRC_DIR)/json.o $(SRC_DIR)/logging.o $(SRC_DIR)/time.o
environment_unittest: environment_unittest.o fake_http_server.o $(SRC_DIR)/environment.o $(SRC_DIR)/configuration.o $(SRC_DIR)/format.o $(SRC_DIR)/json.o $(SRC_DIR)/logging.o $(SRC_DIR)/time.o
$(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@
format_unittest: format_unittest.o $(SRC_DIR)/format.o
$(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@
Expand Down
22 changes: 21 additions & 1 deletion test/environment_unittest.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "../src/environment.h"
#include "fake_http_server.h"
#include "gtest/gtest.h"

#include <fstream>
Expand All @@ -12,9 +13,15 @@ class EnvironmentTest : public ::testing::Test {
static void ReadApplicationDefaultCredentials(const Environment& environment) {
environment.ReadApplicationDefaultCredentials();
}

static void SetMetadataServerUrlForTest(Environment* environment,
const std::string& url) {
environment->SetMetadataServerUrlForTest(url);
}
};

namespace {

// A file with a given name in a temporary (unique) directory.
boost::filesystem::path TempPath(const std::string& filename) {
boost::filesystem::path path = boost::filesystem::temp_directory_path();
Expand Down Expand Up @@ -42,6 +49,7 @@ class TemporaryFile {
private:
boost::filesystem::path path_;
};

} // namespace

TEST(TemporaryFile, Basic) {
Expand Down Expand Up @@ -70,7 +78,6 @@ TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsSucceeds) {
TemporaryFile credentials_file(
std::string(test_info_->name()) + "_creds.json",
"{\"client_email\":\"[email protected]\",\"private_key\":\"some_key\"}");
std::string cfg;
Configuration config(std::istringstream(
"CredentialsFile: '" + credentials_file.FullPath().native() + "'\n"
));
Expand Down Expand Up @@ -98,4 +105,17 @@ TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsCaches) {
);
EXPECT_EQ("some_key", environment.CredentialsPrivateKey());
}

TEST_F(EnvironmentTest, GetMetadataString) {
testing::FakeServer server;
server.SetResponse("/a/b/c", "hello");

Configuration config;
Environment environment(config);
SetMetadataServerUrlForTest(&environment, server.GetUrl());

EXPECT_EQ("hello", environment.GetMetadataString("a/b/c"));
EXPECT_EQ("", environment.GetMetadataString("unknown/path"));
}

} // namespace google
47 changes: 47 additions & 0 deletions test/fake_http_server.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#include "fake_http_server.h"

namespace google {
namespace testing {

// Note: An empty port selects a random available port (this behavior
// is not documented).
FakeServer::FakeServer()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about a comment along the lines of "an empty port selects a random available port (undocumented)"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't a class comment — let's move it to where the empty port is passed in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

: server_(Server::options(handler_).address("127.0.0.1").port("")) {
server_.listen();
server_thread_ = std::thread([this] { server_.run(); });
}

FakeServer::~FakeServer() {
server_.stop();
server_thread_.join();
}

std::string FakeServer::GetUrl() {
network::uri_builder builder;
builder.scheme("http").host(server_.address()).port(server_.port()).path("/");
return builder.uri().string();
}

void FakeServer::SetResponse(const std::string& path,
const std::string& response) {
handler_.path_responses[path] = response;
}

void FakeServer::Handler::operator()(Server::request const &request,
Server::connection_ptr connection) {
auto it = path_responses.find(request.destination);
if (it != path_responses.end()) {
connection->set_status(Server::connection::ok);
connection->set_headers(std::map<std::string, std::string>({
{"Content-Type", "text/plain"},
}));
connection->write(it->second);
} else {
// Note: We have to set headers; otherwise, an exception is thrown.
connection->set_status(Server::connection::not_found);
connection->set_headers(std::map<std::string, std::string>());
}
}

} // testing
} // google
38 changes: 38 additions & 0 deletions test/fake_http_server.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#ifndef FAKE_HTTP_SERVER_H_
#define FAKE_HTTP_SERVER_H_

#include <boost/network/protocol/http/server.hpp>

namespace google {
namespace testing {

// Starts a server in a separate thread, allowing it to choose an
// available port.
class FakeServer {
public:
FakeServer();
~FakeServer();

std::string GetUrl();
void SetResponse(const std::string& path, const std::string& response);

private:
struct Handler;
typedef boost::network::http::server<Handler> Server;

// Handler that maps paths to response strings.
struct Handler {
void operator()(Server::request const &request,
Server::connection_ptr connection);
std::map<std::string, std::string> path_responses;
};

Handler handler_;
Server server_;
std::thread server_thread_;
};

} // testing
} // google

#endif // FAKE_HTTP_SERVER_H_