From 72ec9b2745a8fd58fe1a91235b3711df9caee18c Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Wed, 22 Aug 2018 12:30:09 -0400 Subject: [PATCH 1/2] Clean up initialization and allocation of timers in the test. Signed-off-by: Joshua Marantz --- test/common/config/grpc_mux_impl_test.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/common/config/grpc_mux_impl_test.cc b/test/common/config/grpc_mux_impl_test.cc index 170fc64e8956..9520d9cd5052 100644 --- a/test/common/config/grpc_mux_impl_test.cc +++ b/test/common/config/grpc_mux_impl_test.cc @@ -34,15 +34,9 @@ namespace { // is provided in [grpc_]subscription_impl_test.cc. class GrpcMuxImplTest : public testing::Test { public: - GrpcMuxImplTest() : async_client_(new Grpc::MockAsyncClient()), time_source_{} {} + GrpcMuxImplTest() : async_client_(new Grpc::MockAsyncClient()), timer_(nullptr), time_source_{} {} void setup() { - EXPECT_CALL(dispatcher_, createTimer_(_)).WillOnce(Invoke([this](Event::TimerCb timer_cb) { - timer_cb_ = timer_cb; - timer_ = new Event::MockTimer(); - return timer_; - })); - grpc_mux_.reset(new GrpcMuxImpl( local_info_, std::unique_ptr(async_client_), dispatcher_, *Protobuf::DescriptorPool::generated_pool()->FindMethodByName( @@ -76,8 +70,8 @@ class GrpcMuxImplTest : public testing::Test { NiceMock dispatcher_; Runtime::MockRandomGenerator random_; Grpc::MockAsyncClient* async_client_; - Event::MockTimer* timer_; - Event::TimerCb timer_cb_; + Event::MockTimer* timer_; // Only used by ResetStream + Event::TimerCb timer_cb_; // Only used by ResetStream Grpc::MockAsyncStream async_stream_; std::unique_ptr grpc_mux_; NiceMock callbacks_; @@ -107,6 +101,11 @@ TEST_F(GrpcMuxImplTest, MultipleTypeUrlStreams) { // Validate behavior when multiple type URL watches are maintained and the stream is reset. TEST_F(GrpcMuxImplTest, ResetStream) { + EXPECT_CALL(dispatcher_, createTimer_(_)).WillOnce(Invoke([this](Event::TimerCb timer_cb) { + timer_cb_ = timer_cb; + timer_ = new Event::MockTimer(); + return timer_; + })); setup(); InSequence s; auto foo_sub = grpc_mux_->subscribe("foo", {"x", "y"}, callbacks_); @@ -119,6 +118,7 @@ TEST_F(GrpcMuxImplTest, ResetStream) { grpc_mux_->start(); EXPECT_CALL(random_, random()); + ASSERT_TRUE(timer_ != nullptr); // timer_ initialized from dispatcher mock. EXPECT_CALL(*timer_, enableTimer(_)); grpc_mux_->onRemoteClose(Grpc::Status::GrpcStatus::Canceled, ""); EXPECT_CALL(*async_client_, start(_, _)).WillOnce(Return(&async_stream_)); From 098414773eafe11463edd2d76829573f5bf8f00d Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Wed, 22 Aug 2018 18:34:24 -0400 Subject: [PATCH 2/2] Sink the test-specific member-vars into locals into the test-method. Signed-off-by: Joshua Marantz --- test/common/config/grpc_mux_impl_test.cc | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/test/common/config/grpc_mux_impl_test.cc b/test/common/config/grpc_mux_impl_test.cc index 9520d9cd5052..16416f6a0a72 100644 --- a/test/common/config/grpc_mux_impl_test.cc +++ b/test/common/config/grpc_mux_impl_test.cc @@ -34,7 +34,7 @@ namespace { // is provided in [grpc_]subscription_impl_test.cc. class GrpcMuxImplTest : public testing::Test { public: - GrpcMuxImplTest() : async_client_(new Grpc::MockAsyncClient()), timer_(nullptr), time_source_{} {} + GrpcMuxImplTest() : async_client_(new Grpc::MockAsyncClient()), time_source_{} {} void setup() { grpc_mux_.reset(new GrpcMuxImpl( @@ -70,8 +70,6 @@ class GrpcMuxImplTest : public testing::Test { NiceMock dispatcher_; Runtime::MockRandomGenerator random_; Grpc::MockAsyncClient* async_client_; - Event::MockTimer* timer_; // Only used by ResetStream - Event::TimerCb timer_cb_; // Only used by ResetStream Grpc::MockAsyncStream async_stream_; std::unique_ptr grpc_mux_; NiceMock callbacks_; @@ -101,10 +99,13 @@ TEST_F(GrpcMuxImplTest, MultipleTypeUrlStreams) { // Validate behavior when multiple type URL watches are maintained and the stream is reset. TEST_F(GrpcMuxImplTest, ResetStream) { - EXPECT_CALL(dispatcher_, createTimer_(_)).WillOnce(Invoke([this](Event::TimerCb timer_cb) { - timer_cb_ = timer_cb; - timer_ = new Event::MockTimer(); - return timer_; + Event::MockTimer* timer = nullptr; + Event::TimerCb timer_cb; + EXPECT_CALL(dispatcher_, createTimer_(_)).WillOnce(Invoke([&timer, &timer_cb](Event::TimerCb cb) { + timer_cb = cb; + EXPECT_EQ(nullptr, timer); + timer = new Event::MockTimer(); + return timer; })); setup(); InSequence s; @@ -118,14 +119,14 @@ TEST_F(GrpcMuxImplTest, ResetStream) { grpc_mux_->start(); EXPECT_CALL(random_, random()); - ASSERT_TRUE(timer_ != nullptr); // timer_ initialized from dispatcher mock. - EXPECT_CALL(*timer_, enableTimer(_)); + ASSERT_TRUE(timer != nullptr); // initialized from dispatcher mock. + EXPECT_CALL(*timer, enableTimer(_)); grpc_mux_->onRemoteClose(Grpc::Status::GrpcStatus::Canceled, ""); EXPECT_CALL(*async_client_, start(_, _)).WillOnce(Return(&async_stream_)); expectSendMessage("foo", {"x", "y"}, ""); expectSendMessage("bar", {}, ""); expectSendMessage("baz", {"z"}, ""); - timer_cb_(); + timer_cb(); expectSendMessage("baz", {}, ""); expectSendMessage("foo", {}, "");