diff --git a/include/envoy/common/time.h b/include/envoy/common/time.h index 6e9bd6c825824..f48ca72f79816 100644 --- a/include/envoy/common/time.h +++ b/include/envoy/common/time.h @@ -21,7 +21,7 @@ typedef std::chrono::time_point MonotonicTime; */ class TimeSource { public: - virtual ~TimeSource() {} + virtual ~TimeSource() = default; /** * @return the current system time; not guaranteed to be monotonically increasing. diff --git a/include/envoy/event/timer.h b/include/envoy/event/timer.h index 7ebc8bb14787f..d6d90d1da39d1 100644 --- a/include/envoy/event/timer.h +++ b/include/envoy/event/timer.h @@ -55,7 +55,7 @@ typedef std::unique_ptr SchedulerPtr; */ class TimeSystem : public TimeSource { public: - virtual ~TimeSystem() {} + virtual ~TimeSystem() = default; using Duration = MonotonicTime::duration; diff --git a/test/README.md b/test/README.md index e83a72fc11cdd..76f4c866f4f7c 100644 --- a/test/README.md +++ b/test/README.md @@ -69,3 +69,33 @@ Examples: EXPECT_THAT(response->headers(), IsSubsetOfHeaders(allowed_headers)); EXPECT_THAT(response->headers(), IsSupersetOfHeaders(required_headers)); ``` + +## Controlling time in tests + +In Envoy production code, time and timers are managed via +[`Event::TimeSystem`](https://github.com/envoyproxy/envoy/blob/master/include/envoy/event/timer.h), +which provides a mechanism for querying the time and setting up time-based +callbacks. Bypassing this abstraction in Envoy code is flagged as a format +violation in CI. + +In tests we use a derivation +[`Event::TestTimeSystem`](test_common/test_time_system.h) which adds the ability +to sleep or do a blocking timed wait on a condition variable. There are two +implementations of the `Event::TestTimeSystem` interface: +`Event::TestRealTimeSystem`, and `Event::SimulatedTimeSystem`. The latter is +recommended for all new tests, as it helps avoid flaky tests on slow machines, +and makes tests run faster. + +Typically we do not want to have both real-time and simulated-time in the same +test; that could lead to hard-to-reproduce results. Thus both implementations +have a mechanism to enforce that only one of them can be instantiated at once. +A runtime assertion occurs if an `Event::TestRealTimeSystem` and +`Event::SimulatedTimeSystem` are instantiated at the same time. Once the +time-system goes out of scope, usually at the end of a test method, the slate +is clean and a new test-method can use a different time system. + +There is also `Event::GlobalTimeSystem`, which can be instantiated in shared +test infrastructure that wants to be agnostic to which `TimeSystem` is used in a +test. When no `TimeSystem` is instantiated in a test, the `Event::GlobalTimeSystem` +will lazy-initialize itself into a concrete `TimeSystem`. Currently this is +`TestRealTimeSystem` but will be changed in the future to `SimulatedTimeSystem`. diff --git a/test/common/common/mutex_tracer_test.cc b/test/common/common/mutex_tracer_test.cc index 386dcf6a399ac..042b0469f1994 100644 --- a/test/common/common/mutex_tracer_test.cc +++ b/test/common/common/mutex_tracer_test.cc @@ -75,8 +75,9 @@ TEST_F(MutexTracerTest, TwoThreadsWithContention) { for (int i = 1; i <= 10; ++i) { int64_t curr_num_lifetime_wait_cycles = tracer_.lifetimeWaitCycles(); - Thread::TestUtil::ContentionGenerator::generateContention(tracer_); + Thread::TestUtil::ContentionGenerator contention_generator; + contention_generator.generateContention(tracer_); EXPECT_EQ(tracer_.numContentions(), i); EXPECT_GT(tracer_.currentWaitCycles(), 0); // This shouldn't be hardcoded. EXPECT_GT(tracer_.lifetimeWaitCycles(), 0); diff --git a/test/common/config/BUILD b/test/common/config/BUILD index d80c5b07dd781..f6b41dac69e14 100644 --- a/test/common/config/BUILD +++ b/test/common/config/BUILD @@ -223,5 +223,6 @@ envoy_cc_test( "//source/common/config:config_provider_lib", "//source/common/protobuf:utility_lib", "//test/mocks/server:server_mocks", + "//test/test_common:simulated_time_system_lib", ], ) diff --git a/test/common/config/config_provider_impl_test.cc b/test/common/config/config_provider_impl_test.cc index 7e928963aed9f..4704f1aa82b06 100644 --- a/test/common/config/config_provider_impl_test.cc +++ b/test/common/config/config_provider_impl_test.cc @@ -5,6 +5,7 @@ #include "test/common/config/dummy_config.pb.h" #include "test/mocks/server/mocks.h" +#include "test/test_common/simulated_time_system.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -211,9 +212,10 @@ class ConfigProviderImplTest : public testing::Test { provider_manager_ = std::make_unique(factory_context_.admin_); } - Event::SimulatedTimeSystem& timeSystem() { return factory_context_.timeSystem(); } + Event::SimulatedTimeSystem& timeSystem() { return time_system_; } protected: + Event::SimulatedTimeSystem time_system_; NiceMock factory_context_; std::unique_ptr provider_manager_; }; diff --git a/test/common/config/grpc_mux_impl_test.cc b/test/common/config/grpc_mux_impl_test.cc index 953eca615477d..d20d699fbdec6 100644 --- a/test/common/config/grpc_mux_impl_test.cc +++ b/test/common/config/grpc_mux_impl_test.cc @@ -18,6 +18,7 @@ #include "test/mocks/runtime/mocks.h" #include "test/test_common/logging.h" #include "test/test_common/simulated_time_system.h" +#include "test/test_common/test_time.h" #include "test/test_common/utility.h" #include "gmock/gmock.h" @@ -37,11 +38,9 @@ namespace { // We test some mux specific stuff below, other unit test coverage for singleton use of GrpcMuxImpl // is provided in [grpc_]subscription_impl_test.cc. -class GrpcMuxImplTest : public testing::Test { +class GrpcMuxImplTestBase : public testing::Test { public: - GrpcMuxImplTest() : async_client_(new Grpc::MockAsyncClient()) { - dispatcher_.setTimeSystem(time_system_); - } + GrpcMuxImplTestBase() : async_client_(new Grpc::MockAsyncClient()) {} void setup() { grpc_mux_ = std::make_unique( @@ -88,12 +87,16 @@ class GrpcMuxImplTest : public testing::Test { Grpc::MockAsyncStream async_stream_; std::unique_ptr grpc_mux_; NiceMock callbacks_; - Event::SimulatedTimeSystem time_system_; NiceMock local_info_; Stats::IsolatedStoreImpl stats_; Envoy::Config::RateLimitSettings rate_limit_settings_; }; +class GrpcMuxImplTest : public GrpcMuxImplTestBase { +public: + Event::SimulatedTimeSystem time_system_; +}; + // Validate behavior when multiple type URL watches are maintained, watches are created/destroyed // (via RAII). TEST_F(GrpcMuxImplTest, MultipleTypeUrlStreams) { @@ -328,11 +331,9 @@ TEST_F(GrpcMuxImplTest, WatchDemux) { // Exactly one test requires a mock time system to provoke behavior that cannot // easily be achieved with a SimulatedTimeSystem. -class GrpcMuxImplTestWithMockTimeSystem : public GrpcMuxImplTest { -protected: - GrpcMuxImplTestWithMockTimeSystem() { dispatcher_.setTimeSystem(mock_time_system_); } - - MockTimeSystem mock_time_system_; +class GrpcMuxImplTestWithMockTimeSystem : public GrpcMuxImplTestBase { +public: + Event::DelegatingTestTimeSystem mock_time_system_; }; // Verifies that rate limiting is not enforced with defaults. @@ -348,7 +349,7 @@ TEST_F(GrpcMuxImplTestWithMockTimeSystem, TooManyRequestsWithDefaultSettings) { })); // Validate that rate limiter is not created. - EXPECT_CALL(mock_time_system_, monotonicTime()).Times(0); + EXPECT_CALL(*mock_time_system_, monotonicTime()).Times(0); setup(); @@ -398,7 +399,7 @@ TEST_F(GrpcMuxImplTestWithMockTimeSystem, TooManyRequestsWithEmptyRateLimitSetti drain_request_timer = new Event::MockTimer(); return drain_request_timer; })); - EXPECT_CALL(mock_time_system_, monotonicTime()) + EXPECT_CALL(*mock_time_system_, monotonicTime()) .WillRepeatedly(Return(std::chrono::steady_clock::time_point{})); RateLimitSettings custom_rate_limit_settings; diff --git a/test/common/router/BUILD b/test/common/router/BUILD index 3394cbd55fbd7..1c303657c3d05 100644 --- a/test/common/router/BUILD +++ b/test/common/router/BUILD @@ -67,6 +67,7 @@ envoy_cc_test( "//test/mocks/server:server_mocks", "//test/mocks/thread_local:thread_local_mocks", "//test/mocks/upstream:upstream_mocks", + "//test/test_common:simulated_time_system_lib", "//test/test_common:utility_lib", ], ) diff --git a/test/common/router/rds_impl_test.cc b/test/common/router/rds_impl_test.cc index 81e52bba4b995..7fa8cac0dc856 100644 --- a/test/common/router/rds_impl_test.cc +++ b/test/common/router/rds_impl_test.cc @@ -20,6 +20,7 @@ #include "test/mocks/thread_local/mocks.h" #include "test/mocks/upstream/mocks.h" #include "test/test_common/printers.h" +#include "test/test_common/simulated_time_system.h" #include "test/test_common/utility.h" #include "gmock/gmock.h" @@ -67,8 +68,9 @@ class RdsTestBase : public testing::Test { })); } - Event::SimulatedTimeSystem& timeSystem() { return factory_context_.timeSystem(); } + Event::SimulatedTimeSystem& timeSystem() { return time_system_; } + Event::SimulatedTimeSystem time_system_; NiceMock factory_context_; Http::MockAsyncClientRequest request_; Http::AsyncClient::Callbacks* callbacks_{}; diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index 94527e33aaa42..fb8c6d9f2c0d7 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -164,9 +164,7 @@ envoy::config::bootstrap::v2::Bootstrap parseBootstrapFromV2Yaml(const std::stri class ClusterManagerImplTest : public testing::Test { public: - ClusterManagerImplTest() : api_(Api::createApiForTest(stats_store_)) { - factory_.dispatcher_.setTimeSystem(time_system_); - } + ClusterManagerImplTest() : api_(Api::createApiForTest(stats_store_)) {} void create(const envoy::config::bootstrap::v2::Bootstrap& bootstrap) { cluster_manager_ = std::make_unique( @@ -241,12 +239,12 @@ class ClusterManagerImplTest : public testing::Test { } Stats::IsolatedStoreImpl stats_store_; + Event::SimulatedTimeSystem time_system_; Api::ApiPtr api_; NiceMock factory_; std::unique_ptr cluster_manager_; AccessLog::MockAccessLogManager log_manager_; NiceMock admin_; - Event::SimulatedTimeSystem time_system_; MockLocalClusterUpdate local_cluster_update_; Http::ContextImpl http_context_; }; diff --git a/test/common/upstream/load_stats_reporter_test.cc b/test/common/upstream/load_stats_reporter_test.cc index 5ac86d9a80c6e..beed927a71704 100644 --- a/test/common/upstream/load_stats_reporter_test.cc +++ b/test/common/upstream/load_stats_reporter_test.cc @@ -30,9 +30,7 @@ class LoadStatsReporterTest : public testing::Test { public: LoadStatsReporterTest() : retry_timer_(new Event::MockTimer()), response_timer_(new Event::MockTimer()), - async_client_(new Grpc::MockAsyncClient()) { - dispatcher_.setTimeSystem(time_system_); - } + async_client_(new Grpc::MockAsyncClient()) {} void createLoadStatsReporter() { InSequence s; @@ -68,6 +66,7 @@ class LoadStatsReporterTest : public testing::Test { load_stats_reporter_->onReceiveMessage(std::move(response)); } + Event::SimulatedTimeSystem time_system_; NiceMock cm_; Event::MockDispatcher dispatcher_; Stats::IsolatedStoreImpl stats_store_; @@ -78,7 +77,6 @@ class LoadStatsReporterTest : public testing::Test { Event::TimerCb response_timer_cb_; Grpc::MockAsyncStream async_stream_; Grpc::MockAsyncClient* async_client_; - Event::SimulatedTimeSystem time_system_; NiceMock local_info_; }; diff --git a/test/exe/main_common_test.cc b/test/exe/main_common_test.cc index ac784d8a4e60d..54f8ef9d10e25 100644 --- a/test/exe/main_common_test.cc +++ b/test/exe/main_common_test.cc @@ -274,7 +274,8 @@ TEST_P(AdminRequestTest, AdminRequestContentionEnabled) { started_.WaitForNotification(); // Induce contention to guarantee a non-zero num_contentions count. - Thread::TestUtil::ContentionGenerator::generateContention(MutexTracerImpl::getOrCreateTracer()); + Thread::TestUtil::ContentionGenerator contention_generator; + contention_generator.generateContention(MutexTracerImpl::getOrCreateTracer()); std::string response = adminRequest("/contention", "GET"); EXPECT_THAT(response, Not(HasSubstr("not enabled"))); diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index 646d2e77fb16f..832b5df041b4b 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -183,9 +183,8 @@ void testUtil(const TestUtilOptions& options) { ServerSslSocketFactory server_ssl_socket_factory(std::move(server_cfg), manager, server_stats_store, std::vector{}); - DangerousDeprecatedTestTime test_time; Api::ApiPtr api = Api::createApiForTest(server_stats_store); - Event::DispatcherImpl dispatcher(test_time.timeSystem(), *api); + Event::DispatcherImpl dispatcher(time_system, *api); Network::TcpListenSocket socket(Network::Test::getCanonicalLoopbackAddress(options.version()), nullptr, true); Network::MockListenerCallbacks callbacks; @@ -400,9 +399,8 @@ const std::string testUtilV2(const TestUtilOptionsV2& options) { ServerSslSocketFactory server_ssl_socket_factory(std::move(server_cfg), manager, server_stats_store, server_names); - DangerousDeprecatedTestTime test_time; Api::ApiPtr api = Api::createApiForTest(server_stats_store); - Event::DispatcherImpl dispatcher(test_time.timeSystem(), *api); + Event::DispatcherImpl dispatcher(time_system, *api); Network::TcpListenSocket socket(Network::Test::getCanonicalLoopbackAddress(options.version()), nullptr, true); NiceMock callbacks; @@ -574,11 +572,15 @@ class SslSocketTest : public SslCertsTest, protected: SslSocketTest() : api_(Api::createApiForTest(stats_store_)), - dispatcher_(std::make_unique(test_time_.timeSystem(), *api_)) {} + dispatcher_(std::make_unique(time_system_, *api_)) {} + + void testClientSessionResumption(const std::string& server_ctx_yaml, + const std::string& client_ctx_yaml, bool expect_reuse, + const Network::Address::IpVersion version); + Event::SimulatedTimeSystem time_system_; Stats::IsolatedStoreImpl stats_store_; Api::ApiPtr api_; - DangerousDeprecatedTestTime test_time_; std::unique_ptr dispatcher_; }; @@ -1861,8 +1863,7 @@ TEST_P(SslSocketTest, FlushCloseDuringHandshake) { envoy::api::v2::auth::DownstreamTlsContext tls_context; MessageUtil::loadFromYaml(TestEnvironment::substitute(server_ctx_yaml), tls_context); auto server_cfg = std::make_unique(tls_context, factory_context_); - Event::SimulatedTimeSystem time_system; - ContextManagerImpl manager(time_system); + ContextManagerImpl manager(time_system_); Stats::IsolatedStoreImpl server_stats_store; ServerSslSocketFactory server_ssl_socket_factory(std::move(server_cfg), manager, server_stats_store, std::vector{}); @@ -1922,8 +1923,7 @@ TEST_P(SslSocketTest, HalfClose) { envoy::api::v2::auth::DownstreamTlsContext server_tls_context; MessageUtil::loadFromYaml(TestEnvironment::substitute(server_ctx_yaml), server_tls_context); auto server_cfg = std::make_unique(server_tls_context, factory_context_); - Event::SimulatedTimeSystem time_system; - ContextManagerImpl manager(time_system); + ContextManagerImpl manager(time_system_); Stats::IsolatedStoreImpl server_stats_store; ServerSslSocketFactory server_ssl_socket_factory(std::move(server_cfg), manager, server_stats_store, std::vector{}); @@ -2012,8 +2012,7 @@ TEST_P(SslSocketTest, ClientAuthMultipleCAs) { envoy::api::v2::auth::DownstreamTlsContext server_tls_context; MessageUtil::loadFromYaml(TestEnvironment::substitute(server_ctx_yaml), server_tls_context); auto server_cfg = std::make_unique(server_tls_context, factory_context); - Event::SimulatedTimeSystem time_system; - ContextManagerImpl manager(time_system); + ContextManagerImpl manager(time_system_); Stats::IsolatedStoreImpl server_stats_store; ServerSslSocketFactory server_ssl_socket_factory(std::move(server_cfg), manager, server_stats_store, std::vector{}); @@ -2116,9 +2115,8 @@ void testTicketSessionResumption(const std::string& server_ctx_yaml1, true); NiceMock callbacks; Network::MockConnectionHandler connection_handler; - DangerousDeprecatedTestTime test_time; Api::ApiPtr api = Api::createApiForTest(server_stats_store); - Event::DispatcherImpl dispatcher(test_time.timeSystem(), *api); + Event::DispatcherImpl dispatcher(time_system, *api); Network::ListenerPtr listener1 = dispatcher.createListener(socket1, callbacks, true, false); Network::ListenerPtr listener2 = dispatcher.createListener(socket2, callbacks, true, false); @@ -2507,8 +2505,7 @@ TEST_P(SslSocketTest, ClientAuthCrossListenerSessionResumption) { envoy::api::v2::auth::DownstreamTlsContext tls_context2; MessageUtil::loadFromYaml(TestEnvironment::substitute(server2_ctx_yaml), tls_context2); auto server2_cfg = std::make_unique(tls_context2, factory_context_); - Event::SimulatedTimeSystem time_system; - ContextManagerImpl manager(time_system); + ContextManagerImpl manager(time_system_); Stats::IsolatedStoreImpl server_stats_store; ServerSslSocketFactory server_ssl_socket_factory(std::move(server_cfg), manager, server_stats_store, std::vector{}); @@ -2609,14 +2606,14 @@ TEST_P(SslSocketTest, ClientAuthCrossListenerSessionResumption) { EXPECT_EQ(0UL, client_stats_store.counter("ssl.session_reused").value()); } -void testClientSessionResumption(const std::string& server_ctx_yaml, - const std::string& client_ctx_yaml, bool expect_reuse, - const Network::Address::IpVersion version) { +void SslSocketTest::testClientSessionResumption(const std::string& server_ctx_yaml, + const std::string& client_ctx_yaml, + bool expect_reuse, + const Network::Address::IpVersion version) { InSequence s; testing::NiceMock factory_context; - Event::SimulatedTimeSystem time_system; - ContextManagerImpl manager(time_system); + ContextManagerImpl manager(time_system_); envoy::api::v2::auth::DownstreamTlsContext server_ctx_proto; MessageUtil::loadFromYaml(TestEnvironment::substitute(server_ctx_yaml), server_ctx_proto); @@ -2630,7 +2627,7 @@ void testClientSessionResumption(const std::string& server_ctx_yaml, NiceMock callbacks; Network::MockConnectionHandler connection_handler; Api::ApiPtr api = Api::createApiForTest(server_stats_store); - Event::DispatcherImpl dispatcher(time_system, *api); + Event::DispatcherImpl dispatcher(time_system_, *api); Network::ListenerPtr listener = dispatcher.createListener(socket, callbacks, true, false); Network::ConnectionPtr server_connection; @@ -2883,8 +2880,7 @@ TEST_P(SslSocketTest, SslError) { envoy::api::v2::auth::DownstreamTlsContext tls_context; MessageUtil::loadFromYaml(TestEnvironment::substitute(server_ctx_yaml), tls_context); auto server_cfg = std::make_unique(tls_context, factory_context_); - Event::SimulatedTimeSystem time_system; - ContextManagerImpl manager(time_system); + ContextManagerImpl manager(time_system_); Stats::IsolatedStoreImpl server_stats_store; ServerSslSocketFactory server_ssl_socket_factory(std::move(server_cfg), manager, server_stats_store, std::vector{}); @@ -3486,8 +3482,7 @@ TEST_P(SslSocketTest, DownstreamNotReadySslSocket) { EXPECT_TRUE(server_cfg->tlsCertificates().empty()); EXPECT_FALSE(server_cfg->isReady()); - Event::SimulatedTimeSystem time_system; - ContextManagerImpl manager(time_system); + ContextManagerImpl manager(time_system_); ServerSslSocketFactory server_ssl_socket_factory(std::move(server_cfg), manager, stats_store, std::vector{}); auto transport_socket = server_ssl_socket_factory.createTransportSocket(nullptr); @@ -3510,7 +3505,6 @@ TEST_P(SslSocketTest, UpstreamNotReadySslSocket) { NiceMock random; NiceMock cluster_manager; NiceMock init_manager; - Event::SimulatedTimeSystem time_system; EXPECT_CALL(factory_context, localInfo()).WillOnce(ReturnRef(local_info)); EXPECT_CALL(factory_context, dispatcher()).WillOnce(ReturnRef(dispatcher)); EXPECT_CALL(factory_context, random()).WillOnce(ReturnRef(random)); @@ -3527,7 +3521,7 @@ TEST_P(SslSocketTest, UpstreamNotReadySslSocket) { EXPECT_TRUE(client_cfg->tlsCertificates().empty()); EXPECT_FALSE(client_cfg->isReady()); - ContextManagerImpl manager(time_system); + ContextManagerImpl manager(time_system_); ClientSslSocketFactory client_ssl_socket_factory(std::move(client_cfg), manager, stats_store); auto transport_socket = client_ssl_socket_factory.createTransportSocket(nullptr); EXPECT_EQ(EMPTY_STRING, transport_socket->protocol()); @@ -3546,8 +3540,7 @@ class SslReadBufferLimitTest : public SslSocketTest { downstream_tls_context_); auto server_cfg = std::make_unique(downstream_tls_context_, factory_context_); - Event::SimulatedTimeSystem time_system; - manager_ = std::make_unique(time_system); + manager_ = std::make_unique(time_system_); server_ssl_socket_factory_ = std::make_unique( std::move(server_cfg), *manager_, server_stats_store_, std::vector{}); @@ -3636,7 +3629,7 @@ class SslReadBufferLimitTest : public SslSocketTest { MockWatermarkBuffer* client_write_buffer = nullptr; MockBufferFactory* factory = new StrictMock; dispatcher_ = std::make_unique( - test_time_.timeSystem(), Buffer::WatermarkFactoryPtr{factory}, *api_); + time_system_, Buffer::WatermarkFactoryPtr{factory}, *api_); // By default, expect 4 buffers to be created - the client and server read and write buffers. EXPECT_CALL(*factory, create_(_, _)) diff --git a/test/integration/utility.cc b/test/integration/utility.cc index 76b280b4cbff8..5eb7c783cc751 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -54,8 +54,6 @@ void BufferingStreamDecoder::onComplete() { void BufferingStreamDecoder::onResetStream(Http::StreamResetReason) { ADD_FAILURE(); } -DangerousDeprecatedTestTime IntegrationUtil::evil_singleton_test_time_; - BufferingStreamDecoderPtr IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPtr& addr, const std::string& method, const std::string& url, @@ -64,7 +62,8 @@ IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPt NiceMock mock_stats_store; Api::Impl api(std::chrono::milliseconds(9000), Thread::threadFactoryForTest(), mock_stats_store); - Event::DispatcherPtr dispatcher(api.allocateDispatcher(evil_singleton_test_time_.timeSystem())); + Event::GlobalTimeSystem time_system; + Event::DispatcherPtr dispatcher(api.allocateDispatcher(*time_system)); std::shared_ptr cluster{new NiceMock()}; Upstream::HostDescriptionConstSharedPtr host_description{ Upstream::makeTestHostDescription(cluster, "tcp://127.0.0.1:80")}; @@ -112,7 +111,8 @@ RawConnectionDriver::RawConnectionDriver(uint32_t port, Buffer::Instance& initia ReadCallback data_callback, Network::Address::IpVersion version) { api_ = Api::createApiForTest(stats_store_); - dispatcher_ = api_->allocateDispatcher(IntegrationUtil::evil_singleton_test_time_.timeSystem()); + Event::GlobalTimeSystem time_system; + dispatcher_ = api_->allocateDispatcher(*time_system); callbacks_ = std::make_unique(); client_ = dispatcher_->createClientConnection( Network::Utility::resolveUrl( diff --git a/test/integration/utility.h b/test/integration/utility.h index 23630096ef289..0e3a483cd1bd8 100644 --- a/test/integration/utility.h +++ b/test/integration/utility.h @@ -146,9 +146,6 @@ class IntegrationUtil { const std::string& body, Http::CodecClient::Type type, Network::Address::IpVersion ip_version, const std::string& host = "host", const std::string& content_type = ""); - - // TODO(jmarantz): this should be injectable. - static DangerousDeprecatedTestTime evil_singleton_test_time_; }; // A set of connection callbacks which tracks connection state. diff --git a/test/mocks/common.h b/test/mocks/common.h index 3c787de7b8a74..5908a0f9a804c 100644 --- a/test/mocks/common.h +++ b/test/mocks/common.h @@ -49,18 +49,18 @@ class MockTimeSystem : public Event::TestTimeSystem { // matches recent behavior, where real-time timers were created directly in libevent // by dispatcher_impl.cc. Event::SchedulerPtr createScheduler(Event::Libevent::BasePtr& base) override { - return test_time_.timeSystem().createScheduler(base); + return real_time_.createScheduler(base); } - void sleep(const Duration& duration) override { test_time_.timeSystem().sleep(duration); } + void sleep(const Duration& duration) override { real_time_.sleep(duration); } Thread::CondVar::WaitStatus waitFor(Thread::MutexBasicLockable& mutex, Thread::CondVar& condvar, const Duration& duration) noexcept EXCLUSIVE_LOCKS_REQUIRED(mutex) override { - return test_time_.timeSystem().waitFor(mutex, condvar, duration); + return real_time_.waitFor(mutex, condvar, duration); // NO_CHECK_FORMAT(real_time) } MOCK_METHOD0(systemTime, SystemTime()); MOCK_METHOD0(monotonicTime, MonotonicTime()); - DangerousDeprecatedTestTime test_time_; + Event::TestRealTimeSystem real_time_; // NO_CHECK_FORMAT(real_time) }; class MockTokenBucket : public TokenBucket { diff --git a/test/mocks/event/mocks.cc b/test/mocks/event/mocks.cc index b5e2f9a1bee6c..59d724f85e4da 100644 --- a/test/mocks/event/mocks.cc +++ b/test/mocks/event/mocks.cc @@ -13,7 +13,7 @@ using testing::SaveArg; namespace Envoy { namespace Event { -MockDispatcher::MockDispatcher() : time_system_(&test_time_.timeSystem()) { +MockDispatcher::MockDispatcher() { ON_CALL(*this, clearDeferredDeleteList()).WillByDefault(Invoke([this]() -> void { to_delete_.clear(); })); diff --git a/test/mocks/event/mocks.h b/test/mocks/event/mocks.h index 6f11b90b1cc56..e3a31ce17ca35 100644 --- a/test/mocks/event/mocks.h +++ b/test/mocks/event/mocks.h @@ -30,10 +30,8 @@ class MockDispatcher : public Dispatcher { MockDispatcher(); ~MockDispatcher(); - void setTimeSystem(TimeSystem& time_system) { time_system_ = &time_system; } - // Dispatcher - TimeSystem& timeSystem() override { return *time_system_; } + TimeSystem& timeSystem() override { return time_system_; } Network::ConnectionPtr createServerConnection(Network::ConnectionSocketPtr&& socket, Network::TransportSocketPtr&& transport_socket) override { @@ -109,10 +107,7 @@ class MockDispatcher : public Dispatcher { MOCK_METHOD1(run, void(RunType type)); Buffer::WatermarkFactory& getWatermarkFactory() override { return buffer_factory_; } - // TODO(jmarantz): Switch these to using mock-time. - DangerousDeprecatedTestTime test_time_; - TimeSystem* time_system_; - + GlobalTimeSystem time_system_; std::list to_delete_; MockBufferFactory buffer_factory_; }; diff --git a/test/mocks/server/BUILD b/test/mocks/server/BUILD index cf04ace8041f9..fd64aafbf173c 100644 --- a/test/mocks/server/BUILD +++ b/test/mocks/server/BUILD @@ -43,6 +43,6 @@ envoy_cc_mock( "//test/mocks/thread_local:thread_local_mocks", "//test/mocks/tracing:tracing_mocks", "//test/mocks/upstream:upstream_mocks", - "//test/test_common:simulated_time_system_lib", + "//test/test_common:test_time_lib", ], ) diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index 636f4a97bd1ef..b5505a88281cc 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -39,7 +39,7 @@ #include "test/mocks/thread_local/mocks.h" #include "test/mocks/tracing/mocks.h" #include "test/mocks/upstream/mocks.h" -#include "test/test_common/simulated_time_system.h" +#include "test/test_common/test_time_system.h" #include "absl/strings/string_view.h" #include "gmock/gmock.h" @@ -355,7 +355,7 @@ class MockInstance : public Instance { MOCK_METHOD0(localInfo, const LocalInfo::LocalInfo&()); MOCK_CONST_METHOD0(statsFlushInterval, std::chrono::milliseconds()); - Event::TestTimeSystem& timeSystem() override { return test_time_.timeSystem(); } + Event::TestTimeSystem& timeSystem() override { return time_system_; } std::unique_ptr secret_manager_; testing::NiceMock thread_local_; @@ -364,7 +364,7 @@ class MockInstance : public Instance { new testing::NiceMock()}; testing::NiceMock api_; testing::NiceMock admin_; - DangerousDeprecatedTestTime test_time_; + Event::GlobalTimeSystem time_system_; testing::NiceMock cluster_manager_; Thread::MutexBasicLockable access_log_lock_; testing::NiceMock runtime_loader_; @@ -429,7 +429,7 @@ class MockFactoryContext : public virtual FactoryContext { MOCK_CONST_METHOD0(localInfo, const LocalInfo::LocalInfo&()); MOCK_CONST_METHOD0(listenerMetadata, const envoy::api::v2::core::Metadata&()); MOCK_METHOD0(timeSource, TimeSource&()); - Event::SimulatedTimeSystem& timeSystem() { return time_system_; } + Event::TestTimeSystem& timeSystem() { return time_system_; } Http::Context& httpContext() override { return http_context_; } testing::NiceMock access_log_manager_; @@ -446,7 +446,7 @@ class MockFactoryContext : public virtual FactoryContext { Singleton::ManagerPtr singleton_manager_; testing::NiceMock admin_; Stats::IsolatedStoreImpl listener_scope_; - Event::SimulatedTimeSystem time_system_; + Event::GlobalTimeSystem time_system_; testing::NiceMock overload_manager_; Tracing::HttpNullTracer null_tracer_; Http::ContextImpl http_context_; diff --git a/test/server/BUILD b/test/server/BUILD index 8f0db6396d0ff..0ba84d4870b45 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -102,6 +102,7 @@ envoy_cc_test( "//test/mocks:common_lib", "//test/mocks/server:server_mocks", "//test/mocks/stats:stats_mocks", + "//test/test_common:simulated_time_system_lib", "//test/test_common:utility_lib", ], ) diff --git a/test/test_common/BUILD b/test/test_common/BUILD index b60a122a9f2e5..4b164f56c4b55 100644 --- a/test/test_common/BUILD +++ b/test/test_common/BUILD @@ -160,6 +160,7 @@ envoy_cc_test_library( srcs = ["test_time.cc"], hdrs = ["test_time.h"], deps = [ + ":global_lib", ":test_time_system_interface", "//source/common/event:real_time_system_lib", ], @@ -169,6 +170,7 @@ envoy_cc_test_library( name = "test_time_system_interface", hdrs = ["test_time_system.h"], deps = [ + ":global_lib", "//include/envoy/event:timer_interface", "//source/common/common:thread_lib", ], @@ -189,6 +191,16 @@ envoy_cc_test( srcs = ["simulated_time_system_test.cc"], deps = [ ":simulated_time_system_lib", - "//test/test_common:utility_lib", + ":utility_lib", + ], +) + +envoy_cc_test( + name = "test_time_system_test", + srcs = ["test_time_system_test.cc"], + deps = [ + ":simulated_time_system_lib", + ":test_time_lib", + ":utility_lib", ], ) diff --git a/test/test_common/contention.cc b/test/test_common/contention.cc index 8461ef2d47cc6..10df692c4d8ef 100644 --- a/test/test_common/contention.cc +++ b/test/test_common/contention.cc @@ -7,28 +7,25 @@ namespace Thread { namespace TestUtil { void ContentionGenerator::generateContention(MutexTracerImpl& tracer) { - MutexBasicLockable mu; - Envoy::Thread::ThreadPtr t1 = launchThread(tracer, &mu); - Envoy::Thread::ThreadPtr t2 = launchThread(tracer, &mu); + Envoy::Thread::ThreadPtr t1 = launchThread(tracer); + Envoy::Thread::ThreadPtr t2 = launchThread(tracer); t1->join(); t2->join(); } -Envoy::Thread::ThreadPtr ContentionGenerator::launchThread(MutexTracerImpl& tracer, - MutexBasicLockable* mu) { +Envoy::Thread::ThreadPtr ContentionGenerator::launchThread(MutexTracerImpl& tracer) { return threadFactoryForTest().createThread( - [&tracer, mu]() -> void { holdUntilContention(tracer, mu); }); + [&tracer, this]() -> void { holdUntilContention(tracer); }); } -void ContentionGenerator::holdUntilContention(MutexTracerImpl& tracer, MutexBasicLockable* mu) { - DangerousDeprecatedTestTime test_time; +void ContentionGenerator::holdUntilContention(MutexTracerImpl& tracer) { int64_t curr_num_contentions = tracer.numContentions(); while (tracer.numContentions() == curr_num_contentions) { - test_time.timeSystem().sleep(std::chrono::milliseconds(1)); - LockGuard lock(*mu); + test_time_.timeSystem().sleep(std::chrono::milliseconds(1)); + LockGuard lock(mutex_); // We hold the lock 90% of the time to ensure both contention and eventual acquisition, which // is needed to bump numContentions(). - test_time.timeSystem().sleep(std::chrono::milliseconds(9)); + test_time_.timeSystem().sleep(std::chrono::milliseconds(9)); } } diff --git a/test/test_common/contention.h b/test/test_common/contention.h index ffa32d62ef1e7..77ce14d4872a8 100644 --- a/test/test_common/contention.h +++ b/test/test_common/contention.h @@ -23,12 +23,14 @@ class ContentionGenerator { /** * Generates at least once occurrence of mutex contention, as measured by tracer. */ - static void generateContention(MutexTracerImpl& tracer); + void generateContention(MutexTracerImpl& tracer); private: - static Envoy::Thread::ThreadPtr launchThread(MutexTracerImpl& tracer, MutexBasicLockable* mu); + ThreadPtr launchThread(MutexTracerImpl& tracer); + void holdUntilContention(MutexTracerImpl& tracer); - static void holdUntilContention(MutexTracerImpl& tracer, MutexBasicLockable* mu); + MutexBasicLockable mutex_; + DangerousDeprecatedTestTime test_time_; }; } // namespace TestUtil diff --git a/test/test_common/simulated_time_system.cc b/test/test_common/simulated_time_system.cc index de936e345a94b..26778f7c9c61b 100644 --- a/test/test_common/simulated_time_system.cc +++ b/test/test_common/simulated_time_system.cc @@ -18,9 +18,9 @@ namespace Event { // mechanism used in RealTimeSystem timers is employed for simulated alarms. // Note that libevent is placed into thread-safe mode due to the call to // evthread_use_pthreads() in source/common/event/libevent.cc. -class SimulatedTimeSystem::Alarm : public TimerImpl { +class SimulatedTimeSystemHelper::Alarm : public TimerImpl { public: - Alarm(SimulatedTimeSystem& time_system, Libevent::BasePtr& libevent, TimerCb cb) + Alarm(SimulatedTimeSystemHelper& time_system, Libevent::BasePtr& libevent, TimerCb cb) : TimerImpl(libevent, [this, cb] { runAlarm(cb); }), time_system_(time_system), index_(time_system.nextIndex()), armed_(false) {} @@ -56,7 +56,7 @@ class SimulatedTimeSystem::Alarm : public TimerImpl { cb(); } - SimulatedTimeSystem& time_system_; + SimulatedTimeSystemHelper& time_system_; MonotonicTime time_; uint64_t index_; bool armed_; @@ -64,7 +64,7 @@ class SimulatedTimeSystem::Alarm : public TimerImpl { // Compare two alarms, based on wakeup time and insertion order. Returns true if // a comes before b. -bool SimulatedTimeSystem::CompareAlarms::operator()(const Alarm* a, const Alarm* b) const { +bool SimulatedTimeSystemHelper::CompareAlarms::operator()(const Alarm* a, const Alarm* b) const { if (a != b) { if (a->time() < b->time()) { return true; @@ -79,33 +79,34 @@ bool SimulatedTimeSystem::CompareAlarms::operator()(const Alarm* a, const Alarm* // associated with a scheduler. The scheduler creates the timers with a libevent // context, so that the timer callbacks can be executed via Dispatcher::run() in // the expected thread. -class SimulatedTimeSystem::SimulatedScheduler : public Scheduler { +class SimulatedTimeSystemHelper::SimulatedScheduler : public Scheduler { public: - SimulatedScheduler(SimulatedTimeSystem& time_system, Libevent::BasePtr& libevent) + SimulatedScheduler(SimulatedTimeSystemHelper& time_system, Libevent::BasePtr& libevent) : time_system_(time_system), libevent_(libevent) {} TimerPtr createTimer(const TimerCb& cb) override { - return std::make_unique(time_system_, libevent_, cb); + return std::make_unique(time_system_, libevent_, cb); }; private: - SimulatedTimeSystem& time_system_; + SimulatedTimeSystemHelper& time_system_; Libevent::BasePtr& libevent_; }; -SimulatedTimeSystem::Alarm::~Alarm() { +SimulatedTimeSystemHelper::Alarm::Alarm::~Alarm() { if (armed_) { disableTimer(); } } -void SimulatedTimeSystem::Alarm::disableTimer() { +void SimulatedTimeSystemHelper::Alarm::Alarm::disableTimer() { if (armed_) { time_system_.removeAlarm(this); armed_ = false; } } -void SimulatedTimeSystem::Alarm::enableTimer(const std::chrono::milliseconds& duration) { +void SimulatedTimeSystemHelper::Alarm::Alarm::enableTimer( + const std::chrono::milliseconds& duration) { disableTimer(); armed_ = true; if (duration.count() == 0) { @@ -125,35 +126,37 @@ static int instance_count = 0; // When we initialize our simulated time, we'll start the current time based on // the real current time. But thereafter, real-time will not be used, and time // will march forward only by calling sleep(). -SimulatedTimeSystem::SimulatedTimeSystem() +SimulatedTimeSystemHelper::SimulatedTimeSystemHelper() : monotonic_time_(MonotonicTime(std::chrono::seconds(0))), system_time_(real_time_source_.systemTime()), index_(0), pending_alarms_(0) { ++instance_count; ASSERT(instance_count <= 1); } -SimulatedTimeSystem::~SimulatedTimeSystem() { --instance_count; } +SimulatedTimeSystemHelper::~SimulatedTimeSystemHelper() { --instance_count; } -SystemTime SimulatedTimeSystem::systemTime() { +bool SimulatedTimeSystemHelper::hasInstance() { return instance_count > 0; } + +SystemTime SimulatedTimeSystemHelper::systemTime() { Thread::LockGuard lock(mutex_); return system_time_; } -MonotonicTime SimulatedTimeSystem::monotonicTime() { +MonotonicTime SimulatedTimeSystemHelper::monotonicTime() { Thread::LockGuard lock(mutex_); return monotonic_time_; } -void SimulatedTimeSystem::sleep(const Duration& duration) { +void SimulatedTimeSystemHelper::sleep(const Duration& duration) { mutex_.lock(); MonotonicTime monotonic_time = monotonic_time_ + std::chrono::duration_cast(duration); setMonotonicTimeAndUnlock(monotonic_time); } -Thread::CondVar::WaitStatus SimulatedTimeSystem::waitFor(Thread::MutexBasicLockable& mutex, - Thread::CondVar& condvar, - const Duration& duration) noexcept { +Thread::CondVar::WaitStatus SimulatedTimeSystemHelper::waitFor(Thread::MutexBasicLockable& mutex, + Thread::CondVar& condvar, + const Duration& duration) noexcept { const Duration real_time_poll_delay(std::chrono::milliseconds(50)); const MonotonicTime end_time = monotonicTime() + duration; @@ -187,27 +190,27 @@ Thread::CondVar::WaitStatus SimulatedTimeSystem::waitFor(Thread::MutexBasicLocka return Thread::CondVar::WaitStatus::Timeout; } -int64_t SimulatedTimeSystem::nextIndex() { +int64_t SimulatedTimeSystemHelper::nextIndex() { Thread::LockGuard lock(mutex_); return index_++; } -void SimulatedTimeSystem::addAlarm(Alarm* alarm, const std::chrono::milliseconds& duration) { +void SimulatedTimeSystemHelper::addAlarm(Alarm* alarm, const std::chrono::milliseconds& duration) { Thread::LockGuard lock(mutex_); alarm->setTime(monotonic_time_ + duration); alarms_.insert(alarm); } -void SimulatedTimeSystem::removeAlarm(Alarm* alarm) { +void SimulatedTimeSystemHelper::removeAlarm(Alarm* alarm) { Thread::LockGuard lock(mutex_); alarms_.erase(alarm); } -SchedulerPtr SimulatedTimeSystem::createScheduler(Libevent::BasePtr& libevent) { +SchedulerPtr SimulatedTimeSystemHelper::createScheduler(Libevent::BasePtr& libevent) { return std::make_unique(*this, libevent); } -void SimulatedTimeSystem::setMonotonicTimeAndUnlock(const MonotonicTime& monotonic_time) { +void SimulatedTimeSystemHelper::setMonotonicTimeAndUnlock(const MonotonicTime& monotonic_time) { // We don't have a convenient LockGuard construct that allows temporarily // dropping the lock to run a callback. The main issue here is that we must // be careful not to be holding mutex_ when an exception can be thrown. @@ -243,7 +246,7 @@ void SimulatedTimeSystem::setMonotonicTimeAndUnlock(const MonotonicTime& monoton mutex_.unlock(); } -void SimulatedTimeSystem::setSystemTime(const SystemTime& system_time) { +void SimulatedTimeSystemHelper::setSystemTime(const SystemTime& system_time) { mutex_.lock(); if (system_time > system_time_) { MonotonicTime monotonic_time = diff --git a/test/test_common/simulated_time_system.h b/test/test_common/simulated_time_system.h index 9921581905507..f14693b449bea 100644 --- a/test/test_common/simulated_time_system.h +++ b/test/test_common/simulated_time_system.h @@ -1,5 +1,7 @@ #pragma once +#include "envoy/event/timer.h" + #include "common/common/lock_guard.h" #include "common/common/thread.h" #include "common/common/utility.h" @@ -9,14 +11,15 @@ namespace Envoy { namespace Event { -// Represents a simulated time system, where time is advanced by calling -// sleep(), setSystemTime(), or setMonotonicTime(). systemTime() and -// monotonicTime() are maintained in the class, and alarms are fired in response -// to adjustments in time. -class SimulatedTimeSystem : public TestTimeSystem { +// Implements a simulated time system including a scheduler for timers. This is +// designed to be used as as the exclusive time-system resident in a process at +// any particular time, and as such should not be instantiated directly by +// tests. Instead it should be instantied via SimulatedTimeSystem, declared +// below. +class SimulatedTimeSystemHelper : public TestTimeSystem { public: - SimulatedTimeSystem(); - ~SimulatedTimeSystem(); + SimulatedTimeSystemHelper(); + ~SimulatedTimeSystemHelper() override; // TimeSystem SchedulerPtr createScheduler(Libevent::BasePtr&) override; @@ -43,9 +46,6 @@ class SimulatedTimeSystem : public TestTimeSystem { mutex_.lock(); setMonotonicTimeAndUnlock(monotonic_time); } - template void setMonotonicTime(const Duration& duration) { - setMonotonicTime(MonotonicTime(duration)); - } /** * Sets the system-time, whether forward or backward. If time moves forward, @@ -55,9 +55,8 @@ class SimulatedTimeSystem : public TestTimeSystem { * @param system_time The desired new system time. */ void setSystemTime(const SystemTime& system_time); - template void setSystemTime(const Duration& duration) { - setSystemTime(SystemTime(duration)); - } + + static bool hasInstance(); private: class SimulatedScheduler; @@ -100,5 +99,24 @@ class SimulatedTimeSystem : public TestTimeSystem { std::atomic pending_alarms_; }; +// Represents a simulated time system, where time is advanced by calling +// sleep(), setSystemTime(), or setMonotonicTime(). systemTime() and +// monotonicTime() are maintained in the class, and alarms are fired in response +// to adjustments in time. +class SimulatedTimeSystem : public DelegatingTestTimeSystem { +public: + void setMonotonicTime(const MonotonicTime& monotonic_time) { + timeSystem().setMonotonicTime(monotonic_time); + } + void setSystemTime(const SystemTime& system_time) { timeSystem().setSystemTime(system_time); } + + template void setMonotonicTime(const Duration& duration) { + setMonotonicTime(MonotonicTime(duration)); + } + template void setSystemTime(const Duration& duration) { + setSystemTime(SystemTime(duration)); + } +}; + } // namespace Event } // namespace Envoy diff --git a/test/test_common/test_time.cc b/test/test_common/test_time.cc index ffe2fb29c5128..24bd072e6b4fb 100644 --- a/test/test_common/test_time.cc +++ b/test/test_common/test_time.cc @@ -2,6 +2,8 @@ #include "common/common/utility.h" +#include "test/test_common/global.h" + namespace Envoy { DangerousDeprecatedTestTime::DangerousDeprecatedTestTime() {} @@ -16,5 +18,9 @@ Thread::CondVar::WaitStatus TestRealTimeSystem::waitFor(Thread::MutexBasicLockab return condvar.waitFor(lock, duration); } +SystemTime TestRealTimeSystem::systemTime() { return real_time_system_.systemTime(); } + +MonotonicTime TestRealTimeSystem::monotonicTime() { return real_time_system_.monotonicTime(); } + } // namespace Event } // namespace Envoy diff --git a/test/test_common/test_time.h b/test/test_common/test_time.h index 5f7501305b440..ea0411f6a2c95 100644 --- a/test/test_common/test_time.h +++ b/test/test_common/test_time.h @@ -2,6 +2,7 @@ #include "common/event/real_time_system.h" +#include "test/test_common/global.h" #include "test/test_common/test_time_system.h" namespace Envoy { @@ -21,28 +22,41 @@ class TestRealTimeSystem : public TestTimeSystem { } // TimeSource - SystemTime systemTime() override { return real_time_system_.systemTime(); } - MonotonicTime monotonicTime() override { return real_time_system_.monotonicTime(); } + SystemTime systemTime() override; + MonotonicTime monotonicTime() override; private: Event::RealTimeSystem real_time_system_; }; +class GlobalTimeSystem : public DelegatingTestTimeSystemBase { +public: + TestTimeSystem& timeSystem() override { + if (singleton_->timeSystem() == nullptr) { + // TODO(#4160): Switch default to SimulatedTimeSystem. + singleton_->set(new TestRealTimeSystem); + } + return *singleton_->timeSystem(); + } + +private: + Test::Global singleton_; +}; + } // namespace Event // Instantiates real-time sources for testing purposes. In general, this is a // bad idea, and tests should use simulated or mock time. // -// TODO(#4160): change all references to this class to instantiate instead to -// some kind of mock or simulated-time source. +// TODO(#4160): change most references to this class to SimulatedTimeSystem. class DangerousDeprecatedTestTime { public: DangerousDeprecatedTestTime(); - Event::TestTimeSystem& timeSystem() { return time_system_; } + Event::TestTimeSystem& timeSystem() { return time_system_.timeSystem(); } private: - Event::TestRealTimeSystem time_system_; + Event::DelegatingTestTimeSystem time_system_; }; } // namespace Envoy diff --git a/test/test_common/test_time_system.h b/test/test_common/test_time_system.h index 60a8291249173..d7b1ec11b2e4e 100644 --- a/test/test_common/test_time_system.h +++ b/test/test_common/test_time_system.h @@ -2,14 +2,21 @@ #include "envoy/event/timer.h" +#include "common/common/assert.h" #include "common/common/thread.h" +#include "test/test_common/global.h" + namespace Envoy { namespace Event { +class TestTimeSystem; + // Adds sleep() and waitFor() interfaces to Event::TimeSystem. class TestTimeSystem : public Event::TimeSystem { public: + virtual ~TestTimeSystem() = default; + /** * Advances time forward by the specified duration, running any timers * along the way that have been scheduled to fire. @@ -41,5 +48,81 @@ class TestTimeSystem : public Event::TimeSystem { } }; +// There should only be one instance of any time-system resident in a test +// process at once. This helper class is used with Test::Global to help enforce +// that with an ASSERT. Each time-system derivation should have a helper +// implementation which is referenced from a delegate (see +// DelegatingTestTimeSystemBase). In each delegate, a SingletonTimeSystemHelper +// should be instantiated via Test::Global. Only one +// instance of SingletonTimeSystemHelper per process, at a time. When all +// references to the delegates are destructed, the singleton will be destroyed +// as well, so each test-method will get a fresh start. +class SingletonTimeSystemHelper { +public: + SingletonTimeSystemHelper() : time_system_(nullptr) {} + + void set(TestTimeSystem* time_system) { + RELEASE_ASSERT(time_system_ == nullptr, "Unexpected reset of SingletonTimeSystemHelper"); + time_system_.reset(time_system); + } + + TestTimeSystem* timeSystem() { return time_system_.get(); } + +private: + std::unique_ptr time_system_; +}; + +// Implements the TestTimeSystem interface, delegating implementation of all +// methods to a TestTimeSystem reference supplied by a timeSystem() method in a +// subclass. +template class DelegatingTestTimeSystemBase : public TestTimeSystem { +public: + void sleep(const Duration& duration) override { timeSystem().sleep(duration); } + + Thread::CondVar::WaitStatus + waitFor(Thread::MutexBasicLockable& mutex, Thread::CondVar& condvar, + const Duration& duration) noexcept EXCLUSIVE_LOCKS_REQUIRED(mutex) override { + return timeSystem().waitFor(mutex, condvar, duration); + } + + SchedulerPtr createScheduler(Libevent::BasePtr& base_ptr) override { + return timeSystem().createScheduler(base_ptr); + } + SystemTime systemTime() override { return timeSystem().systemTime(); } + MonotonicTime monotonicTime() override { return timeSystem().monotonicTime(); } + + TimeSystemVariant& operator*() { return timeSystem(); } + + virtual TimeSystemVariant& timeSystem() PURE; +}; + +// Wraps a concrete time-system in a delegate that ensures thare is only one +// time-system of any variant resident in a process at a time. Attempts to +// instantiate multiple instances of the same type of time-system will simply +// reference the same shared delegate, which will be deleted when the last one +// goes out of scope. Attempts to instantiate different types of type-systems +// will result in a RELEASE_ASSERT. See the testcases in +// test_time_system_test.cc to understand the allowable sequences. +template +class DelegatingTestTimeSystem : public DelegatingTestTimeSystemBase { +public: + DelegatingTestTimeSystem() { + TestTimeSystem* time_system = singleton_->timeSystem(); + if (time_system == nullptr) { + time_system_ = new TimeSystemVariant; + singleton_->set(time_system_); + } else { + time_system_ = dynamic_cast(time_system); + RELEASE_ASSERT(time_system_, "Two different types of time-systems allocated"); + } + } + + TimeSystemVariant& timeSystem() override { return *time_system_; } + +private: + TimeSystemVariant* time_system_; + Test::Global singleton_; +}; + } // namespace Event } // namespace Envoy diff --git a/test/test_common/test_time_system_test.cc b/test/test_common/test_time_system_test.cc new file mode 100644 index 0000000000000..30268d3fe6bfa --- /dev/null +++ b/test/test_common/test_time_system_test.cc @@ -0,0 +1,50 @@ +#include "test/test_common/simulated_time_system.h" +#include "test/test_common/test_time.h" +#include "test/test_common/test_time_system.h" +#include "test/test_common/utility.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Event { +namespace Test { + +class TestTimeSystemTest : public testing::Test { +protected: +}; + +TEST_F(TestTimeSystemTest, TwoSimsSameReference) { + SimulatedTimeSystem t1, t2; + EXPECT_EQ(&t1.timeSystem(), &t2.timeSystem()); +} + +TEST_F(TestTimeSystemTest, TwoRealsSameReference) { + DangerousDeprecatedTestTime t1, t2; + EXPECT_EQ(&t1.timeSystem(), &t2.timeSystem()); +} + +TEST_F(TestTimeSystemTest, SimThenRealConflict) { + SimulatedTimeSystem t1; + EXPECT_DEATH_LOG_TO_STDERR({ DangerousDeprecatedTestTime t2; }, + ".*Two different types of time-systems allocated.*"); +} + +TEST_F(TestTimeSystemTest, SimThenRealSerial) { + { SimulatedTimeSystem t1; } + { DangerousDeprecatedTestTime t2; } +} + +TEST_F(TestTimeSystemTest, RealThenSim) { + DangerousDeprecatedTestTime t1; + EXPECT_DEATH_LOG_TO_STDERR({ SimulatedTimeSystem t2; }, + ".*Two different types of time-systems allocated.*"); +} + +TEST_F(TestTimeSystemTest, RealThenSimSerial) { + { DangerousDeprecatedTestTime t2; } + { SimulatedTimeSystem t1; } +} + +} // namespace Test +} // namespace Event +} // namespace Envoy diff --git a/tools/check_format.py b/tools/check_format.py index efd065eb4b320..6468a275a97a7 100755 --- a/tools/check_format.py +++ b/tools/check_format.py @@ -190,6 +190,8 @@ def whitelistedForProtobufDeps(file_path): # specific cases. They should be passed down from where they are instantied to where # they need to be used, e.g. through the ServerInstance, Dispatcher, or ClusterManager. def whitelistedForRealTime(file_path): + if file_path.endswith(".md"): + return True return file_path in REAL_TIME_WHITELIST