From 7e45279f8cdf76f5a7f2ce1dc127878a5c144fb1 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Thu, 22 Aug 2019 08:23:13 -0400 Subject: [PATCH] use SymbolTableCreator rather than fakes in a few stray places. Signed-off-by: Joshua Marantz --- test/common/stats/BUILD | 6 +-- test/common/stats/allocator_impl_test.cc | 12 +++-- test/common/stats/metric_impl_test.cc | 10 ++-- test/common/stats/stat_merger_test.cc | 6 +-- .../stats/thread_local_store_speed_test.cc | 10 ++-- test/common/stats/thread_local_store_test.cc | 52 ++++++++++--------- 6 files changed, 52 insertions(+), 44 deletions(-) diff --git a/test/common/stats/BUILD b/test/common/stats/BUILD index 78157bdee706..ab725c0a2608 100644 --- a/test/common/stats/BUILD +++ b/test/common/stats/BUILD @@ -15,7 +15,7 @@ envoy_cc_test( srcs = ["allocator_impl_test.cc"], deps = [ "//source/common/stats:allocator_lib", - "//source/common/stats:fake_symbol_table_lib", + "//source/common/stats:symbol_table_creator_lib", "//test/test_common:logging_lib", ], ) @@ -33,7 +33,7 @@ envoy_cc_test( srcs = ["metric_impl_test.cc"], deps = [ "//source/common/stats:allocator_lib", - "//source/common/stats:fake_symbol_table_lib", + "//source/common/stats:symbol_table_creator_lib", "//source/common/stats:utility_lib", "//test/test_common:logging_lib", ], @@ -43,9 +43,9 @@ envoy_cc_test( name = "stat_merger_test", srcs = ["stat_merger_test.cc"], deps = [ - "//source/common/stats:fake_symbol_table_lib", "//source/common/stats:isolated_store_lib", "//source/common/stats:stat_merger_lib", + "//source/common/stats:symbol_table_creator_lib", "//source/common/stats:thread_local_store_lib", "//test/test_common:utility_lib", ], diff --git a/test/common/stats/allocator_impl_test.cc b/test/common/stats/allocator_impl_test.cc index 37339b138478..f1b646a46b29 100644 --- a/test/common/stats/allocator_impl_test.cc +++ b/test/common/stats/allocator_impl_test.cc @@ -1,7 +1,7 @@ #include #include "common/stats/allocator_impl.h" -#include "common/stats/fake_symbol_table_impl.h" +#include "common/stats/symbol_table_creator.h" #include "test/test_common/logging.h" @@ -13,21 +13,23 @@ namespace { class AllocatorImplTest : public testing::Test { protected: - AllocatorImplTest() : alloc_(symbol_table_), pool_(symbol_table_) {} + AllocatorImplTest() + : symbol_table_(SymbolTableCreator::makeSymbolTable()), alloc_(*symbol_table_), + pool_(*symbol_table_) {} ~AllocatorImplTest() override { clearStorage(); } StatNameStorage makeStatStorage(absl::string_view name) { - return StatNameStorage(name, symbol_table_); + return StatNameStorage(name, *symbol_table_); } StatName makeStat(absl::string_view name) { return pool_.add(name); } void clearStorage() { pool_.clear(); - EXPECT_EQ(0, symbol_table_.numSymbols()); + EXPECT_EQ(0, symbol_table_->numSymbols()); } - FakeSymbolTableImpl symbol_table_; + SymbolTablePtr symbol_table_; AllocatorImpl alloc_; StatNamePool pool_; }; diff --git a/test/common/stats/metric_impl_test.cc b/test/common/stats/metric_impl_test.cc index 16e90a220199..b178842fa865 100644 --- a/test/common/stats/metric_impl_test.cc +++ b/test/common/stats/metric_impl_test.cc @@ -1,7 +1,7 @@ #include #include "common/stats/allocator_impl.h" -#include "common/stats/fake_symbol_table_impl.h" +#include "common/stats/symbol_table_creator.h" #include "common/stats/utility.h" #include "test/test_common/logging.h" @@ -14,17 +14,19 @@ namespace { class MetricImplTest : public testing::Test { protected: - MetricImplTest() : alloc_(symbol_table_), pool_(symbol_table_) {} + MetricImplTest() + : symbol_table_(SymbolTableCreator::makeSymbolTable()), alloc_(*symbol_table_), + pool_(*symbol_table_) {} ~MetricImplTest() override { clearStorage(); } StatName makeStat(absl::string_view name) { return pool_.add(name); } void clearStorage() { pool_.clear(); - EXPECT_EQ(0, symbol_table_.numSymbols()); + EXPECT_EQ(0, symbol_table_->numSymbols()); } - FakeSymbolTableImpl symbol_table_; + SymbolTablePtr symbol_table_; AllocatorImpl alloc_; StatNamePool pool_; }; diff --git a/test/common/stats/stat_merger_test.cc b/test/common/stats/stat_merger_test.cc index f56e8d98c718..4168bef2218c 100644 --- a/test/common/stats/stat_merger_test.cc +++ b/test/common/stats/stat_merger_test.cc @@ -1,8 +1,8 @@ #include -#include "common/stats/fake_symbol_table_impl.h" #include "common/stats/isolated_store_impl.h" #include "common/stats/stat_merger.h" +#include "common/stats/symbol_table_creator.h" #include "common/stats/thread_local_store.h" #include "test/test_common/utility.h" @@ -182,8 +182,8 @@ TEST_F(StatMergerTest, gaugeMergeImportMode) { class StatMergerThreadLocalTest : public testing::Test { protected: - FakeSymbolTableImpl symbol_table_; - AllocatorImpl alloc_{symbol_table_}; + SymbolTablePtr symbol_table_{SymbolTableCreator::makeSymbolTable()}; + AllocatorImpl alloc_{*symbol_table_}; ThreadLocalStoreImpl store_{alloc_}; }; diff --git a/test/common/stats/thread_local_store_speed_test.cc b/test/common/stats/thread_local_store_speed_test.cc index 877c6bc7bb09..6ea30cb61e84 100644 --- a/test/common/stats/thread_local_store_speed_test.cc +++ b/test/common/stats/thread_local_store_speed_test.cc @@ -22,18 +22,18 @@ namespace Envoy { class ThreadLocalStorePerf { public: ThreadLocalStorePerf() - : heap_alloc_(symbol_table_), store_(heap_alloc_), - api_(Api::createApiForTest(store_, time_system_)) { + : symbol_table_(Stats::SymbolTableCreator::makeSymbolTable()), heap_alloc_(*symbol_table_), + store_(heap_alloc_), api_(Api::createApiForTest(store_, time_system_)) { store_.setTagProducer(std::make_unique(stats_config_)); Stats::TestUtil::forEachSampleStat(1000, [this](absl::string_view name) { - stat_names_.push_back(std::make_unique(name, symbol_table_)); + stat_names_.push_back(std::make_unique(name, *symbol_table_)); }); } ~ThreadLocalStorePerf() { for (auto& stat_name_storage : stat_names_) { - stat_name_storage->free(symbol_table_); + stat_name_storage->free(*symbol_table_); } store_.shutdownThreading(); if (tls_) { @@ -54,7 +54,7 @@ class ThreadLocalStorePerf { } private: - Stats::FakeSymbolTableImpl symbol_table_; + Stats::SymbolTablePtr symbol_table_; Event::SimulatedTimeSystem time_system_; Stats::AllocatorImpl heap_alloc_; Stats::ThreadLocalStoreImpl store_; diff --git a/test/common/stats/thread_local_store_test.cc b/test/common/stats/thread_local_store_test.cc index be0b87058ced..fb4b2d594ee4 100644 --- a/test/common/stats/thread_local_store_test.cc +++ b/test/common/stats/thread_local_store_test.cc @@ -39,7 +39,8 @@ const uint64_t MaxStatNameLength = 127; class StatsThreadLocalStoreTest : public testing::Test { public: StatsThreadLocalStoreTest() - : alloc_(symbol_table_), store_(std::make_unique(alloc_)) { + : symbol_table_(SymbolTableCreator::makeSymbolTable()), alloc_(*symbol_table_), + store_(std::make_unique(alloc_)) { store_->addSink(sink_); } @@ -48,7 +49,7 @@ class StatsThreadLocalStoreTest : public testing::Test { store_->addSink(sink_); } - Stats::FakeSymbolTableImpl symbol_table_; + SymbolTablePtr symbol_table_; NiceMock main_thread_dispatcher_; NiceMock tls_; AllocatorImpl alloc_; @@ -78,7 +79,7 @@ class HistogramTest : public testing::Test { public: using NameHistogramMap = std::map; - HistogramTest() : alloc_(symbol_table_) {} + HistogramTest() : symbol_table_(SymbolTableCreator::makeSymbolTable()), alloc_(*symbol_table_) {} void SetUp() override { store_ = std::make_unique(alloc_); @@ -166,7 +167,7 @@ class HistogramTest : public testing::Test { } } - FakeSymbolTableImpl symbol_table_; + SymbolTablePtr symbol_table_; NiceMock main_thread_dispatcher_; NiceMock tls_; AllocatorImpl alloc_; @@ -182,7 +183,7 @@ TEST_F(StatsThreadLocalStoreTest, NoTls) { Counter& c1 = store_->counter("c1"); EXPECT_EQ(&c1, &store_->counter("c1")); - StatNameManagedStorage c1_name("c1", symbol_table_); + StatNameManagedStorage c1_name("c1", *symbol_table_); c1.add(100); auto found_counter = store_->findCounter(c1_name.statName()); ASSERT_TRUE(found_counter.has_value()); @@ -193,7 +194,7 @@ TEST_F(StatsThreadLocalStoreTest, NoTls) { Gauge& g1 = store_->gauge("g1", Gauge::ImportMode::Accumulate); EXPECT_EQ(&g1, &store_->gauge("g1", Gauge::ImportMode::Accumulate)); - StatNameManagedStorage g1_name("g1", symbol_table_); + StatNameManagedStorage g1_name("g1", *symbol_table_); g1.set(100); auto found_gauge = store_->findGauge(g1_name.statName()); ASSERT_TRUE(found_gauge.has_value()); @@ -204,7 +205,7 @@ TEST_F(StatsThreadLocalStoreTest, NoTls) { Histogram& h1 = store_->histogram("h1"); EXPECT_EQ(&h1, &store_->histogram("h1")); - StatNameManagedStorage h1_name("h1", symbol_table_); + StatNameManagedStorage h1_name("h1", *symbol_table_); auto found_histogram = store_->findHistogram(h1_name.statName()); ASSERT_TRUE(found_histogram.has_value()); EXPECT_EQ(&h1, &found_histogram->get()); @@ -230,7 +231,7 @@ TEST_F(StatsThreadLocalStoreTest, Tls) { Counter& c1 = store_->counter("c1"); EXPECT_EQ(&c1, &store_->counter("c1")); - StatNameManagedStorage c1_name("c1", symbol_table_); + StatNameManagedStorage c1_name("c1", *symbol_table_); c1.add(100); auto found_counter = store_->findCounter(c1_name.statName()); ASSERT_TRUE(found_counter.has_value()); @@ -241,7 +242,7 @@ TEST_F(StatsThreadLocalStoreTest, Tls) { Gauge& g1 = store_->gauge("g1", Gauge::ImportMode::Accumulate); EXPECT_EQ(&g1, &store_->gauge("g1", Gauge::ImportMode::Accumulate)); - StatNameManagedStorage g1_name("g1", symbol_table_); + StatNameManagedStorage g1_name("g1", *symbol_table_); g1.set(100); auto found_gauge = store_->findGauge(g1_name.statName()); ASSERT_TRUE(found_gauge.has_value()); @@ -252,7 +253,7 @@ TEST_F(StatsThreadLocalStoreTest, Tls) { Histogram& h1 = store_->histogram("h1"); EXPECT_EQ(&h1, &store_->histogram("h1")); - StatNameManagedStorage h1_name("h1", symbol_table_); + StatNameManagedStorage h1_name("h1", *symbol_table_); auto found_histogram = store_->findHistogram(h1_name.statName()); ASSERT_TRUE(found_histogram.has_value()); EXPECT_EQ(&h1, &found_histogram->get()); @@ -284,11 +285,11 @@ TEST_F(StatsThreadLocalStoreTest, BasicScope) { Counter& c2 = scope1->counter("c2"); EXPECT_EQ("c1", c1.name()); EXPECT_EQ("scope1.c2", c2.name()); - StatNameManagedStorage c1_name("c1", symbol_table_); + StatNameManagedStorage c1_name("c1", *symbol_table_); auto found_counter = store_->findCounter(c1_name.statName()); ASSERT_TRUE(found_counter.has_value()); EXPECT_EQ(&c1, &found_counter->get()); - StatNameManagedStorage c2_name("scope1.c2", symbol_table_); + StatNameManagedStorage c2_name("scope1.c2", *symbol_table_); auto found_counter2 = store_->findCounter(c2_name.statName()); ASSERT_TRUE(found_counter2.has_value()); EXPECT_EQ(&c2, &found_counter2->get()); @@ -297,11 +298,11 @@ TEST_F(StatsThreadLocalStoreTest, BasicScope) { Gauge& g2 = scope1->gauge("g2", Gauge::ImportMode::Accumulate); EXPECT_EQ("g1", g1.name()); EXPECT_EQ("scope1.g2", g2.name()); - StatNameManagedStorage g1_name("g1", symbol_table_); + StatNameManagedStorage g1_name("g1", *symbol_table_); auto found_gauge = store_->findGauge(g1_name.statName()); ASSERT_TRUE(found_gauge.has_value()); EXPECT_EQ(&g1, &found_gauge->get()); - StatNameManagedStorage g2_name("scope1.g2", symbol_table_); + StatNameManagedStorage g2_name("scope1.g2", *symbol_table_); auto found_gauge2 = store_->findGauge(g2_name.statName()); ASSERT_TRUE(found_gauge2.has_value()); EXPECT_EQ(&g2, &found_gauge2->get()); @@ -314,11 +315,11 @@ TEST_F(StatsThreadLocalStoreTest, BasicScope) { h1.recordValue(100); EXPECT_CALL(sink_, onHistogramComplete(Ref(h2), 200)); h2.recordValue(200); - StatNameManagedStorage h1_name("h1", symbol_table_); + StatNameManagedStorage h1_name("h1", *symbol_table_); auto found_histogram = store_->findHistogram(h1_name.statName()); ASSERT_TRUE(found_histogram.has_value()); EXPECT_EQ(&h1, &found_histogram->get()); - StatNameManagedStorage h2_name("scope1.h2", symbol_table_); + StatNameManagedStorage h2_name("scope1.h2", *symbol_table_); auto found_histogram2 = store_->findHistogram(h2_name.statName()); ASSERT_TRUE(found_histogram2.has_value()); EXPECT_EQ(&h2, &found_histogram2->get()); @@ -380,7 +381,7 @@ TEST_F(StatsThreadLocalStoreTest, NestedScopes) { ScopePtr scope1 = store_->createScope("scope1."); Counter& c1 = scope1->counter("foo.bar"); EXPECT_EQ("scope1.foo.bar", c1.name()); - StatNameManagedStorage c1_name("scope1.foo.bar", symbol_table_); + StatNameManagedStorage c1_name("scope1.foo.bar", *symbol_table_); auto found_counter = store_->findCounter(c1_name.statName()); ASSERT_TRUE(found_counter.has_value()); EXPECT_EQ(&c1, &found_counter->get()); @@ -389,7 +390,7 @@ TEST_F(StatsThreadLocalStoreTest, NestedScopes) { Counter& c2 = scope2->counter("bar"); EXPECT_EQ(&c1, &c2); EXPECT_EQ("scope1.foo.bar", c2.name()); - StatNameManagedStorage c2_name("scope1.foo.bar", symbol_table_); + StatNameManagedStorage c2_name("scope1.foo.bar", *symbol_table_); auto found_counter2 = store_->findCounter(c2_name.statName()); ASSERT_TRUE(found_counter2.has_value()); @@ -455,12 +456,14 @@ TEST_F(StatsThreadLocalStoreTest, OverlappingScopes) { class LookupWithStatNameTest : public testing::Test { public: - LookupWithStatNameTest() : alloc_(symbol_table_), store_(alloc_), pool_(symbol_table_) {} + LookupWithStatNameTest() + : symbol_table_(SymbolTableCreator::makeSymbolTable()), alloc_(*symbol_table_), + store_(alloc_), pool_(*symbol_table_) {} ~LookupWithStatNameTest() override { store_.shutdownThreading(); } StatName makeStatName(absl::string_view name) { return pool_.add(name); } - Stats::FakeSymbolTableImpl symbol_table_; + SymbolTablePtr symbol_table_; AllocatorImpl alloc_; ThreadLocalStoreImpl store_; StatNamePool pool_; @@ -664,7 +667,8 @@ TEST_F(StatsMatcherTLSTest, TestExclusionRegex) { class RememberStatsMatcherTest : public testing::TestWithParam { public: RememberStatsMatcherTest() - : heap_alloc_(symbol_table_), store_(heap_alloc_), scope_(store_.createScope("scope.")) { + : symbol_table_(SymbolTableCreator::makeSymbolTable()), heap_alloc_(*symbol_table_), + store_(heap_alloc_), scope_(store_.createScope("scope.")) { if (GetParam()) { store_.initializeThreading(main_thread_dispatcher_, tls_); } @@ -755,7 +759,7 @@ class RememberStatsMatcherTest : public testing::TestWithParam { }; } - Stats::FakeSymbolTableImpl symbol_table_; + Stats::SymbolTablePtr symbol_table_; NiceMock main_thread_dispatcher_; NiceMock tls_; AllocatorImpl heap_alloc_; @@ -968,11 +972,11 @@ TEST_F(StatsThreadLocalStoreTest, MergeDuringShutDown) { } TEST(ThreadLocalStoreThreadTest, ConstructDestruct) { - Stats::FakeSymbolTableImpl symbol_table; + SymbolTablePtr symbol_table(SymbolTableCreator::makeSymbolTable()); Api::ApiPtr api = Api::createApiForTest(); Event::DispatcherPtr dispatcher = api->allocateDispatcher(); NiceMock tls; - AllocatorImpl alloc(symbol_table); + AllocatorImpl alloc(*symbol_table); ThreadLocalStoreImpl store(alloc); store.initializeThreading(*dispatcher, tls);