From f464387404d7300e2cd552f1ce8f3d95890eee95 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Fri, 8 Feb 2019 15:20:25 -0500 Subject: [PATCH 1/3] Add 0-arg createApiForTest() and use it where the stats are not otherwise used. Signed-off-by: Joshua Marantz --- test/common/config/utility_test.cc | 3 +-- test/common/event/dispatcher_impl_test.cc | 6 ++---- test/common/event/file_event_impl_test.cc | 6 ++---- test/common/grpc/async_client_manager_impl_test.cc | 3 +-- test/common/grpc/google_grpc_creds_test.cc | 3 +-- test/common/json/config_schemas_test.cc | 3 +-- test/common/json/json_loader_test.cc | 3 +-- test/common/network/connection_impl_test.cc | 12 ++++-------- test/common/network/dns_impl_test.cc | 6 ++---- test/common/network/listener_impl_test.cc | 9 ++------- test/common/network/udp_listener_impl_test.cc | 3 +-- test/common/protobuf/utility_test.cc | 3 +-- test/common/router/config_impl_test.cc | 3 +-- test/common/runtime/runtime_impl_test.cc | 3 +-- test/common/secret/sds_api_test.cc | 3 +-- test/common/secret/secret_manager_impl_test.cc | 3 +-- test/common/thread_local/thread_local_impl_test.cc | 3 +-- test/config_test/config_test.cc | 9 +++------ test/test_common/BUILD | 1 + test/test_common/utility.cc | 13 +++++++++++-- test/test_common/utility.h | 1 + 21 files changed, 40 insertions(+), 59 deletions(-) diff --git a/test/common/config/utility_test.cc b/test/common/config/utility_test.cc index c3ebc97e5d235..d725a8f449b3a 100644 --- a/test/common/config/utility_test.cc +++ b/test/common/config/utility_test.cc @@ -208,8 +208,7 @@ TEST(UtilityTest, UnixClusterStatic) { } TEST(UtilityTest, CheckFilesystemSubscriptionBackingPath) { - Stats::MockIsolatedStatsStore stats_store; - Api::ApiPtr api = Api::createApiForTest(stats_store); + Api::ApiPtr api = Api::createApiForTest(); EXPECT_THROW_WITH_MESSAGE( Utility::checkFilesystemSubscriptionBackingPath("foo", *api), EnvoyException, diff --git a/test/common/event/dispatcher_impl_test.cc b/test/common/event/dispatcher_impl_test.cc index 6677fbfe679d9..619176c5ba9bc 100644 --- a/test/common/event/dispatcher_impl_test.cc +++ b/test/common/event/dispatcher_impl_test.cc @@ -29,8 +29,7 @@ class TestDeferredDeletable : public DeferredDeletable { TEST(DeferredDeleteTest, DeferredDelete) { InSequence s; - Stats::IsolatedStoreImpl stats_store; - Api::ApiPtr api = Api::createApiForTest(stats_store); + Api::ApiPtr api = Api::createApiForTest(); DispatcherImpl dispatcher(*api); ReadyWatcher watcher1; @@ -62,7 +61,7 @@ TEST(DeferredDeleteTest, DeferredDelete) { class DispatcherImplTest : public TestBase { protected: DispatcherImplTest() - : api_(Api::createApiForTest(stat_store_)), + : api_(Api::createApiForTest()), dispatcher_(std::make_unique(*api_)), work_finished_(false) { dispatcher_thread_ = api_->threadFactory().createThread([this]() { // Must create a keepalive timer to keep the dispatcher from exiting. @@ -80,7 +79,6 @@ class DispatcherImplTest : public TestBase { dispatcher_thread_->join(); } - Stats::IsolatedStoreImpl stat_store_; Api::ApiPtr api_; Thread::ThreadPtr dispatcher_thread_; DispatcherPtr dispatcher_; diff --git a/test/common/event/file_event_impl_test.cc b/test/common/event/file_event_impl_test.cc index d43dbeef0bc40..7e674ec4ffab1 100644 --- a/test/common/event/file_event_impl_test.cc +++ b/test/common/event/file_event_impl_test.cc @@ -15,7 +15,7 @@ namespace Event { class FileEventImplTest : public TestBase { public: - FileEventImplTest() : api_(Api::createApiForTest(stats_store_)), dispatcher_(*api_) {} + FileEventImplTest() : api_(Api::createApiForTest()), dispatcher_(*api_) {} void SetUp() override { int rc = socketpair(AF_UNIX, SOCK_DGRAM, 0, fds_); @@ -32,7 +32,6 @@ class FileEventImplTest : public TestBase { protected: int fds_[2]; - Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; DispatcherImpl dispatcher_; }; @@ -52,8 +51,7 @@ TEST_P(FileEventImplActivateTest, Activate) { } ASSERT_NE(-1, fd); - Stats::IsolatedStoreImpl stats_store; - Api::ApiPtr api = Api::createApiForTest(stats_store); + Api::ApiPtr api = Api::createApiForTest(); DispatcherImpl dispatcher(*api); ReadyWatcher read_event; EXPECT_CALL(read_event, ready()).Times(1); diff --git a/test/common/grpc/async_client_manager_impl_test.cc b/test/common/grpc/async_client_manager_impl_test.cc index 8d3c15f0a9fdc..a64ea8697020e 100644 --- a/test/common/grpc/async_client_manager_impl_test.cc +++ b/test/common/grpc/async_client_manager_impl_test.cc @@ -18,11 +18,10 @@ namespace { class AsyncClientManagerImplTest : public TestBase { public: - AsyncClientManagerImplTest() : api_(Api::createApiForTest(api_stats_store_)) {} + AsyncClientManagerImplTest() : api_(Api::createApiForTest()) {} Upstream::MockClusterManager cm_; NiceMock tls_; - Stats::IsolatedStoreImpl api_stats_store_; Stats::MockStore scope_; DangerousDeprecatedTestTime test_time_; Api::ApiPtr api_; diff --git a/test/common/grpc/google_grpc_creds_test.cc b/test/common/grpc/google_grpc_creds_test.cc index 1bbb3bd353401..7ba5c677636ba 100644 --- a/test/common/grpc/google_grpc_creds_test.cc +++ b/test/common/grpc/google_grpc_creds_test.cc @@ -16,9 +16,8 @@ namespace { class CredsUtilityTest : public TestBase { public: - CredsUtilityTest() : api_(Api::createApiForTest(stats_store_)) {} + CredsUtilityTest() : api_(Api::createApiForTest()) {} - Stats::MockIsolatedStatsStore stats_store_; Api::ApiPtr api_; }; diff --git a/test/common/json/config_schemas_test.cc b/test/common/json/config_schemas_test.cc index 5a4d6eba6dc8d..20da497190938 100644 --- a/test/common/json/config_schemas_test.cc +++ b/test/common/json/config_schemas_test.cc @@ -29,9 +29,8 @@ std::vector generateTestInputs() { class ConfigSchemasTest : public TestBaseWithParam { protected: - ConfigSchemasTest() : api_(Api::createApiForTest(stats_store_)) {} + ConfigSchemasTest() : api_(Api::createApiForTest()) {} - Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; }; diff --git a/test/common/json/json_loader_test.cc b/test/common/json/json_loader_test.cc index 2846fa4f0a556..9fc615e5cc3c3 100644 --- a/test/common/json/json_loader_test.cc +++ b/test/common/json/json_loader_test.cc @@ -12,9 +12,8 @@ namespace Json { class JsonLoaderTest : public TestBase { protected: - JsonLoaderTest() : api_(Api::createApiForTest(stats_store_)) {} + JsonLoaderTest() : api_(Api::createApiForTest()) {} - Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; }; diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc index f00422b5125ca..ab378a59bf4f1 100644 --- a/test/common/network/connection_impl_test.cc +++ b/test/common/network/connection_impl_test.cc @@ -78,9 +78,8 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, ConnectionImplDeathTest, TestUtility::ipTestParamsToString); TEST_P(ConnectionImplDeathTest, BadFd) { - Stats::IsolatedStoreImpl stats_store; Event::SimulatedTimeSystem time_system; - Api::ApiPtr api = Api::createApiForTest(stats_store); + Api::ApiPtr api = Api::createApiForTest(); Event::DispatcherImpl dispatcher(*api); IoHandlePtr io_handle = std::make_unique(); EXPECT_DEATH_LOG_TO_STDERR( @@ -92,7 +91,7 @@ TEST_P(ConnectionImplDeathTest, BadFd) { class ConnectionImplTest : public TestBaseWithParam { public: - ConnectionImplTest() : api_(Api::createApiForTest(stats_store_)) {} + ConnectionImplTest() : api_(Api::createApiForTest()) {} void setUpBasicConnection() { if (dispatcher_.get() == nullptr) { @@ -208,7 +207,6 @@ class ConnectionImplTest : public TestBaseWithParam { Event::SimulatedTimeSystem time_system_; Event::DispatcherPtr dispatcher_; - Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; Network::TcpListenSocket socket_{Network::Test::getAnyAddress(GetParam()), nullptr, true}; Network::MockListenerCallbacks listener_callbacks_; @@ -1645,9 +1643,8 @@ TEST_P(ReadBufferLimitTest, SomeLimit) { class TcpClientConnectionImplTest : public TestBaseWithParam { protected: - TcpClientConnectionImplTest() : api_(Api::createApiForTest(stats_store_)), dispatcher_(*api_) {} + TcpClientConnectionImplTest() : api_(Api::createApiForTest()), dispatcher_(*api_) {} - Stats::IsolatedStoreImpl stats_store_; Event::SimulatedTimeSystem time_system_; Api::ApiPtr api_; Event::DispatcherImpl dispatcher_; @@ -1689,9 +1686,8 @@ TEST_P(TcpClientConnectionImplTest, BadConnectConnRefused) { class PipeClientConnectionImplTest : public TestBase { protected: - PipeClientConnectionImplTest() : api_(Api::createApiForTest(stats_store_)), dispatcher_(*api_) {} + PipeClientConnectionImplTest() : api_(Api::createApiForTest()), dispatcher_(*api_) {} - Stats::IsolatedStoreImpl stats_store_; Event::SimulatedTimeSystem time_system_; Api::ApiPtr api_; Event::DispatcherImpl dispatcher_; diff --git a/test/common/network/dns_impl_test.cc b/test/common/network/dns_impl_test.cc index d5c0403d592d5..b03c4f0ba304b 100644 --- a/test/common/network/dns_impl_test.cc +++ b/test/common/network/dns_impl_test.cc @@ -324,9 +324,8 @@ class DnsResolverImplPeer { class DnsImplConstructor : public TestBase { protected: - DnsImplConstructor() : api_(Api::createApiForTest(stats_store_)), dispatcher_(*api_) {} + DnsImplConstructor() : api_(Api::createApiForTest()), dispatcher_(*api_) {} - Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; Event::DispatcherImpl dispatcher_; }; @@ -404,7 +403,7 @@ TEST_F(DnsImplConstructor, BadCustomResolvers) { class DnsImplTest : public TestBaseWithParam { public: - DnsImplTest() : api_(Api::createApiForTest(stats_store_)), dispatcher_(*api_) {} + DnsImplTest() : api_(Api::createApiForTest()), dispatcher_(*api_) {} void SetUp() override { resolver_ = dispatcher_.createDnsResolver({}); @@ -434,7 +433,6 @@ class DnsImplTest : public TestBaseWithParam { std::unique_ptr peer_; Network::MockConnectionHandler connection_handler_; Network::TcpListenSocketPtr socket_; - Stats::IsolatedStoreImpl stats_store_; std::unique_ptr listener_; Api::ApiPtr api_; Event::DispatcherImpl dispatcher_; diff --git a/test/common/network/listener_impl_test.cc b/test/common/network/listener_impl_test.cc index 05592b2072e45..f752d16ce6b28 100644 --- a/test/common/network/listener_impl_test.cc +++ b/test/common/network/listener_impl_test.cc @@ -21,8 +21,7 @@ namespace Network { static void errorCallbackTest(Address::IpVersion version) { // Force the error callback to fire by closing the socket under the listener. We run this entire // test in the forked process to avoid confusion when the fork happens. - Stats::IsolatedStoreImpl stats_store; - Api::ApiPtr api = Api::createApiForTest(stats_store); + Api::ApiPtr api = Api::createApiForTest(); Event::DispatcherImpl dispatcher(*api); Network::TcpListenSocket socket(Network::Test::getCanonicalLoopbackAddress(version), nullptr, @@ -77,11 +76,10 @@ class ListenerImplTest : public TestBaseWithParam { : version_(GetParam()), alt_address_(Network::Test::findOrCheckFreePort( Network::Test::getCanonicalLoopbackAddress(version_), Address::SocketType::Stream)), - api_(Api::createApiForTest(stats_store_)), dispatcher_(*api_) {} + api_(Api::createApiForTest()), dispatcher_(*api_) {} const Address::IpVersion version_; const Address::InstanceConstSharedPtr alt_address_; - Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; Event::DispatcherImpl dispatcher_; }; @@ -121,7 +119,6 @@ TEST_P(ListenerImplTest, SetListeningSocketOptionsError) { } TEST_P(ListenerImplTest, UseActualDst) { - Stats::IsolatedStoreImpl stats_store; Network::TcpListenSocket socket(Network::Test::getCanonicalLoopbackAddress(version_), nullptr, true); Network::TcpListenSocket socketDst(alt_address_, nullptr, false); @@ -158,7 +155,6 @@ TEST_P(ListenerImplTest, UseActualDst) { } TEST_P(ListenerImplTest, WildcardListenerUseActualDst) { - Stats::IsolatedStoreImpl stats_store; Network::TcpListenSocket socket(Network::Test::getAnyAddress(version_), nullptr, true); Network::MockListenerCallbacks listener_callbacks; Network::MockConnectionHandler connection_handler; @@ -196,7 +192,6 @@ TEST_P(ListenerImplTest, WildcardListenerUseActualDst) { // local and remote addresses of the connection should be IPv4 instances, as the connection really // is an IPv4 connection. TEST_P(ListenerImplTest, WildcardListenerIpv4Compat) { - Stats::IsolatedStoreImpl stats_store; auto option = std::make_unique(); auto options = std::make_shared>(); EXPECT_CALL(*option, setOption(_, envoy::api::v2::core::SocketOption::STATE_PREBIND)) diff --git a/test/common/network/udp_listener_impl_test.cc b/test/common/network/udp_listener_impl_test.cc index ae2639c7a6c56..990336736b3da 100644 --- a/test/common/network/udp_listener_impl_test.cc +++ b/test/common/network/udp_listener_impl_test.cc @@ -42,7 +42,7 @@ class UdpListenerImplTest : public TestBaseWithParam { : version_(GetParam()), alt_address_(Network::Test::findOrCheckFreePort( Network::Test::getCanonicalLoopbackAddress(version_), Address::SocketType::Stream)), - api_(Api::createApiForTest(stats_store_)), dispatcher_(*api_) {} + api_(Api::createApiForTest()), dispatcher_(*api_) {} SocketPtr getSocket(Address::SocketType type, const Address::InstanceConstSharedPtr& address, const Network::Socket::OptionsSharedPtr& options, bool bind) { @@ -115,7 +115,6 @@ class UdpListenerImplTest : public TestBaseWithParam { const Address::IpVersion version_; const Address::InstanceConstSharedPtr alt_address_; - Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; DangerousDeprecatedTestTime test_time_; Event::DispatcherImpl dispatcher_; diff --git a/test/common/protobuf/utility_test.cc b/test/common/protobuf/utility_test.cc index 1352ecd17910b..0ec2bd3a5a41e 100644 --- a/test/common/protobuf/utility_test.cc +++ b/test/common/protobuf/utility_test.cc @@ -17,9 +17,8 @@ namespace Envoy { class ProtobufUtilityTest : public TestBase { protected: - ProtobufUtilityTest() : api_(Api::createApiForTest(stats_store_)) {} + ProtobufUtilityTest() : api_(Api::createApiForTest()) {} - Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; }; diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 37926cb9518ad..25f0dc0a738f9 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -102,11 +102,10 @@ envoy::api::v2::RouteConfiguration parseRouteConfigurationFromV2Yaml(const std:: class ConfigImplTestBase { protected: - ConfigImplTestBase() : api_(Api::createApiForTest(stats_)) { + ConfigImplTestBase() : api_(Api::createApiForTest()) { ON_CALL(factory_context_, api()).WillByDefault(ReturnRef(*api_)); } - Stats::IsolatedStoreImpl stats_; Api::ApiPtr api_; NiceMock factory_context_; }; diff --git a/test/common/runtime/runtime_impl_test.cc b/test/common/runtime/runtime_impl_test.cc index aa99434d3bef4..a01659a6ec078 100644 --- a/test/common/runtime/runtime_impl_test.cc +++ b/test/common/runtime/runtime_impl_test.cc @@ -262,9 +262,8 @@ TEST(LoaderImplTest, All) { class DiskLayerTest : public TestBase { protected: - DiskLayerTest() : api_(Api::createApiForTest(store_)) {} + DiskLayerTest() : api_(Api::createApiForTest()) {} - Stats::IsolatedStoreImpl store_; Api::ApiPtr api_; }; diff --git a/test/common/secret/sds_api_test.cc b/test/common/secret/sds_api_test.cc index 39808f24d432d..d3bbdad07e7cb 100644 --- a/test/common/secret/sds_api_test.cc +++ b/test/common/secret/sds_api_test.cc @@ -28,9 +28,8 @@ namespace { class SdsApiTest : public TestBase { protected: - SdsApiTest() : api_(Api::createApiForTest(stats_store_)) {} + SdsApiTest() : api_(Api::createApiForTest()) {} - Stats::MockIsolatedStatsStore stats_store_; Api::ApiPtr api_; }; diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index 7b3aa38238f66..71c9ea1e94c36 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -24,9 +24,8 @@ namespace { class SecretManagerImplTest : public TestBase { protected: - SecretManagerImplTest() : api_(Api::createApiForTest(stats_store_)) {} + SecretManagerImplTest() : api_(Api::createApiForTest()) {} - Stats::MockIsolatedStatsStore stats_store_; Api::ApiPtr api_; }; diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index 99ff8eaa426cf..19ca1b3dea691 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -116,8 +116,7 @@ TEST_F(ThreadLocalInstanceImplTest, RunOnAllThreads) { TEST(ThreadLocalInstanceImplDispatcherTest, Dispatcher) { InstanceImpl tls; - Stats::IsolatedStoreImpl stats_store; - Api::ApiPtr api = Api::createApiForTest(stats_store); + Api::ApiPtr api = Api::createApiForTest(); Event::DispatcherImpl main_dispatcher(*api); Event::DispatcherImpl thread_dispatcher(*api); diff --git a/test/config_test/config_test.cc b/test/config_test/config_test.cc index e9aab4f379564..4b5543909738d 100644 --- a/test/config_test/config_test.cc +++ b/test/config_test/config_test.cc @@ -44,7 +44,7 @@ OptionsImpl asConfigYaml(const OptionsImpl& src, Api::Api& api) { class ConfigTest { public: ConfigTest(const OptionsImpl& options) - : api_(Api::createApiForTest(stats_store_)), options_(options) { + : api_(Api::createApiForTest()), options_(options) { ON_CALL(server_, options()).WillByDefault(ReturnRef(options_)); ON_CALL(server_, random()).WillByDefault(ReturnRef(random_)); ON_CALL(server_, sslContextManager()).WillByDefault(ReturnRef(ssl_context_manager_)); @@ -98,7 +98,6 @@ class ConfigTest { server_.thread_local_.shutdownThread(); } - Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; NiceMock server_; NiceMock ssl_context_manager_; @@ -114,8 +113,7 @@ class ConfigTest { }; void testMerge() { - Stats::IsolatedStoreImpl stats; - Api::ApiPtr api = Api::createApiForTest(stats); + Api::ApiPtr api = Api::createApiForTest(); const std::string overlay = "static_resources: { clusters: [{name: 'foo'}]}"; OptionsImpl options(Server::createTestOptionsImpl("google_com_proxy.v2.yaml", overlay, @@ -127,8 +125,7 @@ void testMerge() { uint32_t run(const std::string& directory) { uint32_t num_tested = 0; - Stats::IsolatedStoreImpl stats; - Api::ApiPtr api = Api::createApiForTest(stats); + Api::ApiPtr api = Api::createApiForTest(); for (const std::string& filename : TestUtility::listFiles(directory, false)) { ENVOY_LOG_MISC(info, "testing {}.\n", filename); OptionsImpl options( diff --git a/test/test_common/BUILD b/test/test_common/BUILD index 6a19c4e5a2ec5..29182405e928f 100644 --- a/test/test_common/BUILD +++ b/test/test_common/BUILD @@ -109,6 +109,7 @@ envoy_cc_test_library( "//source/common/protobuf:utility_lib", "//source/common/stats:stats_lib", "//source/common/stats:stats_options_lib", + "//test/mocks/stats:stats_mocks", "@envoy_api//envoy/config/bootstrap/v2:bootstrap_cc", ], ) diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc index e88e7a9d4e539..c411edd4a7f2b 100644 --- a/test/test_common/utility.cc +++ b/test/test_common/utility.cc @@ -42,6 +42,7 @@ #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" +#include "test/mocks/stats/mocks.h" #include "test/test_common/test_base.h" using testing::GTEST_FLAG(random_seed); @@ -401,18 +402,26 @@ ThreadFactory& threadFactoryForTest() { namespace Api { -class TimeSystemProvider { +class TestImplProvider { protected: Event::GlobalTimeSystem global_time_system_; + testing::NiceMock default_stats_store_; }; -class TestImpl : public TimeSystemProvider, public Impl { +class TestImpl : public TestImplProvider, public Impl { public: TestImpl(std::chrono::milliseconds file_flush_interval_msec, Thread::ThreadFactory& thread_factory, Stats::Store& stats_store) : Impl(file_flush_interval_msec, thread_factory, stats_store, global_time_system_) {} + TestImpl(std::chrono::milliseconds file_flush_interval_msec, + Thread::ThreadFactory& thread_factory) + : Impl(file_flush_interval_msec, thread_factory, default_stats_store_, global_time_system_) {} }; +ApiPtr createApiForTest() { + return std::make_unique(std::chrono::milliseconds(1000), Thread::threadFactoryForTest()); +} + ApiPtr createApiForTest(Stats::Store& stat_store) { return std::make_unique(std::chrono::milliseconds(1000), Thread::threadFactoryForTest(), stat_store); diff --git a/test/test_common/utility.h b/test/test_common/utility.h index c61b49b57a01e..fd1676d8bc626 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -526,6 +526,7 @@ ThreadFactory& threadFactoryForTest(); } // namespace Thread namespace Api { +ApiPtr createApiForTest(); ApiPtr createApiForTest(Stats::Store& stat_store); ApiPtr createApiForTest(Stats::Store& stat_store, Event::TimeSystem& time_system); } // namespace Api From 5eb7327fda5f6b835b8ee61298f08aa4fcb24fad Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Sun, 10 Feb 2019 14:46:01 -0500 Subject: [PATCH 2/3] Add a way to create test APIs without specifying stats_store, and use this in places where the test doesn't do anything else with stats. Signed-off-by: Joshua Marantz --- test/common/event/dispatcher_impl_test.cc | 4 ++-- test/common/filesystem/watcher_impl_test.cc | 4 +--- test/common/http/codec_client_test.cc | 3 +-- test/common/upstream/cluster_manager_impl_test.cc | 3 +-- test/config_test/config_test.cc | 3 +-- .../extensions/resource_monitors/fixed_heap/config_test.cc | 5 +---- .../resource_monitors/injected_resource/config_test.cc | 4 +--- .../injected_resource/injected_resource_monitor_test.cc | 3 +-- test/fuzz/main.cc | 7 ++----- test/server/config_validation/async_client_test.cc | 3 +-- test/server/configuration_impl_test.cc | 3 +-- test/server/worker_impl_test.cc | 3 +-- test/test_common/utility.cc | 3 ++- 13 files changed, 16 insertions(+), 32 deletions(-) diff --git a/test/common/event/dispatcher_impl_test.cc b/test/common/event/dispatcher_impl_test.cc index 619176c5ba9bc..a53b858852569 100644 --- a/test/common/event/dispatcher_impl_test.cc +++ b/test/common/event/dispatcher_impl_test.cc @@ -61,8 +61,8 @@ TEST(DeferredDeleteTest, DeferredDelete) { class DispatcherImplTest : public TestBase { protected: DispatcherImplTest() - : api_(Api::createApiForTest()), - dispatcher_(std::make_unique(*api_)), work_finished_(false) { + : api_(Api::createApiForTest()), dispatcher_(std::make_unique(*api_)), + work_finished_(false) { dispatcher_thread_ = api_->threadFactory().createThread([this]() { // Must create a keepalive timer to keep the dispatcher from exiting. std::chrono::milliseconds time_interval(500); diff --git a/test/common/filesystem/watcher_impl_test.cc b/test/common/filesystem/watcher_impl_test.cc index da6b5fd80cddf..69024e526bf93 100644 --- a/test/common/filesystem/watcher_impl_test.cc +++ b/test/common/filesystem/watcher_impl_test.cc @@ -4,7 +4,6 @@ #include "common/common/assert.h" #include "common/event/dispatcher_impl.h" #include "common/filesystem/watcher_impl.h" -#include "common/stats/isolated_store_impl.h" #include "test/test_common/environment.h" #include "test/test_common/test_base.h" @@ -17,9 +16,8 @@ namespace Filesystem { class WatcherImplTest : public TestBase { protected: - WatcherImplTest() : api_(Api::createApiForTest(stats_store_)), dispatcher_(*api_) {} + WatcherImplTest() : api_(Api::createApiForTest()), dispatcher_(*api_) {} - Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; Event::DispatcherImpl dispatcher_; }; diff --git a/test/common/http/codec_client_test.cc b/test/common/http/codec_client_test.cc index 6e38d86bea355..d41dc83068087 100644 --- a/test/common/http/codec_client_test.cc +++ b/test/common/http/codec_client_test.cc @@ -259,7 +259,7 @@ TEST_F(CodecClientTest, WatermarkPassthrough) { // Test the codec getting input from a real TCP connection. class CodecNetworkTest : public TestBaseWithParam { public: - CodecNetworkTest() : api_(Api::createApiForTest(stats_store_)) { + CodecNetworkTest() : api_(Api::createApiForTest()) { dispatcher_ = std::make_unique(*api_); upstream_listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false); Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( @@ -326,7 +326,6 @@ class CodecNetworkTest : public TestBaseWithParam { } protected: - Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; Event::DispatcherPtr dispatcher_; Network::ListenerPtr upstream_listener_; diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index 08f9ecb4936f3..61c3af936c096 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -165,7 +165,7 @@ envoy::config::bootstrap::v2::Bootstrap parseBootstrapFromV2Yaml(const std::stri class ClusterManagerImplTest : public TestBase { public: - ClusterManagerImplTest() : api_(Api::createApiForTest(stats_store_)) {} + ClusterManagerImplTest() : api_(Api::createApiForTest()) {} void create(const envoy::config::bootstrap::v2::Bootstrap& bootstrap) { cluster_manager_ = std::make_unique( @@ -239,7 +239,6 @@ class ClusterManagerImplTest : public TestBase { return metadata; } - Stats::IsolatedStoreImpl stats_store_; Event::SimulatedTimeSystem time_system_; Api::ApiPtr api_; NiceMock factory_; diff --git a/test/config_test/config_test.cc b/test/config_test/config_test.cc index 4b5543909738d..db9aca85ca90c 100644 --- a/test/config_test/config_test.cc +++ b/test/config_test/config_test.cc @@ -43,8 +43,7 @@ OptionsImpl asConfigYaml(const OptionsImpl& src, Api::Api& api) { class ConfigTest { public: - ConfigTest(const OptionsImpl& options) - : api_(Api::createApiForTest()), options_(options) { + ConfigTest(const OptionsImpl& options) : api_(Api::createApiForTest()), options_(options) { ON_CALL(server_, options()).WillByDefault(ReturnRef(options_)); ON_CALL(server_, random()).WillByDefault(ReturnRef(random_)); ON_CALL(server_, sslContextManager()).WillByDefault(ReturnRef(ssl_context_manager_)); diff --git a/test/extensions/resource_monitors/fixed_heap/config_test.cc b/test/extensions/resource_monitors/fixed_heap/config_test.cc index edc9e54d44f68..1eeede7baccd1 100644 --- a/test/extensions/resource_monitors/fixed_heap/config_test.cc +++ b/test/extensions/resource_monitors/fixed_heap/config_test.cc @@ -1,8 +1,6 @@ #include "envoy/config/resource_monitor/fixed_heap/v2alpha/fixed_heap.pb.validate.h" #include "envoy/registry/registry.h" -#include "common/stats/isolated_store_impl.h" - #include "server/resource_monitor_config_impl.h" #include "extensions/resource_monitors/fixed_heap/config.h" @@ -24,8 +22,7 @@ TEST(FixedHeapMonitorFactoryTest, CreateMonitor) { envoy::config::resource_monitor::fixed_heap::v2alpha::FixedHeapConfig config; config.set_max_heap_size_bytes(std::numeric_limits::max()); Event::MockDispatcher dispatcher; - Stats::IsolatedStoreImpl stats_store; - Api::ApiPtr api = Api::createApiForTest(stats_store); + Api::ApiPtr api = Api::createApiForTest(); Server::Configuration::ResourceMonitorFactoryContextImpl context(dispatcher, *api); auto monitor = factory->createResourceMonitor(config, context); EXPECT_NE(monitor, nullptr); diff --git a/test/extensions/resource_monitors/injected_resource/config_test.cc b/test/extensions/resource_monitors/injected_resource/config_test.cc index c595ee80b80c3..3e80cf9b9c91c 100644 --- a/test/extensions/resource_monitors/injected_resource/config_test.cc +++ b/test/extensions/resource_monitors/injected_resource/config_test.cc @@ -2,7 +2,6 @@ #include "envoy/registry/registry.h" #include "common/event/dispatcher_impl.h" -#include "common/stats/isolated_store_impl.h" #include "server/resource_monitor_config_impl.h" @@ -25,8 +24,7 @@ TEST(InjectedResourceMonitorFactoryTest, CreateMonitor) { envoy::config::resource_monitor::injected_resource::v2alpha::InjectedResourceConfig config; config.set_filename(TestEnvironment::temporaryPath("injected_resource")); - Stats::IsolatedStoreImpl stats_store; - Api::ApiPtr api = Api::createApiForTest(stats_store); + Api::ApiPtr api = Api::createApiForTest(); Event::DispatcherImpl dispatcher(*api); Server::Configuration::ResourceMonitorFactoryContextImpl context(dispatcher, *api); Server::ResourceMonitorPtr monitor = factory->createResourceMonitor(config, context); diff --git a/test/extensions/resource_monitors/injected_resource/injected_resource_monitor_test.cc b/test/extensions/resource_monitors/injected_resource/injected_resource_monitor_test.cc index a173600bfff2d..fa68bca92efc5 100644 --- a/test/extensions/resource_monitors/injected_resource/injected_resource_monitor_test.cc +++ b/test/extensions/resource_monitors/injected_resource/injected_resource_monitor_test.cc @@ -45,7 +45,7 @@ class MockedCallbacks : public Server::ResourceMonitor::Callbacks { class InjectedResourceMonitorTest : public TestBase { protected: InjectedResourceMonitorTest() - : api_(Api::createApiForTest(stats_store_)), dispatcher_(*api_), + : api_(Api::createApiForTest()), dispatcher_(*api_), resource_filename_(TestEnvironment::temporaryPath("injected_resource")), file_updater_(resource_filename_), monitor_(createMonitor()) {} @@ -64,7 +64,6 @@ class InjectedResourceMonitorTest : public TestBase { return std::make_unique(config, context); } - Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; Event::DispatcherImpl dispatcher_; const std::string resource_filename_; diff --git a/test/fuzz/main.cc b/test/fuzz/main.cc index 6d54b325850a4..7883fcce7aded 100644 --- a/test/fuzz/main.cc +++ b/test/fuzz/main.cc @@ -14,7 +14,6 @@ #include "common/common/assert.h" #include "common/common/logger.h" -#include "common/stats/isolated_store_impl.h" #include "test/fuzz/fuzz_runner.h" #include "test/test_common/environment.h" @@ -29,9 +28,8 @@ std::vector test_corpus_; class FuzzerCorpusTest : public TestBaseWithParam { protected: - FuzzerCorpusTest() : api_(Api::createApiForTest(stats_store_)) {} + FuzzerCorpusTest() : api_(Api::createApiForTest()) {} - Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; }; @@ -55,8 +53,7 @@ int main(int argc, char** argv) { // Ensure we cleanup API resources before we jump into the tests, the test API creates a singleton // time system that we don't want to leak into gtest. { - Envoy::Stats::IsolatedStoreImpl stats_store; - Envoy::Api::ApiPtr api = Envoy::Api::createApiForTest(stats_store); + Envoy::Api::ApiPtr api = Envoy::Api::createApiForTest(); for (int i = 1; i < argc; ++i) { const std::string arg{argv[i]}; if (arg.empty() || arg[0] == '-') { diff --git a/test/server/config_validation/async_client_test.cc b/test/server/config_validation/async_client_test.cc index dceb4dafafb3e..21ef295efacc2 100644 --- a/test/server/config_validation/async_client_test.cc +++ b/test/server/config_validation/async_client_test.cc @@ -15,8 +15,7 @@ TEST(ValidationAsyncClientTest, MockedMethods) { MockAsyncClientCallbacks callbacks; MockAsyncClientStreamCallbacks stream_callbacks; - Stats::IsolatedStoreImpl stats_store; - Api::ApiPtr api = Api::createApiForTest(stats_store); + Api::ApiPtr api = Api::createApiForTest(); ValidationAsyncClient client(*api); EXPECT_EQ(nullptr, client.send(std::move(message), callbacks, AsyncClient::RequestOptions())); EXPECT_EQ(nullptr, client.start(stream_callbacks, AsyncClient::StreamOptions())); diff --git a/test/server/configuration_impl_test.cc b/test/server/configuration_impl_test.cc index 2b6b6217bee9b..5a566b52271f7 100644 --- a/test/server/configuration_impl_test.cc +++ b/test/server/configuration_impl_test.cc @@ -53,14 +53,13 @@ TEST(FilterChainUtility, buildFilterChainFailWithBadFilters) { class ConfigurationImplTest : public TestBase { protected: ConfigurationImplTest() - : api_(Api::createApiForTest(stats_store_)), + : api_(Api::createApiForTest()), cluster_manager_factory_( server_.admin(), server_.runtime(), server_.stats(), server_.threadLocal(), server_.random(), server_.dnsResolver(), server_.sslContextManager(), server_.dispatcher(), server_.localInfo(), server_.secretManager(), *api_, server_.httpContext(), server_.accessLogManager(), server_.singletonManager()) {} - Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; NiceMock server_; Upstream::ProdClusterManagerFactory cluster_manager_factory_; diff --git a/test/server/worker_impl_test.cc b/test/server/worker_impl_test.cc index 06775542ab02d..dd56ad2317a82 100644 --- a/test/server/worker_impl_test.cc +++ b/test/server/worker_impl_test.cc @@ -22,13 +22,12 @@ namespace Server { class WorkerImplTest : public TestBase { public: - WorkerImplTest() : api_(Api::createApiForTest(stats_store_)) { + WorkerImplTest() : api_(Api::createApiForTest()) { // In the real worker the watchdog has timers that prevent exit. Here we need to prevent event // loop exit since we use mock timers. no_exit_timer_->enableTimer(std::chrono::hours(1)); } - Stats::IsolatedStoreImpl stats_store_; NiceMock tls_; Network::MockConnectionHandler* handler_ = new Network::MockConnectionHandler(); NiceMock guard_dog_; diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc index c411edd4a7f2b..f72d2ef677949 100644 --- a/test/test_common/utility.cc +++ b/test/test_common/utility.cc @@ -419,7 +419,8 @@ class TestImpl : public TestImplProvider, public Impl { }; ApiPtr createApiForTest() { - return std::make_unique(std::chrono::milliseconds(1000), Thread::threadFactoryForTest()); + return std::make_unique(std::chrono::milliseconds(1000), + Thread::threadFactoryForTest()); } ApiPtr createApiForTest(Stats::Store& stat_store) { From f69e9620431d4eafae4f1696af8f4ca0507bea0d Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Sun, 10 Feb 2019 15:05:26 -0500 Subject: [PATCH 3/3] Remove a few more dummy stats objects that are not needed. Signed-off-by: Joshua Marantz --- test/common/http/http1/conn_pool_test.cc | 3 +-- .../http/grpc_json_transcoder/json_transcoder_filter_test.cc | 3 +-- test/extensions/filters/http/jwt_authn/jwks_cache_test.cc | 3 +-- test/server/guarddog_impl_test.cc | 1 - test/server/listener_manager_impl_test.cc | 3 +-- 5 files changed, 4 insertions(+), 9 deletions(-) diff --git a/test/common/http/http1/conn_pool_test.cc b/test/common/http/http1/conn_pool_test.cc index ae6152f37eb7b..853b488499b84 100644 --- a/test/common/http/http1/conn_pool_test.cc +++ b/test/common/http/http1/conn_pool_test.cc @@ -46,7 +46,7 @@ class ConnPoolImplForTest : public ConnPoolImpl { NiceMock* upstream_ready_timer) : ConnPoolImpl(dispatcher, Upstream::makeTestHost(cluster, "tcp://127.0.0.1:9000"), Upstream::ResourcePriority::Default, nullptr), - api_(Api::createApiForTest(stats_store_)), mock_dispatcher_(dispatcher), + api_(Api::createApiForTest()), mock_dispatcher_(dispatcher), mock_upstream_ready_timer_(upstream_ready_timer) {} ~ConnPoolImplForTest() { @@ -111,7 +111,6 @@ class ConnPoolImplForTest : public ConnPoolImpl { EXPECT_FALSE(upstream_ready_enabled_); } - Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; Event::MockDispatcher& mock_dispatcher_; NiceMock* mock_upstream_ready_timer_; diff --git a/test/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter_test.cc b/test/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter_test.cc index f5ad48329deab..7ccf851a798a0 100644 --- a/test/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter_test.cc +++ b/test/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter_test.cc @@ -44,9 +44,8 @@ namespace GrpcJsonTranscoder { class GrpcJsonTranscoderFilterTestBase { protected: - GrpcJsonTranscoderFilterTestBase() : api_(Api::createApiForTest(stats_)) {} + GrpcJsonTranscoderFilterTestBase() : api_(Api::createApiForTest()) {} - Stats::IsolatedStoreImpl stats_; Api::ApiPtr api_; }; diff --git a/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc b/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc index 29ce11ed0cd6e..2c27434572c75 100644 --- a/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc +++ b/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc @@ -21,7 +21,7 @@ namespace { class JwksCacheTest : public TestBase { protected: - JwksCacheTest() : api_(Api::createApiForTest(stats_)) {} + JwksCacheTest() : api_(Api::createApiForTest()) {} void SetUp() override { MessageUtil::loadFromYaml(ExampleConfig, config_); cache_ = JwksCache::create(config_, time_system_, *api_); @@ -32,7 +32,6 @@ class JwksCacheTest : public TestBase { JwtAuthentication config_; JwksCachePtr cache_; google::jwt_verify::JwksPtr jwks_; - Stats::IsolatedStoreImpl stats_; Api::ApiPtr api_; }; diff --git a/test/server/guarddog_impl_test.cc b/test/server/guarddog_impl_test.cc index c61ac7946f347..170e034dcad3e 100644 --- a/test/server/guarddog_impl_test.cc +++ b/test/server/guarddog_impl_test.cc @@ -140,7 +140,6 @@ class GuardDogMissTest : public GuardDogTestBase { NiceMock config_miss_; NiceMock config_mega_; - Stats::IsolatedStoreImpl stats_store_; }; TEST_F(GuardDogMissTest, MissTest) { diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index 616b29e36a53c..e2006ec2b127a 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -57,7 +57,7 @@ class ListenerHandle { class ListenerManagerImplTest : public TestBase { protected: - ListenerManagerImplTest() : api_(Api::createApiForTest(stats_)) { + ListenerManagerImplTest() : api_(Api::createApiForTest()) { ON_CALL(server_, api()).WillByDefault(ReturnRef(*api_)); EXPECT_CALL(worker_factory_, createWorker_()).WillOnce(Return(worker_)); manager_ = std::make_unique(server_, listener_factory_, worker_factory_); @@ -122,7 +122,6 @@ class ListenerManagerImplTest : public TestBase { std::unique_ptr manager_; NiceMock guard_dog_; Event::SimulatedTimeSystem time_system_; - Stats::IsolatedStoreImpl stats_; Api::ApiPtr api_; };