From a7b35b20fa21bd99b5d3c37fdc65042143b52e19 Mon Sep 17 00:00:00 2001 From: Andrew Emil Date: Tue, 13 Mar 2018 11:14:14 -0400 Subject: [PATCH 1/7] Support configuration testing --- src/configuration.cc | 8 +++-- src/configuration.h | 6 ++-- test/Makefile | 32 +++++++++++++++--- test/configuration_unittest.cc | 61 ++++++++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 9 deletions(-) create mode 100644 test/configuration_unittest.cc diff --git a/src/configuration.cc b/src/configuration.cc index ca97668a..729885f2 100644 --- a/src/configuration.cc +++ b/src/configuration.cc @@ -20,8 +20,6 @@ #include #include -#include - namespace google { namespace { @@ -132,10 +130,14 @@ int MetadataAgentConfiguration::ParseArguments(int ac, char** av) { } void MetadataAgentConfiguration::ParseConfigFile(const std::string& filename) { - std::lock_guard lock(mutex_); if (filename.empty()) return; YAML::Node config = YAML::LoadFile(filename); + ParseYamlConfig(config); +} + +void MetadataAgentConfiguration::ParseYamlConfig(YAML::Node config) { + std::lock_guard lock(mutex_); project_id_ = config["ProjectId"].as(kDefaultProjectId); credentials_file_ = diff --git a/src/configuration.h b/src/configuration.h index 4cd91f52..ef62026d 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -19,12 +19,16 @@ #include #include +#include + namespace google { class MetadataAgentConfiguration { public: MetadataAgentConfiguration(); int ParseArguments(int ac, char** av); + void ParseConfigFile(const std::string& filename); + void ParseYamlConfig(YAML::Node config); // Shared configuration. const std::string& ProjectId() const { @@ -139,8 +143,6 @@ class MetadataAgentConfiguration { } private: - void ParseConfigFile(const std::string& filename); - mutable std::mutex mutex_; std::string project_id_; std::string credentials_file_; diff --git a/test/Makefile b/test/Makefile index 5b33fc2a..b89cc2cb 100644 --- a/test/Makefile +++ b/test/Makefile @@ -9,10 +9,16 @@ GTEST_SRCS_=$(GTEST_SOURCEDIR)/*.cc $(GTEST_SOURCEDIR)/*.h $(GTEST_HEADERS) # TODO: Factor out the common variables. CPP_NETLIB_DIR=$(LIBDIR)/cpp-netlib YAML_CPP_DIR=$(LIBDIR)/yaml-cpp +YAML_CPP_LIBDIR=$(YAML_CPP_DIR) +SUBMODULE_DIRS=$(CPP_NETLIB_DIR) $(YAML_CPP_DIR) +YAML_CPP_LIBS=$(YAML_CPP_LIBDIR)/libyaml-cpp.a + +CMAKE=cmake CPPFLAGS+=-isystem $(GTEST_DIR)/include -CXXFLAGS=-std=c++11 -g -pthread -LDLIBS=-lpthread +CXXFLAGS=-std=c++11 -g -pthread -I$(YAML_CPP_DIR)/include +LDLIBS=-lpthread -lboost_program_options -lyaml-cpp +LDFLAGS=-L$(YAML_CPP_LIBDIR) # Where to find code under test. SRC_DIR=../src @@ -22,7 +28,8 @@ TEST_SOURCES=$(wildcard $(TEST_DIR)/*_unittest.cc) TEST_OBJS=$(TEST_SOURCES:$(TEST_DIR)/%.cc=%.o) TESTS=\ format_unittest \ - base64_unittest + base64_unittest \ + configuration_unittest GTEST_LIB=gtest_lib.a @@ -47,6 +54,20 @@ init-submodules: git submodule update --init $(GTEST_MODULE) touch init-submodules +$(YAML_CPP_DIR)/Makefile: init-submodules + cd $(YAML_CPP_DIR) && \ + $(CMAKE) -DMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_FLAGS=-std=c++11 \ + -DMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ \ + -DYAML_CPP_BUILD_TOOLS=OFF + +$(YAML_CPP_LIBS): build-yaml-cpp + +build-yaml-cpp: $(YAML_CPP_DIR)/Makefile + cd $(YAML_CPP_DIR) && \ + $(MAKE) + touch build-yaml-cpp + +yaml-cpp: $(YAML_CPP_LIBS) $(SRC_DIR)/%.o: $(SRC_DIR)/%.cc cd $(SRC_DIR) && $(MAKE) $(@:$(SRC_DIR)/%=%) @@ -67,4 +88,7 @@ format_unittest: $(GTEST_LIB) format_unittest.o $(SRC_DIR)/format.o base64_unittest: $(GTEST_LIB) base64_unittest.o $(SRC_DIR)/base64.o $(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@ -.PHONY: all test clean purge +configuration_unittest: $(GTEST_LIB) configuration_unittest.o $(SRC_DIR)/configuration.o + $(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@ + +.PHONY: all test clean purge yaml-cpp diff --git a/test/configuration_unittest.cc b/test/configuration_unittest.cc new file mode 100644 index 00000000..e70296b0 --- /dev/null +++ b/test/configuration_unittest.cc @@ -0,0 +1,61 @@ +#include "../src/configuration.h" +#include "gtest/gtest.h" + +namespace { + +void TestDefaultConfig(google::MetadataAgentConfiguration& config) { + EXPECT_EQ("", config.ProjectId()); + EXPECT_EQ("", config.CredentialsFile()); + EXPECT_EQ(3, config.MetadataApiNumThreads()); + EXPECT_EQ(8000, config.MetadataApiPort()); + EXPECT_EQ(".", config.MetadataApiResourceTypeSeparator()); + EXPECT_EQ(60, config.MetadataReporterIntervalSeconds()); + EXPECT_EQ(false, config.MetadataReporterPurgeDeleted()); + EXPECT_EQ( + "https://stackdriver.googleapis.com/" + "v1beta2/projects/{{project_id}}/resourceMetadata:batchUpdate", + config.MetadataIngestionEndpointFormat()); + EXPECT_EQ(8*1024*1024, config.MetadataIngestionRequestSizeLimitBytes()); + EXPECT_EQ("0.1", config.MetadataIngestionRawContentVersion()); + EXPECT_EQ(60*60, config.InstanceUpdaterIntervalSeconds()); + EXPECT_EQ("", config.InstanceResourceType()); + EXPECT_EQ(60, config.DockerUpdaterIntervalSeconds()); + EXPECT_EQ("unix://%2Fvar%2Frun%2Fdocker.sock/", config.DockerEndpointHost()); + EXPECT_EQ("1.23", config.DockerApiVersion()); + EXPECT_EQ("limit=30", config.DockerContainerFilter()); + EXPECT_EQ(0, config.KubernetesUpdaterIntervalSeconds()); + EXPECT_EQ("https://kubernetes.default.svc", config.KubernetesEndpointHost()); + EXPECT_EQ("", config.KubernetesPodLabelSelector()); + EXPECT_EQ("", config.KubernetesClusterName()); + EXPECT_EQ("", config.KubernetesClusterLocation()); + EXPECT_EQ("", config.KubernetesNodeName()); + EXPECT_EQ(true, config.KubernetesUseWatch()); + EXPECT_EQ("", config.InstanceId()); + EXPECT_EQ("", config.InstanceZone()); +} + +TEST(ConfigurationTest, EmptyTest) { + google::MetadataAgentConfiguration config; + TestDefaultConfig(config); +} + +TEST(ConfigurationTest, BlankYamlTest) { + YAML::Node node; + google::MetadataAgentConfiguration config; + config.ParseYamlConfig(node); + TestDefaultConfig(config); +} + +TEST(ConfigurationTest, SpecificTest) { + YAML::Node node = YAML::Load( + "ProjectId: TestProjectId\n" + "MetadataApiNumThreads: 13\n" + "MetadataReporterPurgeDeleted: true"); + google::MetadataAgentConfiguration config; + config.ParseYamlConfig(node); + EXPECT_EQ("TestProjectId", config.ProjectId()); + EXPECT_EQ(13, config.MetadataApiNumThreads()); + EXPECT_EQ(true, config.MetadataReporterPurgeDeleted()); +} + +} // namespace From b0c64131fb58e6b3e22c64cd3a8d073e174c08ef Mon Sep 17 00:00:00 2001 From: Andrew Emil Date: Thu, 15 Mar 2018 11:08:32 -0400 Subject: [PATCH 2/7] Changes based on PR #80 --- src/configuration.cc | 11 +++- src/configuration.h | 7 +-- test/Makefile | 10 +--- test/configuration_unittest.cc | 104 ++++++++++++++++++++------------- 4 files changed, 75 insertions(+), 57 deletions(-) diff --git a/src/configuration.cc b/src/configuration.cc index 729885f2..81280ebf 100644 --- a/src/configuration.cc +++ b/src/configuration.cc @@ -19,6 +19,9 @@ #include #include #include +#include + +#include namespace google { @@ -131,12 +134,14 @@ int MetadataAgentConfiguration::ParseArguments(int ac, char** av) { void MetadataAgentConfiguration::ParseConfigFile(const std::string& filename) { if (filename.empty()) return; + std::ifstream ifs; + ifs.open (filename, std::ifstream::in); - YAML::Node config = YAML::LoadFile(filename); - ParseYamlConfig(config); + ParseConfiguration(ifs); } -void MetadataAgentConfiguration::ParseYamlConfig(YAML::Node config) { +void MetadataAgentConfiguration::ParseConfiguration(std::istream& input) { + YAML::Node config = YAML::Load(input); std::lock_guard lock(mutex_); project_id_ = config["ProjectId"].as(kDefaultProjectId); diff --git a/src/configuration.h b/src/configuration.h index ef62026d..d3b2d003 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -19,16 +19,12 @@ #include #include -#include - namespace google { class MetadataAgentConfiguration { public: MetadataAgentConfiguration(); int ParseArguments(int ac, char** av); - void ParseConfigFile(const std::string& filename); - void ParseYamlConfig(YAML::Node config); // Shared configuration. const std::string& ProjectId() const { @@ -143,6 +139,9 @@ class MetadataAgentConfiguration { } private: + friend class MetadataAgentConfigurationTest; + void ParseConfigFile(const std::string& filename); + void ParseConfiguration(std::istream& input); mutable std::mutex mutex_; std::string project_id_; std::string credentials_file_; diff --git a/test/Makefile b/test/Makefile index 49f17d68..eef7eefb 100644 --- a/test/Makefile +++ b/test/Makefile @@ -10,7 +10,6 @@ GTEST_SRCS_=$(GTEST_SOURCEDIR)/*.cc $(GTEST_SOURCEDIR)/*.h $(GTEST_HEADERS) CPP_NETLIB_DIR=$(LIBDIR)/cpp-netlib YAML_CPP_DIR=$(LIBDIR)/yaml-cpp YAML_CPP_LIBDIR=$(YAML_CPP_DIR) -SUBMODULE_DIRS=$(CPP_NETLIB_DIR) $(YAML_CPP_DIR) YAML_CPP_LIBS=$(YAML_CPP_LIBDIR)/libyaml-cpp.a @@ -56,13 +55,8 @@ init-submodules: git submodule update --init $(GTEST_MODULE) touch init-submodules -$(YAML_CPP_DIR)/Makefile: init-submodules - cd $(YAML_CPP_DIR) && \ - $(CMAKE) -DMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_FLAGS=-std=c++11 \ - -DMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ \ - -DYAML_CPP_BUILD_TOOLS=OFF - -$(YAML_CPP_LIBS): build-yaml-cpp +$(YAML_CPP_LIBS): + cd $(SRC_DIR) && $(MAKE) $@ build-yaml-cpp: $(YAML_CPP_DIR)/Makefile cd $(YAML_CPP_DIR) && \ diff --git a/test/configuration_unittest.cc b/test/configuration_unittest.cc index e70296b0..6b8a279c 100644 --- a/test/configuration_unittest.cc +++ b/test/configuration_unittest.cc @@ -1,61 +1,81 @@ #include "../src/configuration.h" #include "gtest/gtest.h" -namespace { +namespace google { -void TestDefaultConfig(google::MetadataAgentConfiguration& config) { - EXPECT_EQ("", config.ProjectId()); - EXPECT_EQ("", config.CredentialsFile()); - EXPECT_EQ(3, config.MetadataApiNumThreads()); - EXPECT_EQ(8000, config.MetadataApiPort()); - EXPECT_EQ(".", config.MetadataApiResourceTypeSeparator()); - EXPECT_EQ(60, config.MetadataReporterIntervalSeconds()); - EXPECT_EQ(false, config.MetadataReporterPurgeDeleted()); - EXPECT_EQ( - "https://stackdriver.googleapis.com/" - "v1beta2/projects/{{project_id}}/resourceMetadata:batchUpdate", - config.MetadataIngestionEndpointFormat()); - EXPECT_EQ(8*1024*1024, config.MetadataIngestionRequestSizeLimitBytes()); - EXPECT_EQ("0.1", config.MetadataIngestionRawContentVersion()); - EXPECT_EQ(60*60, config.InstanceUpdaterIntervalSeconds()); - EXPECT_EQ("", config.InstanceResourceType()); - EXPECT_EQ(60, config.DockerUpdaterIntervalSeconds()); - EXPECT_EQ("unix://%2Fvar%2Frun%2Fdocker.sock/", config.DockerEndpointHost()); - EXPECT_EQ("1.23", config.DockerApiVersion()); - EXPECT_EQ("limit=30", config.DockerContainerFilter()); - EXPECT_EQ(0, config.KubernetesUpdaterIntervalSeconds()); - EXPECT_EQ("https://kubernetes.default.svc", config.KubernetesEndpointHost()); - EXPECT_EQ("", config.KubernetesPodLabelSelector()); - EXPECT_EQ("", config.KubernetesClusterName()); - EXPECT_EQ("", config.KubernetesClusterLocation()); - EXPECT_EQ("", config.KubernetesNodeName()); - EXPECT_EQ(true, config.KubernetesUseWatch()); - EXPECT_EQ("", config.InstanceId()); - EXPECT_EQ("", config.InstanceZone()); -} +class MetadataAgentConfigurationTest : public ::testing::Test { + public: + void CallParseConfiguration(MetadataAgentConfiguration& config, std::istream& stream) { + config.ParseConfiguration(stream); + } + + static void SetUpTestCase() {} + static void TearDownTestCase() {} -TEST(ConfigurationTest, EmptyTest) { - google::MetadataAgentConfiguration config; + void TestDefaultConfig(MetadataAgentConfiguration& config) { + EXPECT_EQ("", config.ProjectId()); + EXPECT_EQ("", config.CredentialsFile()); + EXPECT_EQ(3, config.MetadataApiNumThreads()); + EXPECT_EQ(8000, config.MetadataApiPort()); + EXPECT_EQ(".", config.MetadataApiResourceTypeSeparator()); + EXPECT_EQ(60, config.MetadataReporterIntervalSeconds()); + EXPECT_EQ(false, config.MetadataReporterPurgeDeleted()); + EXPECT_EQ( + "https://stackdriver.googleapis.com/" + "v1beta2/projects/{{project_id}}/resourceMetadata:batchUpdate", + config.MetadataIngestionEndpointFormat()); + EXPECT_EQ(8*1024*1024, config.MetadataIngestionRequestSizeLimitBytes()); + EXPECT_EQ("0.1", config.MetadataIngestionRawContentVersion()); + EXPECT_EQ(60*60, config.InstanceUpdaterIntervalSeconds()); + EXPECT_EQ("", config.InstanceResourceType()); + EXPECT_EQ(60, config.DockerUpdaterIntervalSeconds()); + EXPECT_EQ("unix://%2Fvar%2Frun%2Fdocker.sock/", config.DockerEndpointHost()); + EXPECT_EQ("1.23", config.DockerApiVersion()); + EXPECT_EQ("limit=30", config.DockerContainerFilter()); + EXPECT_EQ(0, config.KubernetesUpdaterIntervalSeconds()); + EXPECT_EQ("https://kubernetes.default.svc", config.KubernetesEndpointHost()); + EXPECT_EQ("", config.KubernetesPodLabelSelector()); + EXPECT_EQ("", config.KubernetesClusterName()); + EXPECT_EQ("", config.KubernetesClusterLocation()); + EXPECT_EQ("", config.KubernetesNodeName()); + EXPECT_EQ(true, config.KubernetesUseWatch()); + EXPECT_EQ("", config.InstanceId()); + EXPECT_EQ("", config.InstanceZone()); + } +}; + +TEST_F(MetadataAgentConfigurationTest, NoConfigTest) { + MetadataAgentConfiguration config; TestDefaultConfig(config); } -TEST(ConfigurationTest, BlankYamlTest) { - YAML::Node node; - google::MetadataAgentConfiguration config; - config.ParseYamlConfig(node); +TEST_F(MetadataAgentConfigurationTest, EmptyConfigTest) { + MetadataAgentConfiguration config; + std::stringstream stream(""); + CallParseConfiguration(config, stream); TestDefaultConfig(config); } -TEST(ConfigurationTest, SpecificTest) { - YAML::Node node = YAML::Load( +TEST_F(MetadataAgentConfigurationTest, PopulatedConfigTest) { + MetadataAgentConfiguration config; + std::stringstream stream( "ProjectId: TestProjectId\n" "MetadataApiNumThreads: 13\n" "MetadataReporterPurgeDeleted: true"); - google::MetadataAgentConfiguration config; - config.ParseYamlConfig(node); + CallParseConfiguration(config, stream); EXPECT_EQ("TestProjectId", config.ProjectId()); EXPECT_EQ(13, config.MetadataApiNumThreads()); EXPECT_EQ(true, config.MetadataReporterPurgeDeleted()); } -} // namespace +TEST_F(MetadataAgentConfigurationTest, CommentSkippedTest) { + MetadataAgentConfiguration config; + std::stringstream stream( + "ProjectId: TestProjectId\n" + "#MetadataApiNumThreads: 13\n" + "MetadataReporterPurgeDeleted: true"); + CallParseConfiguration(config, stream); + EXPECT_EQ(3, config.MetadataApiNumThreads()); +} + +} // google From 6a9cc6961c4eea5d2acacdbf4902c168d8c34bb5 Mon Sep 17 00:00:00 2001 From: Andrew Emil Date: Thu, 15 Mar 2018 18:27:55 -0400 Subject: [PATCH 3/7] Addressed further code review comments on PR #80 --- src/configuration.cc | 3 +- src/configuration.h | 1 + test/Makefile | 13 ++--- test/configuration_unittest.cc | 90 +++++++++++++++++----------------- 4 files changed, 50 insertions(+), 57 deletions(-) diff --git a/src/configuration.cc b/src/configuration.cc index 81280ebf..c0909f01 100644 --- a/src/configuration.cc +++ b/src/configuration.cc @@ -134,9 +134,8 @@ int MetadataAgentConfiguration::ParseArguments(int ac, char** av) { void MetadataAgentConfiguration::ParseConfigFile(const std::string& filename) { if (filename.empty()) return; - std::ifstream ifs; - ifs.open (filename, std::ifstream::in); + std::ifstream ifs(filename); ParseConfiguration(ifs); } diff --git a/src/configuration.h b/src/configuration.h index d3b2d003..4df06d10 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -142,6 +142,7 @@ class MetadataAgentConfiguration { friend class MetadataAgentConfigurationTest; void ParseConfigFile(const std::string& filename); void ParseConfiguration(std::istream& input); + mutable std::mutex mutex_; std::string project_id_; std::string credentials_file_; diff --git a/test/Makefile b/test/Makefile index eef7eefb..d97df0b0 100644 --- a/test/Makefile +++ b/test/Makefile @@ -29,7 +29,7 @@ TESTS=\ base64_unittest \ format_unittest \ base64_unittest \ - configuration_unittest + configuration_unittest \ time_unittest GTEST_LIB=gtest_lib.a @@ -58,13 +58,6 @@ init-submodules: $(YAML_CPP_LIBS): cd $(SRC_DIR) && $(MAKE) $@ -build-yaml-cpp: $(YAML_CPP_DIR)/Makefile - cd $(YAML_CPP_DIR) && \ - $(MAKE) - touch build-yaml-cpp - -yaml-cpp: $(YAML_CPP_LIBS) - $(SRC_DIR)/%.o: $(SRC_DIR)/%.cc cd $(SRC_DIR) && $(MAKE) $(@:$(SRC_DIR)/%=%) @@ -83,9 +76,9 @@ format_unittest: $(GTEST_LIB) format_unittest.o $(SRC_DIR)/format.o $(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@ base64_unittest: $(GTEST_LIB) base64_unittest.o $(SRC_DIR)/base64.o $(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@ -configuration_unittest: $(GTEST_LIB) configuration_unittest.o $(SRC_DIR)/configuration.o +configuration_unittest: $(GTEST_LIB) configuration_unittest.o $(SRC_DIR)/configuration.o $(YAML_CPP_LIBS) $(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@ time_unittest: $(GTEST_LIB) time_unittest.o $(SRC_DIR)/time.o $(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@ -.PHONY: all test clean purge yaml-cpp \ No newline at end of file +.PHONY: all test clean purge yaml-cpp diff --git a/test/configuration_unittest.cc b/test/configuration_unittest.cc index 6b8a279c..82e233dc 100644 --- a/test/configuration_unittest.cc +++ b/test/configuration_unittest.cc @@ -5,77 +5,77 @@ namespace google { class MetadataAgentConfigurationTest : public ::testing::Test { public: - void CallParseConfiguration(MetadataAgentConfiguration& config, std::istream& stream) { + MetadataAgentConfiguration config; + static void ParseConfiguration(MetadataAgentConfiguration& config, const std::string configString) { + std::stringstream stream(configString); config.ParseConfiguration(stream); } - - static void SetUpTestCase() {} - static void TearDownTestCase() {} - - void TestDefaultConfig(MetadataAgentConfiguration& config) { - EXPECT_EQ("", config.ProjectId()); - EXPECT_EQ("", config.CredentialsFile()); - EXPECT_EQ(3, config.MetadataApiNumThreads()); - EXPECT_EQ(8000, config.MetadataApiPort()); - EXPECT_EQ(".", config.MetadataApiResourceTypeSeparator()); - EXPECT_EQ(60, config.MetadataReporterIntervalSeconds()); - EXPECT_EQ(false, config.MetadataReporterPurgeDeleted()); - EXPECT_EQ( - "https://stackdriver.googleapis.com/" - "v1beta2/projects/{{project_id}}/resourceMetadata:batchUpdate", - config.MetadataIngestionEndpointFormat()); - EXPECT_EQ(8*1024*1024, config.MetadataIngestionRequestSizeLimitBytes()); - EXPECT_EQ("0.1", config.MetadataIngestionRawContentVersion()); - EXPECT_EQ(60*60, config.InstanceUpdaterIntervalSeconds()); - EXPECT_EQ("", config.InstanceResourceType()); - EXPECT_EQ(60, config.DockerUpdaterIntervalSeconds()); - EXPECT_EQ("unix://%2Fvar%2Frun%2Fdocker.sock/", config.DockerEndpointHost()); - EXPECT_EQ("1.23", config.DockerApiVersion()); - EXPECT_EQ("limit=30", config.DockerContainerFilter()); - EXPECT_EQ(0, config.KubernetesUpdaterIntervalSeconds()); - EXPECT_EQ("https://kubernetes.default.svc", config.KubernetesEndpointHost()); - EXPECT_EQ("", config.KubernetesPodLabelSelector()); - EXPECT_EQ("", config.KubernetesClusterName()); - EXPECT_EQ("", config.KubernetesClusterLocation()); - EXPECT_EQ("", config.KubernetesNodeName()); - EXPECT_EQ(true, config.KubernetesUseWatch()); - EXPECT_EQ("", config.InstanceId()); - EXPECT_EQ("", config.InstanceZone()); - } }; +static void TestDefaultConfig(MetadataAgentConfiguration& config) { + EXPECT_EQ("", config.ProjectId()); + EXPECT_EQ("", config.CredentialsFile()); + EXPECT_EQ(3, config.MetadataApiNumThreads()); + EXPECT_EQ(8000, config.MetadataApiPort()); + EXPECT_EQ(".", config.MetadataApiResourceTypeSeparator()); + EXPECT_EQ(60, config.MetadataReporterIntervalSeconds()); + EXPECT_EQ(false, config.MetadataReporterPurgeDeleted()); + EXPECT_EQ( + "https://stackdriver.googleapis.com/" + "v1beta2/projects/{{project_id}}/resourceMetadata:batchUpdate", + config.MetadataIngestionEndpointFormat()); + EXPECT_EQ(8*1024*1024, config.MetadataIngestionRequestSizeLimitBytes()); + EXPECT_EQ("0.1", config.MetadataIngestionRawContentVersion()); + EXPECT_EQ(60*60, config.InstanceUpdaterIntervalSeconds()); + EXPECT_EQ("", config.InstanceResourceType()); + EXPECT_EQ(60, config.DockerUpdaterIntervalSeconds()); + EXPECT_EQ("unix://%2Fvar%2Frun%2Fdocker.sock/", config.DockerEndpointHost()); + EXPECT_EQ("1.23", config.DockerApiVersion()); + EXPECT_EQ("limit=30", config.DockerContainerFilter()); + EXPECT_EQ(0, config.KubernetesUpdaterIntervalSeconds()); + EXPECT_EQ("https://kubernetes.default.svc", config.KubernetesEndpointHost()); + EXPECT_EQ("", config.KubernetesPodLabelSelector()); + EXPECT_EQ("", config.KubernetesClusterName()); + EXPECT_EQ("", config.KubernetesClusterLocation()); + EXPECT_EQ("", config.KubernetesNodeName()); + EXPECT_EQ(true, config.KubernetesUseWatch()); + EXPECT_EQ("", config.InstanceId()); + EXPECT_EQ("", config.InstanceZone()); +} + TEST_F(MetadataAgentConfigurationTest, NoConfigTest) { - MetadataAgentConfiguration config; TestDefaultConfig(config); } TEST_F(MetadataAgentConfigurationTest, EmptyConfigTest) { - MetadataAgentConfiguration config; - std::stringstream stream(""); - CallParseConfiguration(config, stream); + ParseConfiguration(config, ""); TestDefaultConfig(config); } TEST_F(MetadataAgentConfigurationTest, PopulatedConfigTest) { - MetadataAgentConfiguration config; - std::stringstream stream( + ParseConfiguration(config, "ProjectId: TestProjectId\n" "MetadataApiNumThreads: 13\n" "MetadataReporterPurgeDeleted: true"); - CallParseConfiguration(config, stream); EXPECT_EQ("TestProjectId", config.ProjectId()); EXPECT_EQ(13, config.MetadataApiNumThreads()); EXPECT_EQ(true, config.MetadataReporterPurgeDeleted()); } TEST_F(MetadataAgentConfigurationTest, CommentSkippedTest) { - MetadataAgentConfiguration config; - std::stringstream stream( + ParseConfiguration(config, "ProjectId: TestProjectId\n" "#MetadataApiNumThreads: 13\n" "MetadataReporterPurgeDeleted: true"); - CallParseConfiguration(config, stream); EXPECT_EQ(3, config.MetadataApiNumThreads()); } +TEST_F(MetadataAgentConfigurationTest, EmptyLineTest) { + ParseConfiguration(config, + "ProjectId: TestProjectId\n\n\n\n" + "MetadataReporterPurgeDeleted: true"); + EXPECT_EQ("TestProjectId", config.ProjectId()); + EXPECT_EQ(true, config.MetadataReporterPurgeDeleted()); +} + } // google From ad502687f92f43b2ef73faa0e030a7adfaf14310 Mon Sep 17 00:00:00 2001 From: Andrew Emil Date: Thu, 15 Mar 2018 19:04:13 -0400 Subject: [PATCH 4/7] More code review on PR #80 --- src/configuration.cc | 4 +- src/configuration.h | 1 + test/Makefile | 4 +- test/configuration_unittest.cc | 87 ++++++++++++++++++---------------- 4 files changed, 50 insertions(+), 46 deletions(-) diff --git a/src/configuration.cc b/src/configuration.cc index c0909f01..796b6020 100644 --- a/src/configuration.cc +++ b/src/configuration.cc @@ -135,8 +135,8 @@ int MetadataAgentConfiguration::ParseArguments(int ac, char** av) { void MetadataAgentConfiguration::ParseConfigFile(const std::string& filename) { if (filename.empty()) return; - std::ifstream ifs(filename); - ParseConfiguration(ifs); + std::ifstream input(filename); + ParseConfiguration(input); } void MetadataAgentConfiguration::ParseConfiguration(std::istream& input) { diff --git a/src/configuration.h b/src/configuration.h index 4df06d10..cc4dbb94 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -140,6 +140,7 @@ class MetadataAgentConfiguration { private: friend class MetadataAgentConfigurationTest; + void ParseConfigFile(const std::string& filename); void ParseConfiguration(std::istream& input); diff --git a/test/Makefile b/test/Makefile index d97df0b0..64e9e719 100644 --- a/test/Makefile +++ b/test/Makefile @@ -76,9 +76,9 @@ format_unittest: $(GTEST_LIB) format_unittest.o $(SRC_DIR)/format.o $(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@ base64_unittest: $(GTEST_LIB) base64_unittest.o $(SRC_DIR)/base64.o $(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@ -configuration_unittest: $(GTEST_LIB) configuration_unittest.o $(SRC_DIR)/configuration.o $(YAML_CPP_LIBS) +configuration_unittest: $(GTEST_LIB) $(YAML_CPP_LIBS) configuration_unittest.o $(SRC_DIR)/configuration.o $(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@ time_unittest: $(GTEST_LIB) time_unittest.o $(SRC_DIR)/time.o $(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@ -.PHONY: all test clean purge yaml-cpp +.PHONY: all test clean purge diff --git a/test/configuration_unittest.cc b/test/configuration_unittest.cc index 82e233dc..03ecac20 100644 --- a/test/configuration_unittest.cc +++ b/test/configuration_unittest.cc @@ -4,56 +4,57 @@ namespace google { class MetadataAgentConfigurationTest : public ::testing::Test { - public: - MetadataAgentConfiguration config; - static void ParseConfiguration(MetadataAgentConfiguration& config, const std::string configString) { + protected: + void ParseConfiguration(const std::string& configString) { std::stringstream stream(configString); config.ParseConfiguration(stream); } -}; -static void TestDefaultConfig(MetadataAgentConfiguration& config) { - EXPECT_EQ("", config.ProjectId()); - EXPECT_EQ("", config.CredentialsFile()); - EXPECT_EQ(3, config.MetadataApiNumThreads()); - EXPECT_EQ(8000, config.MetadataApiPort()); - EXPECT_EQ(".", config.MetadataApiResourceTypeSeparator()); - EXPECT_EQ(60, config.MetadataReporterIntervalSeconds()); - EXPECT_EQ(false, config.MetadataReporterPurgeDeleted()); - EXPECT_EQ( - "https://stackdriver.googleapis.com/" - "v1beta2/projects/{{project_id}}/resourceMetadata:batchUpdate", - config.MetadataIngestionEndpointFormat()); - EXPECT_EQ(8*1024*1024, config.MetadataIngestionRequestSizeLimitBytes()); - EXPECT_EQ("0.1", config.MetadataIngestionRawContentVersion()); - EXPECT_EQ(60*60, config.InstanceUpdaterIntervalSeconds()); - EXPECT_EQ("", config.InstanceResourceType()); - EXPECT_EQ(60, config.DockerUpdaterIntervalSeconds()); - EXPECT_EQ("unix://%2Fvar%2Frun%2Fdocker.sock/", config.DockerEndpointHost()); - EXPECT_EQ("1.23", config.DockerApiVersion()); - EXPECT_EQ("limit=30", config.DockerContainerFilter()); - EXPECT_EQ(0, config.KubernetesUpdaterIntervalSeconds()); - EXPECT_EQ("https://kubernetes.default.svc", config.KubernetesEndpointHost()); - EXPECT_EQ("", config.KubernetesPodLabelSelector()); - EXPECT_EQ("", config.KubernetesClusterName()); - EXPECT_EQ("", config.KubernetesClusterLocation()); - EXPECT_EQ("", config.KubernetesNodeName()); - EXPECT_EQ(true, config.KubernetesUseWatch()); - EXPECT_EQ("", config.InstanceId()); - EXPECT_EQ("", config.InstanceZone()); -} + void VerifyDefaultConfig() const { + EXPECT_EQ("", config.ProjectId()); + EXPECT_EQ("", config.CredentialsFile()); + EXPECT_EQ(3, config.MetadataApiNumThreads()); + EXPECT_EQ(8000, config.MetadataApiPort()); + EXPECT_EQ(".", config.MetadataApiResourceTypeSeparator()); + EXPECT_EQ(60, config.MetadataReporterIntervalSeconds()); + EXPECT_EQ(false, config.MetadataReporterPurgeDeleted()); + EXPECT_EQ( + "https://stackdriver.googleapis.com/" + "v1beta2/projects/{{project_id}}/resourceMetadata:batchUpdate", + config.MetadataIngestionEndpointFormat()); + EXPECT_EQ(8*1024*1024, config.MetadataIngestionRequestSizeLimitBytes()); + EXPECT_EQ("0.1", config.MetadataIngestionRawContentVersion()); + EXPECT_EQ(60*60, config.InstanceUpdaterIntervalSeconds()); + EXPECT_EQ("", config.InstanceResourceType()); + EXPECT_EQ(60, config.DockerUpdaterIntervalSeconds()); + EXPECT_EQ("unix://%2Fvar%2Frun%2Fdocker.sock/", config.DockerEndpointHost()); + EXPECT_EQ("1.23", config.DockerApiVersion()); + EXPECT_EQ("limit=30", config.DockerContainerFilter()); + EXPECT_EQ(0, config.KubernetesUpdaterIntervalSeconds()); + EXPECT_EQ("https://kubernetes.default.svc", config.KubernetesEndpointHost()); + EXPECT_EQ("", config.KubernetesPodLabelSelector()); + EXPECT_EQ("", config.KubernetesClusterName()); + EXPECT_EQ("", config.KubernetesClusterLocation()); + EXPECT_EQ("", config.KubernetesNodeName()); + EXPECT_EQ(true, config.KubernetesUseWatch()); + EXPECT_EQ("", config.InstanceId()); + EXPECT_EQ("", config.InstanceZone()); + } + + MetadataAgentConfiguration config; +}; TEST_F(MetadataAgentConfigurationTest, NoConfigTest) { - TestDefaultConfig(config); + VerifyDefaultConfig(); } TEST_F(MetadataAgentConfigurationTest, EmptyConfigTest) { - ParseConfiguration(config, ""); - TestDefaultConfig(config); + ParseConfiguration(""); + VerifyDefaultConfig(); } TEST_F(MetadataAgentConfigurationTest, PopulatedConfigTest) { - ParseConfiguration(config, + ParseConfiguration( "ProjectId: TestProjectId\n" "MetadataApiNumThreads: 13\n" "MetadataReporterPurgeDeleted: true"); @@ -63,16 +64,18 @@ TEST_F(MetadataAgentConfigurationTest, PopulatedConfigTest) { } TEST_F(MetadataAgentConfigurationTest, CommentSkippedTest) { - ParseConfiguration(config, + ParseConfiguration( "ProjectId: TestProjectId\n" "#MetadataApiNumThreads: 13\n" "MetadataReporterPurgeDeleted: true"); EXPECT_EQ(3, config.MetadataApiNumThreads()); } -TEST_F(MetadataAgentConfigurationTest, EmptyLineTest) { - ParseConfiguration(config, - "ProjectId: TestProjectId\n\n\n\n" +TEST_F(MetadataAgentConfigurationTest, BlankLineTest) { + ParseConfiguration( + "ProjectId: TestProjectId\n" + "\n" + "\n" "MetadataReporterPurgeDeleted: true"); EXPECT_EQ("TestProjectId", config.ProjectId()); EXPECT_EQ(true, config.MetadataReporterPurgeDeleted()); From 4b55cda546b32ac183ea19c9ea254f40a0a57754 Mon Sep 17 00:00:00 2001 From: Andrew Emil Date: Thu, 15 Mar 2018 19:26:29 -0400 Subject: [PATCH 5/7] Addressed futher code review comments from PR #80, as well as linting errors --- test/configuration_unittest.cc | 37 +++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/test/configuration_unittest.cc b/test/configuration_unittest.cc index 03ecac20..7d0ce396 100644 --- a/test/configuration_unittest.cc +++ b/test/configuration_unittest.cc @@ -5,8 +5,8 @@ namespace google { class MetadataAgentConfigurationTest : public ::testing::Test { protected: - void ParseConfiguration(const std::string& configString) { - std::stringstream stream(configString); + void ParseConfiguration(const std::string& input) { + std::stringstream stream(input); config.ParseConfiguration(stream); } @@ -21,17 +21,23 @@ class MetadataAgentConfigurationTest : public ::testing::Test { EXPECT_EQ( "https://stackdriver.googleapis.com/" "v1beta2/projects/{{project_id}}/resourceMetadata:batchUpdate", - config.MetadataIngestionEndpointFormat()); + config.MetadataIngestionEndpointFormat() + ); EXPECT_EQ(8*1024*1024, config.MetadataIngestionRequestSizeLimitBytes()); EXPECT_EQ("0.1", config.MetadataIngestionRawContentVersion()); EXPECT_EQ(60*60, config.InstanceUpdaterIntervalSeconds()); EXPECT_EQ("", config.InstanceResourceType()); EXPECT_EQ(60, config.DockerUpdaterIntervalSeconds()); - EXPECT_EQ("unix://%2Fvar%2Frun%2Fdocker.sock/", config.DockerEndpointHost()); + EXPECT_EQ( + "unix://%2Fvar%2Frun%2Fdocker.sock/", + config.DockerEndpointHost() + ); EXPECT_EQ("1.23", config.DockerApiVersion()); EXPECT_EQ("limit=30", config.DockerContainerFilter()); EXPECT_EQ(0, config.KubernetesUpdaterIntervalSeconds()); - EXPECT_EQ("https://kubernetes.default.svc", config.KubernetesEndpointHost()); + EXPECT_EQ("https://kubernetes.default.svc", + config.KubernetesEndpointHost() + ); EXPECT_EQ("", config.KubernetesPodLabelSelector()); EXPECT_EQ("", config.KubernetesClusterName()); EXPECT_EQ("", config.KubernetesClusterLocation()); @@ -44,41 +50,44 @@ class MetadataAgentConfigurationTest : public ::testing::Test { MetadataAgentConfiguration config; }; -TEST_F(MetadataAgentConfigurationTest, NoConfigTest) { +TEST_F(MetadataAgentConfigurationTest, NoConfig) { VerifyDefaultConfig(); } -TEST_F(MetadataAgentConfigurationTest, EmptyConfigTest) { +TEST_F(MetadataAgentConfigurationTest, EmptyConfig) { ParseConfiguration(""); VerifyDefaultConfig(); } -TEST_F(MetadataAgentConfigurationTest, PopulatedConfigTest) { +TEST_F(MetadataAgentConfigurationTest, PopulatedConfig) { ParseConfiguration( "ProjectId: TestProjectId\n" "MetadataApiNumThreads: 13\n" - "MetadataReporterPurgeDeleted: true"); + "MetadataReporterPurgeDeleted: true" + ); EXPECT_EQ("TestProjectId", config.ProjectId()); EXPECT_EQ(13, config.MetadataApiNumThreads()); EXPECT_EQ(true, config.MetadataReporterPurgeDeleted()); } -TEST_F(MetadataAgentConfigurationTest, CommentSkippedTest) { +TEST_F(MetadataAgentConfigurationTest, CommentSkipped) { ParseConfiguration( "ProjectId: TestProjectId\n" "#MetadataApiNumThreads: 13\n" - "MetadataReporterPurgeDeleted: true"); + "MetadataReporterPurgeDeleted: true" + ); EXPECT_EQ(3, config.MetadataApiNumThreads()); } -TEST_F(MetadataAgentConfigurationTest, BlankLineTest) { +TEST_F(MetadataAgentConfigurationTest, BlankLine) { ParseConfiguration( "ProjectId: TestProjectId\n" "\n" "\n" - "MetadataReporterPurgeDeleted: true"); + "MetadataReporterPurgeDeleted: true" + ); EXPECT_EQ("TestProjectId", config.ProjectId()); EXPECT_EQ(true, config.MetadataReporterPurgeDeleted()); } -} // google +} // namespace google From 95e5d161d911203156ce081fbf2300363130750c Mon Sep 17 00:00:00 2001 From: Andrew Emil Date: Fri, 16 Mar 2018 09:25:52 -0400 Subject: [PATCH 6/7] Reverted unneeded whitespace changes --- test/configuration_unittest.cc | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/test/configuration_unittest.cc b/test/configuration_unittest.cc index 7d0ce396..3432abdf 100644 --- a/test/configuration_unittest.cc +++ b/test/configuration_unittest.cc @@ -18,26 +18,21 @@ class MetadataAgentConfigurationTest : public ::testing::Test { EXPECT_EQ(".", config.MetadataApiResourceTypeSeparator()); EXPECT_EQ(60, config.MetadataReporterIntervalSeconds()); EXPECT_EQ(false, config.MetadataReporterPurgeDeleted()); - EXPECT_EQ( - "https://stackdriver.googleapis.com/" - "v1beta2/projects/{{project_id}}/resourceMetadata:batchUpdate", - config.MetadataIngestionEndpointFormat() - ); + EXPECT_EQ("https://stackdriver.googleapis.com/" + "v1beta2/projects/{{project_id}}/resourceMetadata:batchUpdate", + config.MetadataIngestionEndpointFormat()); EXPECT_EQ(8*1024*1024, config.MetadataIngestionRequestSizeLimitBytes()); EXPECT_EQ("0.1", config.MetadataIngestionRawContentVersion()); EXPECT_EQ(60*60, config.InstanceUpdaterIntervalSeconds()); EXPECT_EQ("", config.InstanceResourceType()); EXPECT_EQ(60, config.DockerUpdaterIntervalSeconds()); - EXPECT_EQ( - "unix://%2Fvar%2Frun%2Fdocker.sock/", - config.DockerEndpointHost() - ); + EXPECT_EQ("unix://%2Fvar%2Frun%2Fdocker.sock/", + config.DockerEndpointHost()); EXPECT_EQ("1.23", config.DockerApiVersion()); EXPECT_EQ("limit=30", config.DockerContainerFilter()); EXPECT_EQ(0, config.KubernetesUpdaterIntervalSeconds()); EXPECT_EQ("https://kubernetes.default.svc", - config.KubernetesEndpointHost() - ); + config.KubernetesEndpointHost()); EXPECT_EQ("", config.KubernetesPodLabelSelector()); EXPECT_EQ("", config.KubernetesClusterName()); EXPECT_EQ("", config.KubernetesClusterLocation()); From 8d091482429787b07036ab941d7afa2f3626db6d Mon Sep 17 00:00:00 2001 From: Andrew Emil Date: Fri, 16 Mar 2018 14:16:57 -0400 Subject: [PATCH 7/7] Makefile changes request on PR #80 --- test/Makefile | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/Makefile b/test/Makefile index 64e9e719..84460837 100644 --- a/test/Makefile +++ b/test/Makefile @@ -13,9 +13,8 @@ YAML_CPP_LIBDIR=$(YAML_CPP_DIR) YAML_CPP_LIBS=$(YAML_CPP_LIBDIR)/libyaml-cpp.a -CMAKE=cmake -CPPFLAGS+=-isystem $(GTEST_DIR)/include -CXXFLAGS=-std=c++11 -g -pthread -I$(YAML_CPP_DIR)/include +CPPFLAGS+=-isystem $(GTEST_DIR)/include -I$(YAML_CPP_DIR)/include +CXXFLAGS=-std=c++11 -g -pthread LDLIBS=-lpthread -lboost_program_options -lyaml-cpp LDFLAGS=-L$(YAML_CPP_LIBDIR) @@ -26,10 +25,9 @@ TEST_DIR=. TEST_SOURCES=$(wildcard $(TEST_DIR)/*_unittest.cc) TEST_OBJS=$(TEST_SOURCES:$(TEST_DIR)/%.cc=%.o) TESTS=\ - base64_unittest \ - format_unittest \ base64_unittest \ configuration_unittest \ + format_unittest \ time_unittest GTEST_LIB=gtest_lib.a