From 58db94e5d9f3e55f14760f936a42e5e6ddb6bd5b Mon Sep 17 00:00:00 2001 From: ncteisen Date: Sun, 29 Jul 2018 20:30:05 -0700 Subject: reviewer comments --- test/core/channel/channel_trace_test.cc | 25 ++++++++-------- test/core/channel/channelz_test.cc | 53 ++++++++++++++------------------- 2 files changed, 35 insertions(+), 43 deletions(-) (limited to 'test/core/channel') diff --git a/test/core/channel/channel_trace_test.cc b/test/core/channel/channel_trace_test.cc index 9bc9dde8d6..3f5aa85c8c 100644 --- a/test/core/channel/channel_trace_test.cc +++ b/test/core/channel/channel_trace_test.cc @@ -89,7 +89,6 @@ void ValidateChannelTrace(ChannelTrace* tracer, grpc_json* json = tracer->RenderJson(); EXPECT_NE(json, nullptr); char* json_str = grpc_json_dump_to_string(json, 0); - gpr_log(GPR_ERROR, "%s", json_str); grpc_json_destroy(json); grpc::testing::ValidateChannelTraceProtoJsonTranslation(json_str); grpc_json* parsed_json = grpc_json_parse_string(json_str); @@ -161,14 +160,14 @@ TEST_P(ChannelTracerTest, ComplexTest) { ChannelTrace::Severity::Info, grpc_slice_from_static_string("subchannel one created"), sc1); ValidateChannelTrace(&tracer, 3, GetParam()); - AddSimpleTrace(sc1->counter_and_tracer()->trace()); - AddSimpleTrace(sc1->counter_and_tracer()->trace()); - AddSimpleTrace(sc1->counter_and_tracer()->trace()); - ValidateChannelTrace(sc1->counter_and_tracer()->trace(), 3, GetParam()); - AddSimpleTrace(sc1->counter_and_tracer()->trace()); - AddSimpleTrace(sc1->counter_and_tracer()->trace()); - AddSimpleTrace(sc1->counter_and_tracer()->trace()); - ValidateChannelTrace(sc1->counter_and_tracer()->trace(), 6, GetParam()); + AddSimpleTrace(sc1->trace()); + AddSimpleTrace(sc1->trace()); + AddSimpleTrace(sc1->trace()); + ValidateChannelTrace(sc1->trace(), 3, GetParam()); + AddSimpleTrace(sc1->trace()); + AddSimpleTrace(sc1->trace()); + AddSimpleTrace(sc1->trace()); + ValidateChannelTrace(sc1->trace(), 6, GetParam()); AddSimpleTrace(&tracer); AddSimpleTrace(&tracer); ValidateChannelTrace(&tracer, 5, GetParam()); @@ -208,20 +207,20 @@ TEST_P(ChannelTracerTest, TestNesting) { ChannelTrace::Severity::Info, grpc_slice_from_static_string("subchannel one created"), sc1); ValidateChannelTrace(&tracer, 3, GetParam()); - AddSimpleTrace(sc1->counter_and_tracer()->trace()); + AddSimpleTrace(sc1->trace()); ChannelFixture channel2(GetParam()); RefCountedPtr conn1 = MakeRefCounted(channel2.channel(), GetParam(), true); // nesting one level deeper. - sc1->counter_and_tracer()->trace()->AddTraceEventWithReference( + sc1->trace()->AddTraceEventWithReference( ChannelTrace::Severity::Info, grpc_slice_from_static_string("connection one created"), conn1); ValidateChannelTrace(&tracer, 3, GetParam()); - AddSimpleTrace(conn1->counter_and_tracer()->trace()); + AddSimpleTrace(conn1->trace()); AddSimpleTrace(&tracer); AddSimpleTrace(&tracer); ValidateChannelTrace(&tracer, 5, GetParam()); - ValidateChannelTrace(conn1->counter_and_tracer()->trace(), 1, GetParam()); + ValidateChannelTrace(conn1->trace(), 1, GetParam()); ChannelFixture channel3(GetParam()); RefCountedPtr sc2 = MakeRefCounted(channel3.channel(), GetParam(), true); diff --git a/test/core/channel/channelz_test.cc b/test/core/channel/channelz_test.cc index 5e9a3f89a2..8fa46a18da 100644 --- a/test/core/channel/channelz_test.cc +++ b/test/core/channel/channelz_test.cc @@ -44,17 +44,16 @@ namespace channelz { namespace testing { // testing peer to access channel internals -class CallCountingAndTracingNodePeer { +class CallCountingHelperPeer { public: - CallCountingAndTracingNodePeer(CallCountingAndTracingNode* node) - : node_(node) {} + CallCountingHelperPeer(CallCountingHelper* node) : node_(node) {} grpc_millis last_call_started_millis() { return (grpc_millis)gpr_atm_no_barrier_load( &node_->last_call_started_millis_); } private: - CallCountingAndTracingNode* node_; + CallCountingHelper* node_; }; namespace { @@ -164,8 +163,8 @@ void ValidateChannel(ChannelNode* channel, validate_channel_data_args args) { gpr_free(core_api_json_str); } -grpc_millis GetLastCallStartedMillis(CallCountingAndTracingNode* channel) { - CallCountingAndTracingNodePeer peer(channel); +grpc_millis GetLastCallStartedMillis(CallCountingHelper* channel) { + CallCountingHelperPeer peer(channel); return peer.last_call_started_millis(); } @@ -201,46 +200,40 @@ TEST_P(ChannelzChannelTest, BasicChannelAPIFunctionality) { ChannelFixture channel(GetParam()); ChannelNode* channelz_channel = grpc_channel_get_channelz_node(channel.channel()); - channelz_channel->counter_and_tracer()->RecordCallStarted(); - channelz_channel->counter_and_tracer()->RecordCallFailed(); - channelz_channel->counter_and_tracer()->RecordCallSucceeded(); + channelz_channel->RecordCallStarted(); + channelz_channel->RecordCallFailed(); + channelz_channel->RecordCallSucceeded(); ValidateChannel(channelz_channel, {1, 1, 1}); - channelz_channel->counter_and_tracer()->RecordCallStarted(); - channelz_channel->counter_and_tracer()->RecordCallFailed(); - channelz_channel->counter_and_tracer()->RecordCallSucceeded(); - channelz_channel->counter_and_tracer()->RecordCallStarted(); - channelz_channel->counter_and_tracer()->RecordCallFailed(); - channelz_channel->counter_and_tracer()->RecordCallSucceeded(); + channelz_channel->RecordCallStarted(); + channelz_channel->RecordCallFailed(); + channelz_channel->RecordCallSucceeded(); + channelz_channel->RecordCallStarted(); + channelz_channel->RecordCallFailed(); + channelz_channel->RecordCallSucceeded(); ValidateChannel(channelz_channel, {3, 3, 3}); } TEST_P(ChannelzChannelTest, LastCallStartedMillis) { grpc_core::ExecCtx exec_ctx; - ChannelFixture channel(GetParam()); - ChannelNode* channelz_channel = - grpc_channel_get_channelz_node(channel.channel()); + CallCountingHelper counter; // start a call to set the last call started timestamp - channelz_channel->counter_and_tracer()->RecordCallStarted(); - grpc_millis millis1 = - GetLastCallStartedMillis(channelz_channel->counter_and_tracer()); + counter.RecordCallStarted(); + grpc_millis millis1 = GetLastCallStartedMillis(&counter); // time gone by should not affect the timestamp ChannelzSleep(100); - grpc_millis millis2 = - GetLastCallStartedMillis(channelz_channel->counter_and_tracer()); + grpc_millis millis2 = GetLastCallStartedMillis(&counter); EXPECT_EQ(millis1, millis2); // calls succeeded or failed should not affect the timestamp ChannelzSleep(100); - channelz_channel->counter_and_tracer()->RecordCallFailed(); - channelz_channel->counter_and_tracer()->RecordCallSucceeded(); - grpc_millis millis3 = - GetLastCallStartedMillis(channelz_channel->counter_and_tracer()); + counter.RecordCallFailed(); + counter.RecordCallSucceeded(); + grpc_millis millis3 = GetLastCallStartedMillis(&counter); EXPECT_EQ(millis1, millis3); // another call started should affect the timestamp // sleep for extra long to avoid flakes (since we cache Now()) ChannelzSleep(5000); - channelz_channel->counter_and_tracer()->RecordCallStarted(); - grpc_millis millis4 = - GetLastCallStartedMillis(channelz_channel->counter_and_tracer()); + counter.RecordCallStarted(); + grpc_millis millis4 = GetLastCallStartedMillis(&counter); EXPECT_NE(millis1, millis4); } -- cgit v1.2.3