From 24d8201b0b0550118796e1ab07dd689db75540c6 Mon Sep 17 00:00:00 2001 From: Dhrupad Bhardwaj Date: Wed, 28 Mar 2018 14:09:28 -0400 Subject: [PATCH 1/9] Unittests for MetadataStore and Metadata struct --- src/store.h | 1 + test/Makefile | 3 + test/store_unittest.cc | 253 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 257 insertions(+) create mode 100644 test/store_unittest.cc diff --git a/src/store.h b/src/store.h index bd1b18e5..ac189b95 100644 --- a/src/store.h +++ b/src/store.h @@ -92,6 +92,7 @@ class MetadataStore { private: friend class MetadataReporter; + friend class MetadataAgentStoreTest; std::map GetMetadataMap() const; void PurgeDeletedEntries(); diff --git a/test/Makefile b/test/Makefile index 5214c799..fd28a55a 100644 --- a/test/Makefile +++ b/test/Makefile @@ -35,6 +35,7 @@ TESTS=\ health_checker_unittest \ json_unittest \ resource_unittest \ + store_unittest \ time_unittest GTEST_LIB=gtest_lib.a @@ -85,6 +86,8 @@ configuration_unittest: $(GTEST_LIB) $(YAML_CPP_LIBS) configuration_unittest.o $ $(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@ resource_unittest: $(GTEST_LIB) resource_unittest.o $(SRC_DIR)/resource.o $(SRC_DIR)/json.o $(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@ +store_unittest: $(GTEST_LIB) store_unittest.o $(SRC_DIR)/store.o $(SRC_DIR)/resource.o $(SRC_DIR)/json.o $(SRC_DIR)/logging.o $(SRC_DIR)/time.o $(SRC_DIR)/configuration.o + $(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@ time_unittest: $(GTEST_LIB) time_unittest.o $(SRC_DIR)/time.o $(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@ health_checker_unittest: $(GTEST_LIB) health_checker_unittest.o $(SRC_DIR)/health_checker.o $(SRC_DIR)/configuration.o diff --git a/test/store_unittest.cc b/test/store_unittest.cc new file mode 100644 index 00000000..4d3772db --- /dev/null +++ b/test/store_unittest.cc @@ -0,0 +1,253 @@ +#include "../src/configuration.h" +#include "../src/resource.h" +#include "../src/store.h" +#include "../src/time.h" +#include "gtest/gtest.h" + +namespace google { + +class MetadataAgentStoreTest : public ::testing::Test { + protected: + Configuration *config; + MetadataStore* store; + + void SetupStore() { + config = new Configuration(std::istringstream("")); + store = new MetadataStore(*config); + } + + std::map GetMetadataMap() { + return store->GetMetadataMap(); + } + + virtual void TearDown() { + delete config; + delete store; + } + +}; + +TEST_F(MetadataAgentStoreTest, ResourceIDStore) { + SetupStore(); + MonitoredResource mr("some_resource", {}); + std::vector resourceId(1, "id"); + store->UpdateResource(resourceId, mr); + EXPECT_EQ("some_resource", store->LookupResource("id").type()); +} + +TEST_F(MetadataAgentStoreTest, ResourceNotFoundStore) { + SetupStore(); + EXPECT_THROW(store->LookupResource("some_resource_id").type(), + std::out_of_range); +} + +TEST_F(MetadataAgentStoreTest, ManyResourcesStore) { + SetupStore(); + MonitoredResource mr1("some_resource1", {}); + MonitoredResource mr2("some_resource2", {}); + std::vector resourceId1(1, "id1"); + std::vector resourceId2(1, "id2"); + store->UpdateResource(resourceId1, mr1); + store->UpdateResource(resourceId2, mr2); + EXPECT_EQ("some_resource1", store->LookupResource("id1").type()); + EXPECT_EQ("some_resource2", store->LookupResource("id2").type()); +} + +TEST_F(MetadataAgentStoreTest, ManyResourceIDsStore) { + SetupStore(); + MonitoredResource mr("some_resource", {}); + const char* ids_arr[] = {"id1", "id2", "id3"}; + std::vector resourceIds(ids_arr, std::end(ids_arr)); + store->UpdateResource(resourceIds, mr); + EXPECT_EQ("some_resource", store->LookupResource("id1").type()); + EXPECT_EQ("some_resource", store->LookupResource("id2").type()); + EXPECT_EQ("some_resource", store->LookupResource("id3").type()); +} + +TEST_F(MetadataAgentStoreTest, ManyIdsAndResourcesStore) { + SetupStore(); + MonitoredResource mr1("some_resource1", {}); + MonitoredResource mr2("some_resource2", {}); + const char* ids_arr1[] = {"id1", "id2", "id3"}; + const char* ids_arr2[] = {"id-a", "id-b", "id-c"}; + std::vector resourceIds1(ids_arr1, std::end(ids_arr1)); + store->UpdateResource(resourceIds1, mr1); + std::vector resourceIds2(ids_arr2, std::end(ids_arr2)); + store->UpdateResource(resourceIds2, mr2); + EXPECT_EQ("some_resource1", store->LookupResource("id1").type()); + EXPECT_EQ("some_resource1", store->LookupResource("id2").type()); + EXPECT_EQ("some_resource1", store->LookupResource("id3").type()); + EXPECT_EQ("some_resource2", store->LookupResource("id-a").type()); + EXPECT_EQ("some_resource2", store->LookupResource("id-b").type()); + EXPECT_EQ("some_resource2", store->LookupResource("id-c").type()); +} + +TEST_F(MetadataAgentStoreTest, MultipleResourceIdsUpdateStore) { + SetupStore(); + MonitoredResource mr("some_resource", {}); + const char* ids_arr[] = {"id1", "id2"}; + std::vector resourceIds(ids_arr, std::end(ids_arr)); + store->UpdateResource(resourceIds, mr); + EXPECT_EQ("some_resource", store->LookupResource("id1").type()); + EXPECT_EQ("some_resource", store->LookupResource("id2").type()); + const char* ids_arr2[] = {"id-a", "id-b"}; + std::vector resourceIds2(ids_arr2, std::end(ids_arr2)); + store->UpdateResource(resourceIds2, mr); + EXPECT_EQ("some_resource", store->LookupResource("id-a").type()); + EXPECT_EQ("some_resource", store->LookupResource("id-b").type()); +} + +TEST_F(MetadataAgentStoreTest, EmptyMetadataStore) { + SetupStore(); + std::map metadata_map = + GetMetadataMap(); + EXPECT_TRUE(metadata_map.empty()); +} + +TEST_F(MetadataAgentStoreTest, EmptyMetadataNonEmptyResourceStore) { + SetupStore(); + MonitoredResource mr("some_resource", {}); + std::vector resourceId(1, "abc123"); + store->UpdateResource(resourceId, mr); + std::map metadata_map = + GetMetadataMap(); + EXPECT_TRUE(metadata_map.empty()); +} + +TEST_F(MetadataAgentStoreTest, OneMetadataEntryStore) { + SetupStore(); + MonitoredResource mr("some_resource", {}); + MetadataStore::Metadata m( + "default-version", + false, + time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), + time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), + json::Parser::FromString("{\"f\":\"hello\"}") + ); + store->UpdateMetadata(mr, std::move(m)); + std::map metadata_map = + GetMetadataMap(); + EXPECT_FALSE(metadata_map.empty()); + EXPECT_EQ("default-version", metadata_map.at(mr).version); +} + +TEST_F(MetadataAgentStoreTest, MultipleMetadataEntriesStore) { + SetupStore(); + MonitoredResource mr1("some_resource1", {}); + MonitoredResource mr2("some_resource2", {}); + MetadataStore::Metadata m1( + "default-version1", + false, + time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), + time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), + json::Parser::FromString("{\"f\":\"hello\"}") + ); + MetadataStore::Metadata m2( + "default-version2", + false, + time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), + time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), + json::Parser::FromString("{\"f\":\"hello\"}") + ); + store->UpdateMetadata(mr1, std::move(m1)); + store->UpdateMetadata(mr2, std::move(m2)); + std::map metadata_map = + GetMetadataMap(); + EXPECT_FALSE(metadata_map.empty()); + EXPECT_EQ("default-version1", metadata_map.at(mr1).version); + EXPECT_EQ("default-version2", metadata_map.at(mr2).version); +} + +TEST_F(MetadataAgentStoreTest, UpdateMetadataEntryStore) { + SetupStore(); + MonitoredResource mr("some_resource", {}); + MetadataStore::Metadata m1( + "default-version1", + false, + time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), + time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), + json::Parser::FromString("{\"f\":\"hello\"}") + ); + store->UpdateMetadata(mr, std::move(m1)); + std::map metadata_map = + GetMetadataMap(); + EXPECT_FALSE(metadata_map.empty()); + EXPECT_EQ("default-version1", metadata_map.at(mr).version); + MetadataStore::Metadata m2( + "default-version2", + false, + time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), + time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), + json::Parser::FromString("{\"f\":\"hello\"}") + ); + store->UpdateMetadata(mr, std::move(m2)); + metadata_map = GetMetadataMap(); + EXPECT_EQ("default-version2", metadata_map.at(mr).version); +} + +TEST(MetadataTest, ParseMetadata) { + MetadataStore::Metadata m( + "default-version", + false, + time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), + time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), + json::Parser::FromString("{\"f\":\"hello\"}") + ); + EXPECT_EQ("default-version", m.version); + EXPECT_FALSE(m.is_deleted); + EXPECT_FALSE(m.ignore); + EXPECT_EQ(time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), + m.created_at); + EXPECT_EQ(time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), + m.collected_at); + const auto& obj = m.metadata->As(); + EXPECT_TRUE(obj->Has("f")); + EXPECT_EQ("hello", obj->Get("f")); +} + +TEST(MetadataTest, CloneMetadata) { + MetadataStore::Metadata m( + "default-version", + false, + time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), + time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), + json::Parser::FromString("{\"f\":\"hello\"}") + ); + MetadataStore::Metadata m_clone = m.Clone(); + EXPECT_EQ("default-version", m_clone.version); + EXPECT_FALSE(m_clone.is_deleted); + EXPECT_EQ(time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), + m_clone.created_at); + EXPECT_EQ(time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), + m_clone.collected_at); + const auto& obj = m_clone.metadata->As(); + EXPECT_TRUE(obj->Has("f")); + EXPECT_EQ("hello", obj->Get("f")); +} + +TEST(MetadataTest, TestIgnoredMetadata) { + MetadataStore::Metadata m = MetadataStore::Metadata::IGNORED(); + EXPECT_EQ("", m.version); + EXPECT_EQ(time::rfc3339::FromString("0000-00-00T00:00:00.000000000Z"), + m.created_at); + EXPECT_EQ(time::rfc3339::FromString("0000-00-00T00:00:00.000000000Z"), + m.collected_at); + EXPECT_EQ("{}", m.metadata->ToString()); + EXPECT_FALSE(m.is_deleted); + EXPECT_TRUE(m.ignore); +} + +TEST(MetadataTest, TestClonedIgnoredMetadata) { + MetadataStore::Metadata m = MetadataStore::Metadata::IGNORED(); + MetadataStore::Metadata m_clone = m.Clone(); + EXPECT_EQ("", m_clone.version); + EXPECT_EQ(time::rfc3339::FromString("0000-00-00T00:00:00.000000000Z"), + m_clone.created_at); + EXPECT_EQ(time::rfc3339::FromString("0000-00-00T00:00:00.000000000Z"), + m_clone.collected_at); + EXPECT_EQ("{}", m_clone.metadata->ToString()); + EXPECT_FALSE(m_clone.is_deleted); + EXPECT_TRUE(m_clone.ignore); +} + +} // namespace From b7dd8687718791b8032851cbefb3930c63b9e831 Mon Sep 17 00:00:00 2001 From: Dhrupad Bhardwaj Date: Wed, 28 Mar 2018 14:19:07 -0400 Subject: [PATCH 2/9] Unit test for PurgeDeletedEntries --- test/store_unittest.cc | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test/store_unittest.cc b/test/store_unittest.cc index 4d3772db..d3729cf6 100644 --- a/test/store_unittest.cc +++ b/test/store_unittest.cc @@ -20,6 +20,10 @@ class MetadataAgentStoreTest : public ::testing::Test { return store->GetMetadataMap(); } + void PurgeDeletedEntries() { + store->PurgeDeletedEntries(); + } + virtual void TearDown() { delete config; delete store; @@ -185,6 +189,37 @@ TEST_F(MetadataAgentStoreTest, UpdateMetadataEntryStore) { EXPECT_EQ("default-version2", metadata_map.at(mr).version); } +TEST_F(MetadataAgentStoreTest, PurgeDeletedEntriesStore) { + SetupStore(); + MonitoredResource mr1("some_resource1", {}); + MonitoredResource mr2("some_resource2", {}); + MetadataStore::Metadata m1( + "default-version1", + false, + time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), + time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), + json::Parser::FromString("{\"f\":\"hello\"}") + ); + MetadataStore::Metadata m2( + "default-version2", + true, + time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), + time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), + json::Parser::FromString("{\"f\":\"hello\"}") + ); + store->UpdateMetadata(mr1, std::move(m1)); + store->UpdateMetadata(mr2, std::move(m2)); + std::map metadata_map = + GetMetadataMap(); + EXPECT_FALSE(metadata_map.empty()); + EXPECT_EQ("default-version1", metadata_map.at(mr1).version); + EXPECT_EQ("default-version2", metadata_map.at(mr2).version); + PurgeDeletedEntries(); + metadata_map = GetMetadataMap(); + EXPECT_EQ("default-version1", metadata_map.at(mr1).version); + EXPECT_THROW(metadata_map.at(mr2), std::out_of_range); +} + TEST(MetadataTest, ParseMetadata) { MetadataStore::Metadata m( "default-version", From 0f276c67d003ad3a11d1243b9d0dbc9430e1bc80 Mon Sep 17 00:00:00 2001 From: Dhrupad Bhardwaj Date: Wed, 28 Mar 2018 16:34:13 -0400 Subject: [PATCH 3/9] Address feedback --- src/store.h | 2 +- test/store_unittest.cc | 248 ++++++++++++++++------------------------- 2 files changed, 94 insertions(+), 156 deletions(-) diff --git a/src/store.h b/src/store.h index ac189b95..fef720b8 100644 --- a/src/store.h +++ b/src/store.h @@ -92,7 +92,7 @@ class MetadataStore { private: friend class MetadataReporter; - friend class MetadataAgentStoreTest; + friend class MetadataStoreTest; std::map GetMetadataMap() const; void PurgeDeletedEntries(); diff --git a/test/store_unittest.cc b/test/store_unittest.cc index d3729cf6..97df92ec 100644 --- a/test/store_unittest.cc +++ b/test/store_unittest.cc @@ -6,218 +6,170 @@ namespace google { -class MetadataAgentStoreTest : public ::testing::Test { +class MetadataStoreTest : public ::testing::Test { protected: - Configuration *config; - MetadataStore* store; + Configuration config; + MetadataStore store; - void SetupStore() { - config = new Configuration(std::istringstream("")); - store = new MetadataStore(*config); - } + MetadataStoreTest() : config(), store(config) {} std::map GetMetadataMap() { - return store->GetMetadataMap(); + return store.GetMetadataMap(); } void PurgeDeletedEntries() { - store->PurgeDeletedEntries(); - } - - virtual void TearDown() { - delete config; - delete store; + store.PurgeDeletedEntries(); } }; -TEST_F(MetadataAgentStoreTest, ResourceIDStore) { - SetupStore(); +TEST_F(MetadataStoreTest, ResourceID) { MonitoredResource mr("some_resource", {}); - std::vector resourceId(1, "id"); - store->UpdateResource(resourceId, mr); - EXPECT_EQ("some_resource", store->LookupResource("id").type()); + store.UpdateResource({"id"}, mr); + EXPECT_EQ("some_resource", store.LookupResource("id").type()); } -TEST_F(MetadataAgentStoreTest, ResourceNotFoundStore) { - SetupStore(); - EXPECT_THROW(store->LookupResource("some_resource_id").type(), - std::out_of_range); +TEST_F(MetadataStoreTest, ResourceNotFound) { + EXPECT_THROW(store.LookupResource("some_resource_id"), std::out_of_range); } -TEST_F(MetadataAgentStoreTest, ManyResourcesStore) { - SetupStore(); +TEST_F(MetadataStoreTest, ManyResources) { MonitoredResource mr1("some_resource1", {}); MonitoredResource mr2("some_resource2", {}); - std::vector resourceId1(1, "id1"); - std::vector resourceId2(1, "id2"); - store->UpdateResource(resourceId1, mr1); - store->UpdateResource(resourceId2, mr2); - EXPECT_EQ("some_resource1", store->LookupResource("id1").type()); - EXPECT_EQ("some_resource2", store->LookupResource("id2").type()); + store.UpdateResource({"id1"}, mr1); + store.UpdateResource({"id2"}, mr2); + EXPECT_EQ("some_resource1", store.LookupResource("id1").type()); + EXPECT_EQ("some_resource2", store.LookupResource("id2").type()); } -TEST_F(MetadataAgentStoreTest, ManyResourceIDsStore) { - SetupStore(); +TEST_F(MetadataStoreTest, ManyResourceIDs) { MonitoredResource mr("some_resource", {}); - const char* ids_arr[] = {"id1", "id2", "id3"}; - std::vector resourceIds(ids_arr, std::end(ids_arr)); - store->UpdateResource(resourceIds, mr); - EXPECT_EQ("some_resource", store->LookupResource("id1").type()); - EXPECT_EQ("some_resource", store->LookupResource("id2").type()); - EXPECT_EQ("some_resource", store->LookupResource("id3").type()); + store.UpdateResource({"id1", "id2", "id3"}, mr); + EXPECT_EQ("some_resource", store.LookupResource("id1").type()); + EXPECT_EQ("some_resource", store.LookupResource("id2").type()); + EXPECT_EQ("some_resource", store.LookupResource("id3").type()); } -TEST_F(MetadataAgentStoreTest, ManyIdsAndResourcesStore) { - SetupStore(); +TEST_F(MetadataStoreTest, ManyIdsAndResources) { MonitoredResource mr1("some_resource1", {}); MonitoredResource mr2("some_resource2", {}); - const char* ids_arr1[] = {"id1", "id2", "id3"}; - const char* ids_arr2[] = {"id-a", "id-b", "id-c"}; - std::vector resourceIds1(ids_arr1, std::end(ids_arr1)); - store->UpdateResource(resourceIds1, mr1); - std::vector resourceIds2(ids_arr2, std::end(ids_arr2)); - store->UpdateResource(resourceIds2, mr2); - EXPECT_EQ("some_resource1", store->LookupResource("id1").type()); - EXPECT_EQ("some_resource1", store->LookupResource("id2").type()); - EXPECT_EQ("some_resource1", store->LookupResource("id3").type()); - EXPECT_EQ("some_resource2", store->LookupResource("id-a").type()); - EXPECT_EQ("some_resource2", store->LookupResource("id-b").type()); - EXPECT_EQ("some_resource2", store->LookupResource("id-c").type()); + store.UpdateResource({"id1", "id2", "id3"}, mr1); + store.UpdateResource({"id-a", "id-b", "id-c"}, mr2); + EXPECT_EQ("some_resource1", store.LookupResource("id1").type()); + EXPECT_EQ("some_resource1", store.LookupResource("id2").type()); + EXPECT_EQ("some_resource1", store.LookupResource("id3").type()); + EXPECT_EQ("some_resource2", store.LookupResource("id-a").type()); + EXPECT_EQ("some_resource2", store.LookupResource("id-b").type()); + EXPECT_EQ("some_resource2", store.LookupResource("id-c").type()); } -TEST_F(MetadataAgentStoreTest, MultipleResourceIdsUpdateStore) { - SetupStore(); +TEST_F(MetadataStoreTest, MultipleResourceIdsUpdate) { MonitoredResource mr("some_resource", {}); - const char* ids_arr[] = {"id1", "id2"}; - std::vector resourceIds(ids_arr, std::end(ids_arr)); - store->UpdateResource(resourceIds, mr); - EXPECT_EQ("some_resource", store->LookupResource("id1").type()); - EXPECT_EQ("some_resource", store->LookupResource("id2").type()); - const char* ids_arr2[] = {"id-a", "id-b"}; - std::vector resourceIds2(ids_arr2, std::end(ids_arr2)); - store->UpdateResource(resourceIds2, mr); - EXPECT_EQ("some_resource", store->LookupResource("id-a").type()); - EXPECT_EQ("some_resource", store->LookupResource("id-b").type()); + store.UpdateResource({"id1", "id2"}, mr); + EXPECT_EQ("some_resource", store.LookupResource("id1").type()); + EXPECT_EQ("some_resource", store.LookupResource("id2").type()); + store.UpdateResource({"id-a", "id-b"}, mr); + EXPECT_EQ("some_resource", store.LookupResource("id-a").type()); + EXPECT_EQ("some_resource", store.LookupResource("id-b").type()); } -TEST_F(MetadataAgentStoreTest, EmptyMetadataStore) { - SetupStore(); - std::map metadata_map = - GetMetadataMap(); +TEST_F(MetadataStoreTest, EmptyMetadata) { + const auto metadata_map = GetMetadataMap(); EXPECT_TRUE(metadata_map.empty()); } -TEST_F(MetadataAgentStoreTest, EmptyMetadataNonEmptyResourceStore) { - SetupStore(); +TEST_F(MetadataStoreTest, EmptyMetadataNonemptyResource) { MonitoredResource mr("some_resource", {}); - std::vector resourceId(1, "abc123"); - store->UpdateResource(resourceId, mr); - std::map metadata_map = - GetMetadataMap(); + store.UpdateResource({"id1"}, mr); + const auto metadata_map = GetMetadataMap(); EXPECT_TRUE(metadata_map.empty()); } -TEST_F(MetadataAgentStoreTest, OneMetadataEntryStore) { - SetupStore(); +TEST_F(MetadataStoreTest, OneMetadataEntry) { MonitoredResource mr("some_resource", {}); MetadataStore::Metadata m( "default-version", false, - time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), - time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), - json::Parser::FromString("{\"f\":\"hello\"}") - ); - store->UpdateMetadata(mr, std::move(m)); - std::map metadata_map = - GetMetadataMap(); + std::chrono::system_clock::now(), + std::chrono::system_clock::now(), + json::object({{"f", json::string("hello")}})); + store.UpdateMetadata(mr, std::move(m)); + const auto metadata_map = GetMetadataMap(); EXPECT_FALSE(metadata_map.empty()); EXPECT_EQ("default-version", metadata_map.at(mr).version); } -TEST_F(MetadataAgentStoreTest, MultipleMetadataEntriesStore) { - SetupStore(); +TEST_F(MetadataStoreTest, MultipleMetadataEntries) { MonitoredResource mr1("some_resource1", {}); MonitoredResource mr2("some_resource2", {}); MetadataStore::Metadata m1( "default-version1", false, - time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), - time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), - json::Parser::FromString("{\"f\":\"hello\"}") - ); + std::chrono::system_clock::now(), + std::chrono::system_clock::now(), + json::object({{"f", json::string("hello")}})); MetadataStore::Metadata m2( "default-version2", false, - time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), - time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), - json::Parser::FromString("{\"f\":\"hello\"}") - ); - store->UpdateMetadata(mr1, std::move(m1)); - store->UpdateMetadata(mr2, std::move(m2)); - std::map metadata_map = - GetMetadataMap(); + std::chrono::system_clock::now(), + std::chrono::system_clock::now(), + json::object({{"f", json::string("hello")}})); + store.UpdateMetadata(mr1, std::move(m1)); + store.UpdateMetadata(mr2, std::move(m2)); + const auto metadata_map = GetMetadataMap(); EXPECT_FALSE(metadata_map.empty()); EXPECT_EQ("default-version1", metadata_map.at(mr1).version); EXPECT_EQ("default-version2", metadata_map.at(mr2).version); } -TEST_F(MetadataAgentStoreTest, UpdateMetadataEntryStore) { - SetupStore(); +TEST_F(MetadataStoreTest, UpdateMetadataEntry) { MonitoredResource mr("some_resource", {}); MetadataStore::Metadata m1( "default-version1", false, - time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), - time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), - json::Parser::FromString("{\"f\":\"hello\"}") - ); - store->UpdateMetadata(mr, std::move(m1)); - std::map metadata_map = - GetMetadataMap(); - EXPECT_FALSE(metadata_map.empty()); - EXPECT_EQ("default-version1", metadata_map.at(mr).version); + std::chrono::system_clock::now(), + std::chrono::system_clock::now(), + json::object({{"f", json::string("hello")}})); + store.UpdateMetadata(mr, std::move(m1)); + const auto metadata_map_before = GetMetadataMap(); + EXPECT_FALSE(metadata_map_before.empty()); + EXPECT_EQ("default-version1", metadata_map_before.at(mr).version); MetadataStore::Metadata m2( "default-version2", false, - time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), - time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), - json::Parser::FromString("{\"f\":\"hello\"}") - ); - store->UpdateMetadata(mr, std::move(m2)); - metadata_map = GetMetadataMap(); - EXPECT_EQ("default-version2", metadata_map.at(mr).version); + std::chrono::system_clock::now(), + std::chrono::system_clock::now(), + json::object({{"f", json::string("hello")}})); + store.UpdateMetadata(mr, std::move(m2)); + const auto metadata_map_after = GetMetadataMap(); + EXPECT_EQ("default-version2", metadata_map_after.at(mr).version); } -TEST_F(MetadataAgentStoreTest, PurgeDeletedEntriesStore) { - SetupStore(); +TEST_F(MetadataStoreTest, PurgeDeletedEntries) { MonitoredResource mr1("some_resource1", {}); MonitoredResource mr2("some_resource2", {}); MetadataStore::Metadata m1( "default-version1", false, - time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), - time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), - json::Parser::FromString("{\"f\":\"hello\"}") - ); + std::chrono::system_clock::now(), + std::chrono::system_clock::now(), + json::object({{"f", json::string("hello")}})); MetadataStore::Metadata m2( "default-version2", true, - time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), - time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), - json::Parser::FromString("{\"f\":\"hello\"}") - ); - store->UpdateMetadata(mr1, std::move(m1)); - store->UpdateMetadata(mr2, std::move(m2)); - std::map metadata_map = - GetMetadataMap(); - EXPECT_FALSE(metadata_map.empty()); - EXPECT_EQ("default-version1", metadata_map.at(mr1).version); - EXPECT_EQ("default-version2", metadata_map.at(mr2).version); + std::chrono::system_clock::now(), + std::chrono::system_clock::now(), + json::object({{"f", json::string("hello")}})); + store.UpdateMetadata(mr1, std::move(m1)); + store.UpdateMetadata(mr2, std::move(m2)); + const auto metadata_map_before = GetMetadataMap(); + EXPECT_FALSE(metadata_map_before.empty()); + EXPECT_EQ("default-version1", metadata_map_before.at(mr1).version); + EXPECT_EQ("default-version2", metadata_map_before.at(mr2).version); PurgeDeletedEntries(); - metadata_map = GetMetadataMap(); - EXPECT_EQ("default-version1", metadata_map.at(mr1).version); - EXPECT_THROW(metadata_map.at(mr2), std::out_of_range); + const auto metadata_map_after = GetMetadataMap(); + EXPECT_EQ("default-version1", metadata_map_after.at(mr1).version); + EXPECT_THROW(metadata_map_after.at(mr2), std::out_of_range); } TEST(MetadataTest, ParseMetadata) { @@ -226,8 +178,7 @@ TEST(MetadataTest, ParseMetadata) { false, time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), - json::Parser::FromString("{\"f\":\"hello\"}") - ); + json::object({{"f", json::string("hello")}})); EXPECT_EQ("default-version", m.version); EXPECT_FALSE(m.is_deleted); EXPECT_FALSE(m.ignore); @@ -235,7 +186,7 @@ TEST(MetadataTest, ParseMetadata) { m.created_at); EXPECT_EQ(time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), m.collected_at); - const auto& obj = m.metadata->As(); + const json::Object* obj = m.metadata->As(); EXPECT_TRUE(obj->Has("f")); EXPECT_EQ("hello", obj->Get("f")); } @@ -246,28 +197,22 @@ TEST(MetadataTest, CloneMetadata) { false, time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), - json::Parser::FromString("{\"f\":\"hello\"}") - ); + json::object({{"f", json::string("hello")}})); MetadataStore::Metadata m_clone = m.Clone(); EXPECT_EQ("default-version", m_clone.version); EXPECT_FALSE(m_clone.is_deleted); + EXPECT_FALSE(m_clone.ignore); EXPECT_EQ(time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), m_clone.created_at); EXPECT_EQ(time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), m_clone.collected_at); - const auto& obj = m_clone.metadata->As(); + const json::Object* obj = m_clone.metadata->As(); EXPECT_TRUE(obj->Has("f")); EXPECT_EQ("hello", obj->Get("f")); } TEST(MetadataTest, TestIgnoredMetadata) { MetadataStore::Metadata m = MetadataStore::Metadata::IGNORED(); - EXPECT_EQ("", m.version); - EXPECT_EQ(time::rfc3339::FromString("0000-00-00T00:00:00.000000000Z"), - m.created_at); - EXPECT_EQ(time::rfc3339::FromString("0000-00-00T00:00:00.000000000Z"), - m.collected_at); - EXPECT_EQ("{}", m.metadata->ToString()); EXPECT_FALSE(m.is_deleted); EXPECT_TRUE(m.ignore); } @@ -275,13 +220,6 @@ TEST(MetadataTest, TestIgnoredMetadata) { TEST(MetadataTest, TestClonedIgnoredMetadata) { MetadataStore::Metadata m = MetadataStore::Metadata::IGNORED(); MetadataStore::Metadata m_clone = m.Clone(); - EXPECT_EQ("", m_clone.version); - EXPECT_EQ(time::rfc3339::FromString("0000-00-00T00:00:00.000000000Z"), - m_clone.created_at); - EXPECT_EQ(time::rfc3339::FromString("0000-00-00T00:00:00.000000000Z"), - m_clone.collected_at); - EXPECT_EQ("{}", m_clone.metadata->ToString()); - EXPECT_FALSE(m_clone.is_deleted); EXPECT_TRUE(m_clone.ignore); } From a68a9950fbb250568850f53e28ec30820605901f Mon Sep 17 00:00:00 2001 From: Dhrupad Bhardwaj Date: Wed, 28 Mar 2018 18:37:38 -0400 Subject: [PATCH 4/9] Address more feedback --- test/store_unittest.cc | 96 +++++++++++++++++++++++------------------- 1 file changed, 52 insertions(+), 44 deletions(-) diff --git a/test/store_unittest.cc b/test/store_unittest.cc index 97df92ec..1c473141 100644 --- a/test/store_unittest.cc +++ b/test/store_unittest.cc @@ -13,7 +13,7 @@ class MetadataStoreTest : public ::testing::Test { MetadataStoreTest() : config(), store(config) {} - std::map GetMetadataMap() { + std::map GetMetadataMap() const { return store.GetMetadataMap(); } @@ -23,69 +23,78 @@ class MetadataStoreTest : public ::testing::Test { }; -TEST_F(MetadataStoreTest, ResourceID) { +TEST_F(MetadataStoreTest, ResourceWithOneIdStoredCorrectly) { MonitoredResource mr("some_resource", {}); store.UpdateResource({"id"}, mr); - EXPECT_EQ("some_resource", store.LookupResource("id").type()); + EXPECT_EQ(mr, store.LookupResource("id")); } -TEST_F(MetadataStoreTest, ResourceNotFound) { +TEST_F(MetadataStoreTest, EmptyStoreThrowsError) { EXPECT_THROW(store.LookupResource("some_resource_id"), std::out_of_range); } -TEST_F(MetadataStoreTest, ManyResources) { +TEST_F(MetadataStoreTest, ResourceNotFoundThrowsError) { + MonitoredResource mr("some_resource", {}); + store.UpdateResource({"id"}, mr); + EXPECT_EQ(mr, store.LookupResource("id")); + EXPECT_THROW(store.LookupResource("other_resource"), std::out_of_range); +} + +TEST_F(MetadataStoreTest, MultipleResourcesWithSingleIdsCorrectlyStored) { MonitoredResource mr1("some_resource1", {}); MonitoredResource mr2("some_resource2", {}); store.UpdateResource({"id1"}, mr1); store.UpdateResource({"id2"}, mr2); - EXPECT_EQ("some_resource1", store.LookupResource("id1").type()); - EXPECT_EQ("some_resource2", store.LookupResource("id2").type()); + EXPECT_EQ(mr1, store.LookupResource("id1")); + EXPECT_EQ(mr2, store.LookupResource("id2")); } -TEST_F(MetadataStoreTest, ManyResourceIDs) { +TEST_F(MetadataStoreTest, SingleResourceWithMultipleIdsCorrectlyStored) { MonitoredResource mr("some_resource", {}); store.UpdateResource({"id1", "id2", "id3"}, mr); - EXPECT_EQ("some_resource", store.LookupResource("id1").type()); - EXPECT_EQ("some_resource", store.LookupResource("id2").type()); - EXPECT_EQ("some_resource", store.LookupResource("id3").type()); + EXPECT_EQ(mr, store.LookupResource("id1")); + EXPECT_EQ(mr, store.LookupResource("id2")); + EXPECT_EQ(mr, store.LookupResource("id3")); } -TEST_F(MetadataStoreTest, ManyIdsAndResources) { +TEST_F(MetadataStoreTest, MultipleResourcesAndIdsCorrectlyStored) { MonitoredResource mr1("some_resource1", {}); MonitoredResource mr2("some_resource2", {}); store.UpdateResource({"id1", "id2", "id3"}, mr1); store.UpdateResource({"id-a", "id-b", "id-c"}, mr2); - EXPECT_EQ("some_resource1", store.LookupResource("id1").type()); - EXPECT_EQ("some_resource1", store.LookupResource("id2").type()); - EXPECT_EQ("some_resource1", store.LookupResource("id3").type()); - EXPECT_EQ("some_resource2", store.LookupResource("id-a").type()); - EXPECT_EQ("some_resource2", store.LookupResource("id-b").type()); - EXPECT_EQ("some_resource2", store.LookupResource("id-c").type()); + EXPECT_EQ(mr1, store.LookupResource("id1")); + EXPECT_EQ(mr1, store.LookupResource("id2")); + EXPECT_EQ(mr1, store.LookupResource("id3")); + EXPECT_EQ(mr2, store.LookupResource("id-a")); + EXPECT_EQ(mr2, store.LookupResource("id-b")); + EXPECT_EQ(mr2, store.LookupResource("id-c")); } -TEST_F(MetadataStoreTest, MultipleResourceIdsUpdate) { +TEST_F(MetadataStoreTest, ResourceToIdsAssociationCorrectlyUpdated) { MonitoredResource mr("some_resource", {}); store.UpdateResource({"id1", "id2"}, mr); - EXPECT_EQ("some_resource", store.LookupResource("id1").type()); - EXPECT_EQ("some_resource", store.LookupResource("id2").type()); + EXPECT_EQ(mr, store.LookupResource("id1")); + EXPECT_EQ(mr, store.LookupResource("id2")); store.UpdateResource({"id-a", "id-b"}, mr); - EXPECT_EQ("some_resource", store.LookupResource("id-a").type()); - EXPECT_EQ("some_resource", store.LookupResource("id-b").type()); + EXPECT_EQ(mr, store.LookupResource("id1")); + EXPECT_EQ(mr, store.LookupResource("id2")); + EXPECT_EQ(mr, store.LookupResource("id-a")); + EXPECT_EQ(mr, store.LookupResource("id-b")); } -TEST_F(MetadataStoreTest, EmptyMetadata) { +TEST_F(MetadataStoreTest, DefaultMetadataMapIsEmpty) { const auto metadata_map = GetMetadataMap(); EXPECT_TRUE(metadata_map.empty()); } -TEST_F(MetadataStoreTest, EmptyMetadataNonemptyResource) { +TEST_F(MetadataStoreTest, UpdateResourceDoesNotUpdateMetadata) { MonitoredResource mr("some_resource", {}); store.UpdateResource({"id1"}, mr); const auto metadata_map = GetMetadataMap(); EXPECT_TRUE(metadata_map.empty()); } -TEST_F(MetadataStoreTest, OneMetadataEntry) { +TEST_F(MetadataStoreTest, UpdateMetadataChangesMetadataMap) { MonitoredResource mr("some_resource", {}); MetadataStore::Metadata m( "default-version", @@ -95,11 +104,11 @@ TEST_F(MetadataStoreTest, OneMetadataEntry) { json::object({{"f", json::string("hello")}})); store.UpdateMetadata(mr, std::move(m)); const auto metadata_map = GetMetadataMap(); - EXPECT_FALSE(metadata_map.empty()); + EXPECT_EQ(1, metadata_map.size()); EXPECT_EQ("default-version", metadata_map.at(mr).version); } -TEST_F(MetadataStoreTest, MultipleMetadataEntries) { +TEST_F(MetadataStoreTest, MultipleUpdateMetadataChangesMetadataMap) { MonitoredResource mr1("some_resource1", {}); MonitoredResource mr2("some_resource2", {}); MetadataStore::Metadata m1( @@ -117,12 +126,12 @@ TEST_F(MetadataStoreTest, MultipleMetadataEntries) { store.UpdateMetadata(mr1, std::move(m1)); store.UpdateMetadata(mr2, std::move(m2)); const auto metadata_map = GetMetadataMap(); - EXPECT_FALSE(metadata_map.empty()); + EXPECT_EQ(2, metadata_map.size()); EXPECT_EQ("default-version1", metadata_map.at(mr1).version); EXPECT_EQ("default-version2", metadata_map.at(mr2).version); } -TEST_F(MetadataStoreTest, UpdateMetadataEntry) { +TEST_F(MetadataStoreTest, UpdateMetadataForResourceChangesMetadataEntry) { MonitoredResource mr("some_resource", {}); MetadataStore::Metadata m1( "default-version1", @@ -132,7 +141,7 @@ TEST_F(MetadataStoreTest, UpdateMetadataEntry) { json::object({{"f", json::string("hello")}})); store.UpdateMetadata(mr, std::move(m1)); const auto metadata_map_before = GetMetadataMap(); - EXPECT_FALSE(metadata_map_before.empty()); + EXPECT_EQ(1, metadata_map_before.size()); EXPECT_EQ("default-version1", metadata_map_before.at(mr).version); MetadataStore::Metadata m2( "default-version2", @@ -142,10 +151,11 @@ TEST_F(MetadataStoreTest, UpdateMetadataEntry) { json::object({{"f", json::string("hello")}})); store.UpdateMetadata(mr, std::move(m2)); const auto metadata_map_after = GetMetadataMap(); + EXPECT_EQ(1, metadata_map_after.size()); EXPECT_EQ("default-version2", metadata_map_after.at(mr).version); } -TEST_F(MetadataStoreTest, PurgeDeletedEntries) { +TEST_F(MetadataStoreTest, PurgeDeletedEntriesDeletesCorrectMetadata) { MonitoredResource mr1("some_resource1", {}); MonitoredResource mr2("some_resource2", {}); MetadataStore::Metadata m1( @@ -163,25 +173,26 @@ TEST_F(MetadataStoreTest, PurgeDeletedEntries) { store.UpdateMetadata(mr1, std::move(m1)); store.UpdateMetadata(mr2, std::move(m2)); const auto metadata_map_before = GetMetadataMap(); - EXPECT_FALSE(metadata_map_before.empty()); + EXPECT_EQ(2, metadata_map_before.size()); EXPECT_EQ("default-version1", metadata_map_before.at(mr1).version); EXPECT_EQ("default-version2", metadata_map_before.at(mr2).version); PurgeDeletedEntries(); const auto metadata_map_after = GetMetadataMap(); + EXPECT_EQ(1, metadata_map_after.size()); EXPECT_EQ("default-version1", metadata_map_after.at(mr1).version); EXPECT_THROW(metadata_map_after.at(mr2), std::out_of_range); } -TEST(MetadataTest, ParseMetadata) { +TEST(MetadataTest, MetadataParsedCorrectly) { MetadataStore::Metadata m( "default-version", false, time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), json::object({{"f", json::string("hello")}})); + EXPECT_FALSE(m.ignore); EXPECT_EQ("default-version", m.version); EXPECT_FALSE(m.is_deleted); - EXPECT_FALSE(m.ignore); EXPECT_EQ(time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), m.created_at); EXPECT_EQ(time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), @@ -191,7 +202,7 @@ TEST(MetadataTest, ParseMetadata) { EXPECT_EQ("hello", obj->Get("f")); } -TEST(MetadataTest, CloneMetadata) { +TEST(MetadataTest, MetadataClonedCorrectly) { MetadataStore::Metadata m( "default-version", false, @@ -199,25 +210,22 @@ TEST(MetadataTest, CloneMetadata) { time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), json::object({{"f", json::string("hello")}})); MetadataStore::Metadata m_clone = m.Clone(); - EXPECT_EQ("default-version", m_clone.version); + EXPECT_EQ(m.version, m_clone.version); EXPECT_FALSE(m_clone.is_deleted); EXPECT_FALSE(m_clone.ignore); - EXPECT_EQ(time::rfc3339::FromString("2018-03-03T01:23:45.678901234Z"), - m_clone.created_at); - EXPECT_EQ(time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), - m_clone.collected_at); + EXPECT_EQ(m.created_at, m_clone.created_at); + EXPECT_EQ(m.collected_at, m_clone.collected_at); const json::Object* obj = m_clone.metadata->As(); EXPECT_TRUE(obj->Has("f")); EXPECT_EQ("hello", obj->Get("f")); } -TEST(MetadataTest, TestIgnoredMetadata) { +TEST(MetadataTest, IgnoredMetadataCorrectlyCreated) { MetadataStore::Metadata m = MetadataStore::Metadata::IGNORED(); - EXPECT_FALSE(m.is_deleted); EXPECT_TRUE(m.ignore); } -TEST(MetadataTest, TestClonedIgnoredMetadata) { +TEST(MetadataTest, IgnoredMetadataCorrectlyCloned) { MetadataStore::Metadata m = MetadataStore::Metadata::IGNORED(); MetadataStore::Metadata m_clone = m.Clone(); EXPECT_TRUE(m_clone.ignore); From c02df842f132563c6a4940b1970068492dbd43be Mon Sep 17 00:00:00 2001 From: Dhrupad Bhardwaj Date: Thu, 29 Mar 2018 13:05:46 -0400 Subject: [PATCH 5/9] Address feedback --- test/store_unittest.cc | 53 +++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/test/store_unittest.cc b/test/store_unittest.cc index 1c473141..043d4572 100644 --- a/test/store_unittest.cc +++ b/test/store_unittest.cc @@ -8,9 +8,6 @@ namespace google { class MetadataStoreTest : public ::testing::Test { protected: - Configuration config; - MetadataStore store; - MetadataStoreTest() : config(), store(config) {} std::map GetMetadataMap() const { @@ -21,45 +18,47 @@ class MetadataStoreTest : public ::testing::Test { store.PurgeDeletedEntries(); } + Configuration config; + MetadataStore store; }; -TEST_F(MetadataStoreTest, ResourceWithOneIdStoredCorrectly) { - MonitoredResource mr("some_resource", {}); +TEST_F(MetadataStoreTest, ResourceWithOneIdCorrectlyStored) { + MonitoredResource mr("type", {}); store.UpdateResource({"id"}, mr); EXPECT_EQ(mr, store.LookupResource("id")); } TEST_F(MetadataStoreTest, EmptyStoreThrowsError) { - EXPECT_THROW(store.LookupResource("some_resource_id"), std::out_of_range); + EXPECT_THROW(store.LookupResource("missing_id"), std::out_of_range); } -TEST_F(MetadataStoreTest, ResourceNotFoundThrowsError) { - MonitoredResource mr("some_resource", {}); +TEST_F(MetadataStoreTest, UnknownResourceIdThrowsError) { + MonitoredResource mr("type", {}); store.UpdateResource({"id"}, mr); + EXPECT_THROW(store.LookupResource("unknown_id"), std::out_of_range); EXPECT_EQ(mr, store.LookupResource("id")); - EXPECT_THROW(store.LookupResource("other_resource"), std::out_of_range); } TEST_F(MetadataStoreTest, MultipleResourcesWithSingleIdsCorrectlyStored) { - MonitoredResource mr1("some_resource1", {}); - MonitoredResource mr2("some_resource2", {}); + MonitoredResource mr1("type1", {}); + MonitoredResource mr2("type2", {}); store.UpdateResource({"id1"}, mr1); store.UpdateResource({"id2"}, mr2); - EXPECT_EQ(mr1, store.LookupResource("id1")); EXPECT_EQ(mr2, store.LookupResource("id2")); + EXPECT_EQ(mr1, store.LookupResource("id1")); } TEST_F(MetadataStoreTest, SingleResourceWithMultipleIdsCorrectlyStored) { - MonitoredResource mr("some_resource", {}); + MonitoredResource mr("type", {}); store.UpdateResource({"id1", "id2", "id3"}, mr); + EXPECT_EQ(mr, store.LookupResource("id3")); EXPECT_EQ(mr, store.LookupResource("id1")); EXPECT_EQ(mr, store.LookupResource("id2")); - EXPECT_EQ(mr, store.LookupResource("id3")); } TEST_F(MetadataStoreTest, MultipleResourcesAndIdsCorrectlyStored) { - MonitoredResource mr1("some_resource1", {}); - MonitoredResource mr2("some_resource2", {}); + MonitoredResource mr1("type1", {}); + MonitoredResource mr2("type2", {}); store.UpdateResource({"id1", "id2", "id3"}, mr1); store.UpdateResource({"id-a", "id-b", "id-c"}, mr2); EXPECT_EQ(mr1, store.LookupResource("id1")); @@ -71,7 +70,7 @@ TEST_F(MetadataStoreTest, MultipleResourcesAndIdsCorrectlyStored) { } TEST_F(MetadataStoreTest, ResourceToIdsAssociationCorrectlyUpdated) { - MonitoredResource mr("some_resource", {}); + MonitoredResource mr("type", {}); store.UpdateResource({"id1", "id2"}, mr); EXPECT_EQ(mr, store.LookupResource("id1")); EXPECT_EQ(mr, store.LookupResource("id2")); @@ -88,14 +87,14 @@ TEST_F(MetadataStoreTest, DefaultMetadataMapIsEmpty) { } TEST_F(MetadataStoreTest, UpdateResourceDoesNotUpdateMetadata) { - MonitoredResource mr("some_resource", {}); + MonitoredResource mr("type", {}); store.UpdateResource({"id1"}, mr); const auto metadata_map = GetMetadataMap(); EXPECT_TRUE(metadata_map.empty()); } TEST_F(MetadataStoreTest, UpdateMetadataChangesMetadataMap) { - MonitoredResource mr("some_resource", {}); + MonitoredResource mr("type", {}); MetadataStore::Metadata m( "default-version", false, @@ -109,8 +108,8 @@ TEST_F(MetadataStoreTest, UpdateMetadataChangesMetadataMap) { } TEST_F(MetadataStoreTest, MultipleUpdateMetadataChangesMetadataMap) { - MonitoredResource mr1("some_resource1", {}); - MonitoredResource mr2("some_resource2", {}); + MonitoredResource mr1("type1", {}); + MonitoredResource mr2("type2", {}); MetadataStore::Metadata m1( "default-version1", false, @@ -132,7 +131,7 @@ TEST_F(MetadataStoreTest, MultipleUpdateMetadataChangesMetadataMap) { } TEST_F(MetadataStoreTest, UpdateMetadataForResourceChangesMetadataEntry) { - MonitoredResource mr("some_resource", {}); + MonitoredResource mr("type", {}); MetadataStore::Metadata m1( "default-version1", false, @@ -156,8 +155,8 @@ TEST_F(MetadataStoreTest, UpdateMetadataForResourceChangesMetadataEntry) { } TEST_F(MetadataStoreTest, PurgeDeletedEntriesDeletesCorrectMetadata) { - MonitoredResource mr1("some_resource1", {}); - MonitoredResource mr2("some_resource2", {}); + MonitoredResource mr1("type1", {}); + MonitoredResource mr2("type2", {}); MetadataStore::Metadata m1( "default-version1", false, @@ -183,7 +182,7 @@ TEST_F(MetadataStoreTest, PurgeDeletedEntriesDeletesCorrectMetadata) { EXPECT_THROW(metadata_map_after.at(mr2), std::out_of_range); } -TEST(MetadataTest, MetadataParsedCorrectly) { +TEST(MetadataTest, MetadataCorrectlyConstructed) { MetadataStore::Metadata m( "default-version", false, @@ -202,7 +201,7 @@ TEST(MetadataTest, MetadataParsedCorrectly) { EXPECT_EQ("hello", obj->Get("f")); } -TEST(MetadataTest, MetadataClonedCorrectly) { +TEST(MetadataTest, MetadataCorrectlyCloned) { MetadataStore::Metadata m( "default-version", false, @@ -210,9 +209,9 @@ TEST(MetadataTest, MetadataClonedCorrectly) { time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), json::object({{"f", json::string("hello")}})); MetadataStore::Metadata m_clone = m.Clone(); + EXPECT_FALSE(m_clone.ignore); EXPECT_EQ(m.version, m_clone.version); EXPECT_FALSE(m_clone.is_deleted); - EXPECT_FALSE(m_clone.ignore); EXPECT_EQ(m.created_at, m_clone.created_at); EXPECT_EQ(m.collected_at, m_clone.collected_at); const json::Object* obj = m_clone.metadata->As(); From fba23d24daee6d5e5e87751d0002261a61b0a831 Mon Sep 17 00:00:00 2001 From: Dhrupad Bhardwaj Date: Thu, 29 Mar 2018 15:16:56 -0400 Subject: [PATCH 6/9] Address feedback --- test/store_unittest.cc | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/test/store_unittest.cc b/test/store_unittest.cc index 043d4572..7d34fafd 100644 --- a/test/store_unittest.cc +++ b/test/store_unittest.cc @@ -35,7 +35,7 @@ TEST_F(MetadataStoreTest, EmptyStoreThrowsError) { TEST_F(MetadataStoreTest, UnknownResourceIdThrowsError) { MonitoredResource mr("type", {}); store.UpdateResource({"id"}, mr); - EXPECT_THROW(store.LookupResource("unknown_id"), std::out_of_range); + EXPECT_THROW(store.LookupResource("missing_id"), std::out_of_range); EXPECT_EQ(mr, store.LookupResource("id")); } @@ -196,9 +196,7 @@ TEST(MetadataTest, MetadataCorrectlyConstructed) { m.created_at); EXPECT_EQ(time::rfc3339::FromString("2018-03-03T01:32:45.678901234Z"), m.collected_at); - const json::Object* obj = m.metadata->As(); - EXPECT_TRUE(obj->Has("f")); - EXPECT_EQ("hello", obj->Get("f")); + EXPECT_EQ("{\"f\":\"hello\"}", m.metadata->ToString()); } TEST(MetadataTest, MetadataCorrectlyCloned) { @@ -214,9 +212,7 @@ TEST(MetadataTest, MetadataCorrectlyCloned) { EXPECT_FALSE(m_clone.is_deleted); EXPECT_EQ(m.created_at, m_clone.created_at); EXPECT_EQ(m.collected_at, m_clone.collected_at); - const json::Object* obj = m_clone.metadata->As(); - EXPECT_TRUE(obj->Has("f")); - EXPECT_EQ("hello", obj->Get("f")); + EXPECT_EQ(m.metadata->ToString(), m_clone.metadata->ToString()); } TEST(MetadataTest, IgnoredMetadataCorrectlyCreated) { From 6797d9822653580a242fc499c8b071ae08b95765 Mon Sep 17 00:00:00 2001 From: Dhrupad Bhardwaj Date: Thu, 29 Mar 2018 18:01:21 -0400 Subject: [PATCH 7/9] Address feedback --- test/store_unittest.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/store_unittest.cc b/test/store_unittest.cc index 7d34fafd..5ebaba82 100644 --- a/test/store_unittest.cc +++ b/test/store_unittest.cc @@ -28,11 +28,11 @@ TEST_F(MetadataStoreTest, ResourceWithOneIdCorrectlyStored) { EXPECT_EQ(mr, store.LookupResource("id")); } -TEST_F(MetadataStoreTest, EmptyStoreThrowsError) { +TEST_F(MetadataStoreTest, EmptyStoreLookupThrowsError) { EXPECT_THROW(store.LookupResource("missing_id"), std::out_of_range); } -TEST_F(MetadataStoreTest, UnknownResourceIdThrowsError) { +TEST_F(MetadataStoreTest, ResourceLookupFailuresAreIndependent) { MonitoredResource mr("type", {}); store.UpdateResource({"id"}, mr); EXPECT_THROW(store.LookupResource("missing_id"), std::out_of_range); @@ -223,7 +223,7 @@ TEST(MetadataTest, IgnoredMetadataCorrectlyCreated) { TEST(MetadataTest, IgnoredMetadataCorrectlyCloned) { MetadataStore::Metadata m = MetadataStore::Metadata::IGNORED(); MetadataStore::Metadata m_clone = m.Clone(); - EXPECT_TRUE(m_clone.ignore); + EXPECT_EQ(m.ignore, m_clone.ignore); } } // namespace From 1d173e55aad5f8fecbfad39e0cce77932bcff42d Mon Sep 17 00:00:00 2001 From: Dhrupad Bhardwaj Date: Thu, 29 Mar 2018 19:46:23 -0400 Subject: [PATCH 8/9] Address feedback, revert bad changes --- test/store_unittest.cc | 121 +++++++++++++++++++++-------------------- 1 file changed, 61 insertions(+), 60 deletions(-) diff --git a/test/store_unittest.cc b/test/store_unittest.cc index 5ebaba82..656d2ea9 100644 --- a/test/store_unittest.cc +++ b/test/store_unittest.cc @@ -23,9 +23,9 @@ class MetadataStoreTest : public ::testing::Test { }; TEST_F(MetadataStoreTest, ResourceWithOneIdCorrectlyStored) { - MonitoredResource mr("type", {}); - store.UpdateResource({"id"}, mr); - EXPECT_EQ(mr, store.LookupResource("id")); + MonitoredResource resource("type", {}); + store.UpdateResource({"id"}, resource); + EXPECT_EQ(resource, store.LookupResource("id")); } TEST_F(MetadataStoreTest, EmptyStoreLookupThrowsError) { @@ -33,52 +33,52 @@ TEST_F(MetadataStoreTest, EmptyStoreLookupThrowsError) { } TEST_F(MetadataStoreTest, ResourceLookupFailuresAreIndependent) { - MonitoredResource mr("type", {}); - store.UpdateResource({"id"}, mr); + MonitoredResource resource("type", {}); + store.UpdateResource({"id"}, resource); EXPECT_THROW(store.LookupResource("missing_id"), std::out_of_range); - EXPECT_EQ(mr, store.LookupResource("id")); + EXPECT_EQ(resource, store.LookupResource("id")); } TEST_F(MetadataStoreTest, MultipleResourcesWithSingleIdsCorrectlyStored) { - MonitoredResource mr1("type1", {}); - MonitoredResource mr2("type2", {}); - store.UpdateResource({"id1"}, mr1); - store.UpdateResource({"id2"}, mr2); - EXPECT_EQ(mr2, store.LookupResource("id2")); - EXPECT_EQ(mr1, store.LookupResource("id1")); + MonitoredResource resource1("type1", {}); + MonitoredResource resource2("type2", {}); + store.UpdateResource({"id1"}, resource1); + store.UpdateResource({"id2"}, resource2); + EXPECT_EQ(resource2, store.LookupResource("id2")); + EXPECT_EQ(resource1, store.LookupResource("id1")); } TEST_F(MetadataStoreTest, SingleResourceWithMultipleIdsCorrectlyStored) { - MonitoredResource mr("type", {}); - store.UpdateResource({"id1", "id2", "id3"}, mr); - EXPECT_EQ(mr, store.LookupResource("id3")); - EXPECT_EQ(mr, store.LookupResource("id1")); - EXPECT_EQ(mr, store.LookupResource("id2")); + MonitoredResource resource("type", {}); + store.UpdateResource({"id1", "id2", "id3"}, resource); + EXPECT_EQ(resource, store.LookupResource("id3")); + EXPECT_EQ(resource, store.LookupResource("id1")); + EXPECT_EQ(resource, store.LookupResource("id2")); } TEST_F(MetadataStoreTest, MultipleResourcesAndIdsCorrectlyStored) { - MonitoredResource mr1("type1", {}); - MonitoredResource mr2("type2", {}); - store.UpdateResource({"id1", "id2", "id3"}, mr1); - store.UpdateResource({"id-a", "id-b", "id-c"}, mr2); - EXPECT_EQ(mr1, store.LookupResource("id1")); - EXPECT_EQ(mr1, store.LookupResource("id2")); - EXPECT_EQ(mr1, store.LookupResource("id3")); - EXPECT_EQ(mr2, store.LookupResource("id-a")); - EXPECT_EQ(mr2, store.LookupResource("id-b")); - EXPECT_EQ(mr2, store.LookupResource("id-c")); + MonitoredResource resource1("type1", {}); + MonitoredResource resource2("type2", {}); + store.UpdateResource({"id1", "id2", "id3"}, resource1); + store.UpdateResource({"id-a", "id-b", "id-c"}, resource2); + EXPECT_EQ(resource1, store.LookupResource("id1")); + EXPECT_EQ(resource1, store.LookupResource("id2")); + EXPECT_EQ(resource1, store.LookupResource("id3")); + EXPECT_EQ(resource2, store.LookupResource("id-a")); + EXPECT_EQ(resource2, store.LookupResource("id-b")); + EXPECT_EQ(resource2, store.LookupResource("id-c")); } TEST_F(MetadataStoreTest, ResourceToIdsAssociationCorrectlyUpdated) { - MonitoredResource mr("type", {}); - store.UpdateResource({"id1", "id2"}, mr); - EXPECT_EQ(mr, store.LookupResource("id1")); - EXPECT_EQ(mr, store.LookupResource("id2")); - store.UpdateResource({"id-a", "id-b"}, mr); - EXPECT_EQ(mr, store.LookupResource("id1")); - EXPECT_EQ(mr, store.LookupResource("id2")); - EXPECT_EQ(mr, store.LookupResource("id-a")); - EXPECT_EQ(mr, store.LookupResource("id-b")); + MonitoredResource resource("type", {}); + store.UpdateResource({"id1", "id2"}, resource); + EXPECT_EQ(resource, store.LookupResource("id1")); + EXPECT_EQ(resource, store.LookupResource("id2")); + store.UpdateResource({"id-a", "id-b"}, resource); + EXPECT_EQ(resource, store.LookupResource("id1")); + EXPECT_EQ(resource, store.LookupResource("id2")); + EXPECT_EQ(resource, store.LookupResource("id-a")); + EXPECT_EQ(resource, store.LookupResource("id-b")); } TEST_F(MetadataStoreTest, DefaultMetadataMapIsEmpty) { @@ -87,29 +87,29 @@ TEST_F(MetadataStoreTest, DefaultMetadataMapIsEmpty) { } TEST_F(MetadataStoreTest, UpdateResourceDoesNotUpdateMetadata) { - MonitoredResource mr("type", {}); - store.UpdateResource({"id1"}, mr); + MonitoredResource resource("type", {}); + store.UpdateResource({"id1"}, resource); const auto metadata_map = GetMetadataMap(); EXPECT_TRUE(metadata_map.empty()); } TEST_F(MetadataStoreTest, UpdateMetadataChangesMetadataMap) { - MonitoredResource mr("type", {}); + MonitoredResource resource("type", {}); MetadataStore::Metadata m( "default-version", false, std::chrono::system_clock::now(), std::chrono::system_clock::now(), json::object({{"f", json::string("hello")}})); - store.UpdateMetadata(mr, std::move(m)); + store.UpdateMetadata(resource, std::move(m)); const auto metadata_map = GetMetadataMap(); EXPECT_EQ(1, metadata_map.size()); - EXPECT_EQ("default-version", metadata_map.at(mr).version); + EXPECT_EQ("default-version", metadata_map.at(resource).version); } TEST_F(MetadataStoreTest, MultipleUpdateMetadataChangesMetadataMap) { - MonitoredResource mr1("type1", {}); - MonitoredResource mr2("type2", {}); + MonitoredResource resource1("type1", {}); + MonitoredResource resource2("type2", {}); MetadataStore::Metadata m1( "default-version1", false, @@ -122,41 +122,41 @@ TEST_F(MetadataStoreTest, MultipleUpdateMetadataChangesMetadataMap) { std::chrono::system_clock::now(), std::chrono::system_clock::now(), json::object({{"f", json::string("hello")}})); - store.UpdateMetadata(mr1, std::move(m1)); - store.UpdateMetadata(mr2, std::move(m2)); + store.UpdateMetadata(resource1, std::move(m1)); + store.UpdateMetadata(resource2, std::move(m2)); const auto metadata_map = GetMetadataMap(); EXPECT_EQ(2, metadata_map.size()); - EXPECT_EQ("default-version1", metadata_map.at(mr1).version); - EXPECT_EQ("default-version2", metadata_map.at(mr2).version); + EXPECT_EQ("default-version1", metadata_map.at(resource1).version); + EXPECT_EQ("default-version2", metadata_map.at(resource2).version); } TEST_F(MetadataStoreTest, UpdateMetadataForResourceChangesMetadataEntry) { - MonitoredResource mr("type", {}); + MonitoredResource resource("type", {}); MetadataStore::Metadata m1( "default-version1", false, std::chrono::system_clock::now(), std::chrono::system_clock::now(), json::object({{"f", json::string("hello")}})); - store.UpdateMetadata(mr, std::move(m1)); + store.UpdateMetadata(resource, std::move(m1)); const auto metadata_map_before = GetMetadataMap(); EXPECT_EQ(1, metadata_map_before.size()); - EXPECT_EQ("default-version1", metadata_map_before.at(mr).version); + EXPECT_EQ("default-version1", metadata_map_before.at(resource).version); MetadataStore::Metadata m2( "default-version2", false, std::chrono::system_clock::now(), std::chrono::system_clock::now(), json::object({{"f", json::string("hello")}})); - store.UpdateMetadata(mr, std::move(m2)); + store.UpdateMetadata(resource, std::move(m2)); const auto metadata_map_after = GetMetadataMap(); EXPECT_EQ(1, metadata_map_after.size()); - EXPECT_EQ("default-version2", metadata_map_after.at(mr).version); + EXPECT_EQ("default-version2", metadata_map_after.at(resource).version); } TEST_F(MetadataStoreTest, PurgeDeletedEntriesDeletesCorrectMetadata) { - MonitoredResource mr1("type1", {}); - MonitoredResource mr2("type2", {}); + MonitoredResource resource1("type1", {}); + MonitoredResource resource2("type2", {}); MetadataStore::Metadata m1( "default-version1", false, @@ -169,17 +169,17 @@ TEST_F(MetadataStoreTest, PurgeDeletedEntriesDeletesCorrectMetadata) { std::chrono::system_clock::now(), std::chrono::system_clock::now(), json::object({{"f", json::string("hello")}})); - store.UpdateMetadata(mr1, std::move(m1)); - store.UpdateMetadata(mr2, std::move(m2)); + store.UpdateMetadata(resource1, std::move(m1)); + store.UpdateMetadata(resource2, std::move(m2)); const auto metadata_map_before = GetMetadataMap(); EXPECT_EQ(2, metadata_map_before.size()); - EXPECT_EQ("default-version1", metadata_map_before.at(mr1).version); - EXPECT_EQ("default-version2", metadata_map_before.at(mr2).version); + EXPECT_EQ("default-version1", metadata_map_before.at(resource1).version); + EXPECT_EQ("default-version2", metadata_map_before.at(resource2).version); PurgeDeletedEntries(); const auto metadata_map_after = GetMetadataMap(); EXPECT_EQ(1, metadata_map_after.size()); - EXPECT_EQ("default-version1", metadata_map_after.at(mr1).version); - EXPECT_THROW(metadata_map_after.at(mr2), std::out_of_range); + EXPECT_EQ("default-version1", metadata_map_after.at(resource1).version); + EXPECT_THROW(metadata_map_after.at(resource2), std::out_of_range); } TEST(MetadataTest, MetadataCorrectlyConstructed) { @@ -223,6 +223,7 @@ TEST(MetadataTest, IgnoredMetadataCorrectlyCreated) { TEST(MetadataTest, IgnoredMetadataCorrectlyCloned) { MetadataStore::Metadata m = MetadataStore::Metadata::IGNORED(); MetadataStore::Metadata m_clone = m.Clone(); + EXPECT_TRUE(m.ignore); EXPECT_EQ(m.ignore, m_clone.ignore); } From 87eceaa9806a3359b0c8431719b4852153e9d94c Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Thu, 29 Mar 2018 20:39:53 -0400 Subject: [PATCH 9/9] Make the clone of ignored metadata explicit; remove redundant check. --- src/store.h | 2 +- test/store_unittest.cc | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/store.h b/src/store.h index fef720b8..b21008db 100644 --- a/src/store.h +++ b/src/store.h @@ -52,7 +52,7 @@ class MetadataStore { Metadata Clone() const { if (ignore) { - return {}; + return IGNORED(); } return {version, is_deleted, created_at, collected_at, metadata->Clone()}; } diff --git a/test/store_unittest.cc b/test/store_unittest.cc index 656d2ea9..7f5cd846 100644 --- a/test/store_unittest.cc +++ b/test/store_unittest.cc @@ -223,7 +223,6 @@ TEST(MetadataTest, IgnoredMetadataCorrectlyCreated) { TEST(MetadataTest, IgnoredMetadataCorrectlyCloned) { MetadataStore::Metadata m = MetadataStore::Metadata::IGNORED(); MetadataStore::Metadata m_clone = m.Clone(); - EXPECT_TRUE(m.ignore); EXPECT_EQ(m.ignore, m_clone.ignore); }