From 90bd2ce28d009c0bcdf85a4cedb6efffe16c0ec6 Mon Sep 17 00:00:00 2001 From: David Tucker Date: Wed, 23 May 2018 14:32:46 -0400 Subject: [PATCH 1/7] Add unit tests for environment with fake http server. --- src/configuration.cc | 6 +++ src/configuration.h | 5 +++ src/environment.cc | 7 +--- test/environment_unittest.cc | 73 ++++++++++++++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 5 deletions(-) diff --git a/src/configuration.cc b/src/configuration.cc index daab70ce..5026595b 100644 --- a/src/configuration.cc +++ b/src/configuration.cc @@ -39,6 +39,8 @@ constexpr const char kConfigFileFlag[] = "config-file"; constexpr const char kDefaultProjectId[] = ""; constexpr const char kDefaultCredentialsFile[] = ""; +constexpr const char kGceMetadataServerAddress[] = + "http://metadata.google.internal./computeMetadata/v1/"; constexpr const int kMetadataApiDefaultNumThreads = 3; constexpr const int kMetadataApiDefaultPort = 8000; constexpr const char kMetadataApiDefaultResourceTypeSeparator[] = "."; @@ -82,6 +84,7 @@ Configuration::Configuration() : project_id_(kDefaultProjectId), credentials_file_(kDefaultCredentialsFile), verbose_logging_(false), + gce_metadata_server_address_(kGceMetadataServerAddress), metadata_api_num_threads_(kMetadataApiDefaultNumThreads), metadata_api_port_(kMetadataApiDefaultPort), metadata_api_resource_type_separator_( @@ -215,6 +218,9 @@ void Configuration::ParseConfiguration(std::istream& input) { config["ProjectId"].as(project_id_); credentials_file_ = config["CredentialsFile"].as(credentials_file_); + gce_metadata_server_address_ = + config["GceMetadataServerAddress"].as( + gce_metadata_server_address_); metadata_api_num_threads_ = config["MetadataApiNumThreads"].as(metadata_api_num_threads_); metadata_api_port_ = diff --git a/src/configuration.h b/src/configuration.h index 2f90a3b6..64c16cc5 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -43,6 +43,10 @@ class Configuration { std::lock_guard lock(mutex_); return verbose_logging_; } + const std::string& GceMetadataServerAddress() const { + std::lock_guard lock(mutex_); + return gce_metadata_server_address_; + } // Metadata API server configuration options. int MetadataApiNumThreads() const { std::lock_guard lock(mutex_); @@ -180,6 +184,7 @@ class Configuration { std::string project_id_; std::string credentials_file_; bool verbose_logging_; + std::string gce_metadata_server_address_; int metadata_api_num_threads_; int metadata_api_port_; std::string metadata_api_resource_type_separator_; diff --git a/src/environment.cc b/src/environment.cc index bc9ac79b..972b1730 100644 --- a/src/environment.cc +++ b/src/environment.cc @@ -70,9 +70,6 @@ json::value ReadCredentials( return std::move(creds_json); } -constexpr const char kGceMetadataServerAddress[] = - "http://metadata.google.internal./computeMetadata/v1/"; - constexpr const char kGceInstanceResourceType[] = "gce_instance"; } @@ -83,7 +80,7 @@ Environment::Environment(const Configuration& config) 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(config_.GceMetadataServerAddress() + path); request << boost::network::header("Metadata-Flavor", "Google"); try { http::client::response response = client.get(request); @@ -98,7 +95,7 @@ std::string Environment::GetMetadataString(const std::string& path) const { } } catch (const boost::system::system_error& e) { LOG(ERROR) << "Exception: " << e.what() - << ": '" << kGceMetadataServerAddress << path << "'"; + << ": '" << config_.GceMetadataServerAddress() << path << "'"; return ""; } } diff --git a/test/environment_unittest.cc b/test/environment_unittest.cc index 09ab5655..d711e43b 100644 --- a/test/environment_unittest.cc +++ b/test/environment_unittest.cc @@ -4,6 +4,7 @@ #include #include #include +#include namespace google { @@ -15,6 +16,7 @@ class EnvironmentTest : public ::testing::Test { }; 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(); @@ -42,6 +44,35 @@ class TemporaryFile { private: boost::filesystem::path path_; }; + +namespace http = boost::network::http; + +struct FakeHandler; +typedef http::server FakeServer; + +// Fake web server that maps paths to response strings. +class FakeHandler { +public: + FakeHandler(std::map paths) : paths_(paths) {} + void operator() (FakeServer::request const &request, + FakeServer::connection_ptr connection) { + auto it = paths_.find(request.destination); + if (it != paths_.end()) { + connection->set_status(FakeServer::connection::ok); + connection->set_headers(std::map({ + {"Content-Type", "text/plain"}, + })); + connection->write(it->second); + } else { + // Note: We have to set headers; otherwise, an exception is thrown. + connection->set_status(FakeServer::connection::not_found); + connection->set_headers(std::map({})); + } + } +private: + std::map paths_; +}; + } // namespace TEST(TemporaryFile, Basic) { @@ -98,4 +129,46 @@ TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsCaches) { ); EXPECT_EQ("some_key", environment.CredentialsPrivateKey()); } + +TEST_F(EnvironmentTest, GetMetadataStringWithFakeServer) { + // Start a server in a separate thread, and allow it to choose an + // available port. + FakeHandler handler(std::map({{"/a/b/c", "hello"}})); + FakeServer::options options(handler); + FakeServer server(options.address("127.0.0.1").port("")); + server.listen(); + std::thread t1([&server] { server.run(); }); + + // Set the config to use the local address and port. + TemporaryFile credentials_file( + std::string(test_info_->name()) + "_creds.json", + "{\"client_email\":\"user@example.com\",\"private_key\":\"some_key\"}"); + std::string cfg; + Configuration config(std::istringstream( + "CredentialsFile: '" + credentials_file.FullPath().native() + "'\n" + "GceMetadataServerAddress: 'http://127.0.0.1:" + server.port() + "'\n" + )); + Environment environment(config); + + EXPECT_EQ("hello", environment.GetMetadataString("/a/b/c")); + EXPECT_EQ("", environment.GetMetadataString("/unknown/path")); + + server.stop(); + t1.join(); +} + +TEST_F(EnvironmentTest, GetMetadataStringWithInvalidAddress) { + TemporaryFile credentials_file( + std::string(test_info_->name()) + "_creds.json", + "{\"client_email\":\"user@example.com\",\"private_key\":\"some_key\"}"); + std::string cfg; + Configuration config(std::istringstream( + "CredentialsFile: '" + credentials_file.FullPath().native() + "'\n" + "GceMetadataServerAddress: 'http://invalidaddress'\n" + )); + Environment environment(config); + + EXPECT_EQ("", environment.GetMetadataString("/a/b/c")); +} + } // namespace google From b68673494b18a40ce54c8fba484c0490a12176c3 Mon Sep 17 00:00:00 2001 From: "David B. Tucker" Date: Tue, 29 May 2018 17:38:38 -0400 Subject: [PATCH 2/7] Addressed feedback. --- src/configuration.cc | 6 -- src/configuration.h | 5 -- src/environment.cc | 15 ++++- src/environment.h | 5 ++ test/environment_unittest.cc | 119 +++++++++++++++++------------------ 5 files changed, 76 insertions(+), 74 deletions(-) diff --git a/src/configuration.cc b/src/configuration.cc index 5026595b..daab70ce 100644 --- a/src/configuration.cc +++ b/src/configuration.cc @@ -39,8 +39,6 @@ constexpr const char kConfigFileFlag[] = "config-file"; constexpr const char kDefaultProjectId[] = ""; constexpr const char kDefaultCredentialsFile[] = ""; -constexpr const char kGceMetadataServerAddress[] = - "http://metadata.google.internal./computeMetadata/v1/"; constexpr const int kMetadataApiDefaultNumThreads = 3; constexpr const int kMetadataApiDefaultPort = 8000; constexpr const char kMetadataApiDefaultResourceTypeSeparator[] = "."; @@ -84,7 +82,6 @@ Configuration::Configuration() : project_id_(kDefaultProjectId), credentials_file_(kDefaultCredentialsFile), verbose_logging_(false), - gce_metadata_server_address_(kGceMetadataServerAddress), metadata_api_num_threads_(kMetadataApiDefaultNumThreads), metadata_api_port_(kMetadataApiDefaultPort), metadata_api_resource_type_separator_( @@ -218,9 +215,6 @@ void Configuration::ParseConfiguration(std::istream& input) { config["ProjectId"].as(project_id_); credentials_file_ = config["CredentialsFile"].as(credentials_file_); - gce_metadata_server_address_ = - config["GceMetadataServerAddress"].as( - gce_metadata_server_address_); metadata_api_num_threads_ = config["MetadataApiNumThreads"].as(metadata_api_num_threads_); metadata_api_port_ = diff --git a/src/configuration.h b/src/configuration.h index 64c16cc5..2f90a3b6 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -43,10 +43,6 @@ class Configuration { std::lock_guard lock(mutex_); return verbose_logging_; } - const std::string& GceMetadataServerAddress() const { - std::lock_guard lock(mutex_); - return gce_metadata_server_address_; - } // Metadata API server configuration options. int MetadataApiNumThreads() const { std::lock_guard lock(mutex_); @@ -184,7 +180,6 @@ class Configuration { std::string project_id_; std::string credentials_file_; bool verbose_logging_; - std::string gce_metadata_server_address_; int metadata_api_num_threads_; int metadata_api_port_; std::string metadata_api_resource_type_separator_; diff --git a/src/environment.cc b/src/environment.cc index 972b1730..636e5104 100644 --- a/src/environment.cc +++ b/src/environment.cc @@ -70,17 +70,22 @@ json::value ReadCredentials( return std::move(creds_json); } +constexpr const char kGceMetadataServerAddress[] = + "http://metadata.google.internal./computeMetadata/v1/"; + constexpr const char kGceInstanceResourceType[] = "gce_instance"; } Environment::Environment(const Configuration& config) - : config_(config), application_default_credentials_read_(false) {} + : config_(config), + gce_metadata_server_address_(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(config_.GceMetadataServerAddress() + path); + http::client::request request(gce_metadata_server_address_ + path); request << boost::network::header("Metadata-Flavor", "Google"); try { http::client::response response = client.get(request); @@ -95,7 +100,7 @@ std::string Environment::GetMetadataString(const std::string& path) const { } } catch (const boost::system::system_error& e) { LOG(ERROR) << "Exception: " << e.what() - << ": '" << config_.GceMetadataServerAddress() << path << "'"; + << ": '" << gce_metadata_server_address_ << path << "'"; return ""; } } @@ -267,4 +272,8 @@ void Environment::ReadApplicationDefaultCredentials() const { application_default_credentials_read_ = true; } +void Environment::SetGceMetadataServerAddress(const std::string& address) { + gce_metadata_server_address_ = address; +} + } // google diff --git a/src/environment.h b/src/environment.h index aa356b61..7101278a 100644 --- a/src/environment.h +++ b/src/environment.h @@ -48,8 +48,13 @@ class Environment { void ReadApplicationDefaultCredentials() const; + // Used to override server address in tests. + void SetGceMetadataServerAddress(const std::string& address); + const Configuration& config_; + std::string gce_metadata_server_address_; + // Cached data. mutable std::mutex mutex_; mutable std::string project_id_; diff --git a/test/environment_unittest.cc b/test/environment_unittest.cc index d711e43b..a066ad92 100644 --- a/test/environment_unittest.cc +++ b/test/environment_unittest.cc @@ -13,6 +13,11 @@ class EnvironmentTest : public ::testing::Test { static void ReadApplicationDefaultCredentials(const Environment& environment) { environment.ReadApplicationDefaultCredentials(); } + + static void SetGceMetadataServerAddress(Environment* environment, + const std::string& address) { + environment->SetGceMetadataServerAddress(address); + } }; namespace { @@ -45,32 +50,56 @@ class TemporaryFile { boost::filesystem::path path_; }; -namespace http = boost::network::http; - -struct FakeHandler; -typedef http::server FakeServer; - -// Fake web server that maps paths to response strings. -class FakeHandler { -public: - FakeHandler(std::map paths) : paths_(paths) {} - void operator() (FakeServer::request const &request, - FakeServer::connection_ptr connection) { - auto it = paths_.find(request.destination); - if (it != paths_.end()) { - connection->set_status(FakeServer::connection::ok); - connection->set_headers(std::map({ - {"Content-Type", "text/plain"}, - })); - connection->write(it->second); - } else { - // Note: We have to set headers; otherwise, an exception is thrown. - connection->set_status(FakeServer::connection::not_found); - connection->set_headers(std::map({})); - } +// Starts a server in a separate thread, allowing it to choose an +// available port. +class FakeServerThread { + public: + FakeServerThread() + : server_(Server::options(handler_).address("127.0.0.1").port("")) { + server_.listen(); + thread_ = std::thread([this] { server_.run(); }); + } + + void SetResponse(const std::string& path, const std::string& response) { + handler_.path_responses[path] = response; + } + + std::string HostPort() { + return server_.address() + ":" + server_.port(); } -private: - std::map paths_; + + ~FakeServerThread() { + server_.stop(); + thread_.join(); + } + + private: + struct Handler; + typedef boost::network::http::server Server; + + // Handler that maps paths to response strings. + struct Handler { + void 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({ + {"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::map path_responses; + }; + + Handler handler_; + Server server_; + std::thread thread_; }; } // namespace @@ -101,7 +130,6 @@ TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsSucceeds) { TemporaryFile credentials_file( std::string(test_info_->name()) + "_creds.json", "{\"client_email\":\"user@example.com\",\"private_key\":\"some_key\"}"); - std::string cfg; Configuration config(std::istringstream( "CredentialsFile: '" + credentials_file.FullPath().native() + "'\n" )); @@ -131,44 +159,15 @@ TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsCaches) { } TEST_F(EnvironmentTest, GetMetadataStringWithFakeServer) { - // Start a server in a separate thread, and allow it to choose an - // available port. - FakeHandler handler(std::map({{"/a/b/c", "hello"}})); - FakeServer::options options(handler); - FakeServer server(options.address("127.0.0.1").port("")); - server.listen(); - std::thread t1([&server] { server.run(); }); - - // Set the config to use the local address and port. - TemporaryFile credentials_file( - std::string(test_info_->name()) + "_creds.json", - "{\"client_email\":\"user@example.com\",\"private_key\":\"some_key\"}"); - std::string cfg; - Configuration config(std::istringstream( - "CredentialsFile: '" + credentials_file.FullPath().native() + "'\n" - "GceMetadataServerAddress: 'http://127.0.0.1:" + server.port() + "'\n" - )); + FakeServerThread server; + server.SetResponse("/a/b/c", "hello"); + + Configuration config; Environment environment(config); + SetGceMetadataServerAddress(&environment, "http://" + server.HostPort()); EXPECT_EQ("hello", environment.GetMetadataString("/a/b/c")); EXPECT_EQ("", environment.GetMetadataString("/unknown/path")); - - server.stop(); - t1.join(); -} - -TEST_F(EnvironmentTest, GetMetadataStringWithInvalidAddress) { - TemporaryFile credentials_file( - std::string(test_info_->name()) + "_creds.json", - "{\"client_email\":\"user@example.com\",\"private_key\":\"some_key\"}"); - std::string cfg; - Configuration config(std::istringstream( - "CredentialsFile: '" + credentials_file.FullPath().native() + "'\n" - "GceMetadataServerAddress: 'http://invalidaddress'\n" - )); - Environment environment(config); - - EXPECT_EQ("", environment.GetMetadataString("/a/b/c")); } } // namespace google From 3d7c1c4c3e4cb5cd3ccedfd4b4835f485a262aef Mon Sep 17 00:00:00 2001 From: "David B. Tucker" Date: Tue, 29 May 2018 17:55:27 -0400 Subject: [PATCH 3/7] Fix slashes in tests. --- test/environment_unittest.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/environment_unittest.cc b/test/environment_unittest.cc index a066ad92..518d8f36 100644 --- a/test/environment_unittest.cc +++ b/test/environment_unittest.cc @@ -164,10 +164,10 @@ TEST_F(EnvironmentTest, GetMetadataStringWithFakeServer) { Configuration config; Environment environment(config); - SetGceMetadataServerAddress(&environment, "http://" + server.HostPort()); + SetGceMetadataServerAddress(&environment, "http://" + server.HostPort() + "/"); - EXPECT_EQ("hello", environment.GetMetadataString("/a/b/c")); - EXPECT_EQ("", environment.GetMetadataString("/unknown/path")); + EXPECT_EQ("hello", environment.GetMetadataString("a/b/c")); + EXPECT_EQ("", environment.GetMetadataString("unknown/path")); } } // namespace google From ff6fe287dd974ebb6d52e1c7a5e0fd4aae2d526b Mon Sep 17 00:00:00 2001 From: "David B. Tucker" Date: Wed, 30 May 2018 14:49:17 -0400 Subject: [PATCH 4/7] Addressed more feedback. --- src/environment.cc | 10 +++--- src/environment.h | 6 ++-- test/Makefile | 2 +- test/environment_unittest.cc | 64 ++++-------------------------------- test/fake_http_server.cc | 43 ++++++++++++++++++++++++ test/fake_http_server.h | 38 +++++++++++++++++++++ 6 files changed, 95 insertions(+), 68 deletions(-) create mode 100644 test/fake_http_server.cc create mode 100644 test/fake_http_server.h diff --git a/src/environment.cc b/src/environment.cc index 636e5104..a4dd7583 100644 --- a/src/environment.cc +++ b/src/environment.cc @@ -79,13 +79,13 @@ constexpr const char kGceInstanceResourceType[] = "gce_instance"; Environment::Environment(const Configuration& config) : config_(config), - gce_metadata_server_address_(kGceMetadataServerAddress), + 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(gce_metadata_server_address_ + path); + http::client::request request(metadata_server_url_ + path); request << boost::network::header("Metadata-Flavor", "Google"); try { http::client::response response = client.get(request); @@ -100,7 +100,7 @@ std::string Environment::GetMetadataString(const std::string& path) const { } } catch (const boost::system::system_error& e) { LOG(ERROR) << "Exception: " << e.what() - << ": '" << gce_metadata_server_address_ << path << "'"; + << ": '" << metadata_server_url_ << path << "'"; return ""; } } @@ -272,8 +272,8 @@ void Environment::ReadApplicationDefaultCredentials() const { application_default_credentials_read_ = true; } -void Environment::SetGceMetadataServerAddress(const std::string& address) { - gce_metadata_server_address_ = address; +void Environment::SetMetadataServerUrlForTest(const std::string& url) { + metadata_server_url_ = url; } } // google diff --git a/src/environment.h b/src/environment.h index 7101278a..14c1d36b 100644 --- a/src/environment.h +++ b/src/environment.h @@ -48,13 +48,10 @@ class Environment { void ReadApplicationDefaultCredentials() const; - // Used to override server address in tests. - void SetGceMetadataServerAddress(const std::string& address); + void SetMetadataServerUrlForTest(const std::string& url); const Configuration& config_; - std::string gce_metadata_server_address_; - // Cached data. mutable std::mutex mutex_; mutable std::string project_id_; @@ -65,6 +62,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_; }; diff --git a/test/Makefile b/test/Makefile index 5129a492..3ad7a3bc 100644 --- a/test/Makefile +++ b/test/Makefile @@ -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 $@ diff --git a/test/environment_unittest.cc b/test/environment_unittest.cc index 518d8f36..fd4657f8 100644 --- a/test/environment_unittest.cc +++ b/test/environment_unittest.cc @@ -1,10 +1,10 @@ #include "../src/environment.h" +#include "fake_http_server.h" #include "gtest/gtest.h" #include #include #include -#include namespace google { @@ -14,9 +14,9 @@ class EnvironmentTest : public ::testing::Test { environment.ReadApplicationDefaultCredentials(); } - static void SetGceMetadataServerAddress(Environment* environment, - const std::string& address) { - environment->SetGceMetadataServerAddress(address); + static void SetMetadataServerUrlForTest(Environment* environment, + const std::string& url) { + environment->SetMetadataServerUrlForTest(url); } }; @@ -50,58 +50,6 @@ class TemporaryFile { boost::filesystem::path path_; }; -// Starts a server in a separate thread, allowing it to choose an -// available port. -class FakeServerThread { - public: - FakeServerThread() - : server_(Server::options(handler_).address("127.0.0.1").port("")) { - server_.listen(); - thread_ = std::thread([this] { server_.run(); }); - } - - void SetResponse(const std::string& path, const std::string& response) { - handler_.path_responses[path] = response; - } - - std::string HostPort() { - return server_.address() + ":" + server_.port(); - } - - ~FakeServerThread() { - server_.stop(); - thread_.join(); - } - - private: - struct Handler; - typedef boost::network::http::server Server; - - // Handler that maps paths to response strings. - struct Handler { - void 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({ - {"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::map path_responses; - }; - - Handler handler_; - Server server_; - std::thread thread_; -}; - } // namespace TEST(TemporaryFile, Basic) { @@ -159,12 +107,12 @@ TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsCaches) { } TEST_F(EnvironmentTest, GetMetadataStringWithFakeServer) { - FakeServerThread server; + testing::FakeServerThread server; server.SetResponse("/a/b/c", "hello"); Configuration config; Environment environment(config); - SetGceMetadataServerAddress(&environment, "http://" + server.HostPort() + "/"); + SetMetadataServerUrlForTest(&environment, server.GetUrl()); EXPECT_EQ("hello", environment.GetMetadataString("a/b/c")); EXPECT_EQ("", environment.GetMetadataString("unknown/path")); diff --git a/test/fake_http_server.cc b/test/fake_http_server.cc new file mode 100644 index 00000000..dfee7d41 --- /dev/null +++ b/test/fake_http_server.cc @@ -0,0 +1,43 @@ +#include "fake_http_server.h" + +namespace google { +namespace testing { + +FakeServerThread::FakeServerThread() + : server_(Server::options(handler_).address("127.0.0.1").port("")) { + server_.listen(); + server_thread_ = std::thread([this] { server_.run(); }); +} + +FakeServerThread::~FakeServerThread() { + server_.stop(); + server_thread_.join(); +} + +std::string FakeServerThread::GetUrl() { + return std::string("http://") + server_.address() + ":" + server_.port() + "/"; +} + +void FakeServerThread::SetResponse(const std::string& path, + const std::string& response) { + handler_.path_responses[path] = response; +} + +void FakeServerThread::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({ + {"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()); + } +} + +} // testing +} // google diff --git a/test/fake_http_server.h b/test/fake_http_server.h new file mode 100644 index 00000000..7d388ec6 --- /dev/null +++ b/test/fake_http_server.h @@ -0,0 +1,38 @@ +#ifndef FAKE_HTTP_SERVER_H_ +#define FAKE_HTTP_SERVER_H_ + +#include + +namespace google { +namespace testing { + +// Starts a server in a separate thread, allowing it to choose an +// available port. +class FakeServerThread { + public: + FakeServerThread(); + ~FakeServerThread(); + + std::string GetUrl(); + void SetResponse(const std::string& path, const std::string& response); + + private: + struct Handler; + typedef boost::network::http::server Server; + + // Handler that maps paths to response strings. + struct Handler { + void operator() (Server::request const &request, + Server::connection_ptr connection); + std::map path_responses; + }; + + Handler handler_; + Server server_; + std::thread server_thread_; +}; + +} // testing +} // google + +#endif // FAKE_HTTP_SERVER_H_ From 639adf7586a57e20c4e44dbbeb76b3578e119e27 Mon Sep 17 00:00:00 2001 From: "David B. Tucker" Date: Wed, 30 May 2018 15:02:25 -0400 Subject: [PATCH 5/7] A few comments I missed. --- src/environment.cc | 4 ---- src/environment.h | 4 +++- test/environment_unittest.cc | 2 +- test/fake_http_server.cc | 14 +++++++------- test/fake_http_server.h | 6 +++--- 5 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/environment.cc b/src/environment.cc index a4dd7583..c3d2e843 100644 --- a/src/environment.cc +++ b/src/environment.cc @@ -272,8 +272,4 @@ void Environment::ReadApplicationDefaultCredentials() const { application_default_credentials_read_ = true; } -void Environment::SetMetadataServerUrlForTest(const std::string& url) { - metadata_server_url_ = url; -} - } // google diff --git a/src/environment.h b/src/environment.h index 14c1d36b..8c547381 100644 --- a/src/environment.h +++ b/src/environment.h @@ -48,7 +48,9 @@ class Environment { void ReadApplicationDefaultCredentials() const; - void SetMetadataServerUrlForTest(const std::string& url); + void SetMetadataServerUrlForTest(const std::string& url) { + metadata_server_url_ = url; + } const Configuration& config_; diff --git a/test/environment_unittest.cc b/test/environment_unittest.cc index fd4657f8..8707ca24 100644 --- a/test/environment_unittest.cc +++ b/test/environment_unittest.cc @@ -107,7 +107,7 @@ TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsCaches) { } TEST_F(EnvironmentTest, GetMetadataStringWithFakeServer) { - testing::FakeServerThread server; + testing::FakeServer server; server.SetResponse("/a/b/c", "hello"); Configuration config; diff --git a/test/fake_http_server.cc b/test/fake_http_server.cc index dfee7d41..ae348c13 100644 --- a/test/fake_http_server.cc +++ b/test/fake_http_server.cc @@ -3,28 +3,28 @@ namespace google { namespace testing { -FakeServerThread::FakeServerThread() +FakeServer::FakeServer() : server_(Server::options(handler_).address("127.0.0.1").port("")) { server_.listen(); server_thread_ = std::thread([this] { server_.run(); }); } -FakeServerThread::~FakeServerThread() { +FakeServer::~FakeServer() { server_.stop(); server_thread_.join(); } -std::string FakeServerThread::GetUrl() { +std::string FakeServer::GetUrl() { return std::string("http://") + server_.address() + ":" + server_.port() + "/"; } -void FakeServerThread::SetResponse(const std::string& path, - const std::string& response) { +void FakeServer::SetResponse(const std::string& path, + const std::string& response) { handler_.path_responses[path] = response; } -void FakeServerThread::Handler::operator() (Server::request const &request, - Server::connection_ptr connection) { +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); diff --git a/test/fake_http_server.h b/test/fake_http_server.h index 7d388ec6..a8fefc2c 100644 --- a/test/fake_http_server.h +++ b/test/fake_http_server.h @@ -8,10 +8,10 @@ namespace testing { // Starts a server in a separate thread, allowing it to choose an // available port. -class FakeServerThread { +class FakeServer { public: - FakeServerThread(); - ~FakeServerThread(); + FakeServer(); + ~FakeServer(); std::string GetUrl(); void SetResponse(const std::string& path, const std::string& response); From 2b5db18de690420c271bd7afda3fbdad796d0dd5 Mon Sep 17 00:00:00 2001 From: "David B. Tucker" Date: Wed, 30 May 2018 17:03:26 -0400 Subject: [PATCH 6/7] More feedback. --- src/environment.h | 1 + test/environment_unittest.cc | 2 +- test/fake_http_server.cc | 10 +++++++--- test/fake_http_server.h | 4 ++-- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/environment.h b/src/environment.h index 8c547381..2ca4c079 100644 --- a/src/environment.h +++ b/src/environment.h @@ -48,6 +48,7 @@ class Environment { void ReadApplicationDefaultCredentials() const; + // The url must end in a '/'. void SetMetadataServerUrlForTest(const std::string& url) { metadata_server_url_ = url; } diff --git a/test/environment_unittest.cc b/test/environment_unittest.cc index 8707ca24..a76e530b 100644 --- a/test/environment_unittest.cc +++ b/test/environment_unittest.cc @@ -106,7 +106,7 @@ TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsCaches) { EXPECT_EQ("some_key", environment.CredentialsPrivateKey()); } -TEST_F(EnvironmentTest, GetMetadataStringWithFakeServer) { +TEST_F(EnvironmentTest, GetMetadataString) { testing::FakeServer server; server.SetResponse("/a/b/c", "hello"); diff --git a/test/fake_http_server.cc b/test/fake_http_server.cc index ae348c13..fed3fb60 100644 --- a/test/fake_http_server.cc +++ b/test/fake_http_server.cc @@ -3,6 +3,8 @@ namespace google { namespace testing { +// Note: An empty port selects a random available port (this behavior +// is not documented). FakeServer::FakeServer() : server_(Server::options(handler_).address("127.0.0.1").port("")) { server_.listen(); @@ -15,7 +17,9 @@ FakeServer::~FakeServer() { } std::string FakeServer::GetUrl() { - return std::string("http://") + server_.address() + ":" + server_.port() + "/"; + 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, @@ -23,8 +27,8 @@ void FakeServer::SetResponse(const std::string& path, handler_.path_responses[path] = response; } -void FakeServer::Handler::operator() (Server::request const &request, - Server::connection_ptr connection) { +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); diff --git a/test/fake_http_server.h b/test/fake_http_server.h index a8fefc2c..7eb5881a 100644 --- a/test/fake_http_server.h +++ b/test/fake_http_server.h @@ -22,8 +22,8 @@ class FakeServer { // Handler that maps paths to response strings. struct Handler { - void operator() (Server::request const &request, - Server::connection_ptr connection); + void operator()(Server::request const &request, + Server::connection_ptr connection); std::map path_responses; }; From a86a3736412a1a8903b28e59ae68bf4d9f831951 Mon Sep 17 00:00:00 2001 From: "David B. Tucker" Date: Mon, 4 Jun 2018 12:14:38 -0400 Subject: [PATCH 7/7] Move comment. --- test/fake_http_server.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/fake_http_server.cc b/test/fake_http_server.cc index fed3fb60..d5980dec 100644 --- a/test/fake_http_server.cc +++ b/test/fake_http_server.cc @@ -3,9 +3,9 @@ namespace google { namespace testing { -// Note: An empty port selects a random available port (this behavior -// is not documented). FakeServer::FakeServer() + // Note: An empty port selects a random available port (this behavior + // is not documented). : server_(Server::options(handler_).address("127.0.0.1").port("")) { server_.listen(); server_thread_ = std::thread([this] { server_.run(); });