From 5d373c40bdd616d2d0e65b1c6936cc2d07ba4044 Mon Sep 17 00:00:00 2001 From: ncteisen Date: Tue, 17 Jul 2018 11:57:31 -0700 Subject: reviewer feedback --- test/core/channel/channel_trace_test.cc | 12 ++++---- test/core/channel/channelz_registry_test.cc | 45 ++++------------------------- test/core/channel/channelz_test.cc | 27 +++++++---------- 3 files changed, 21 insertions(+), 63 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 07b0e3de06..99d9a4847f 100644 --- a/test/core/channel/channel_trace_test.cc +++ b/test/core/channel/channel_trace_test.cc @@ -34,8 +34,6 @@ #include "test/core/util/test_config.h" #include "test/cpp/util/channel_trace_proto_helper.h" -// remove me -#include #include #include @@ -157,7 +155,7 @@ TEST_P(ChannelTracerTest, ComplexTest) { AddSimpleTrace(&tracer); ChannelFixture channel1(GetParam()); RefCountedPtr sc1 = - MakeRefCounted(channel1.channel(), GetParam()); + MakeRefCounted(channel1.channel(), GetParam(), true); tracer.AddTraceEventReferencingSubchannel( ChannelTrace::Severity::Info, grpc_slice_from_static_string("subchannel one created"), sc1); @@ -175,7 +173,7 @@ TEST_P(ChannelTracerTest, ComplexTest) { ValidateChannelTrace(&tracer, 5, GetParam()); ChannelFixture channel2(GetParam()); RefCountedPtr sc2 = - MakeRefCounted(channel2.channel(), GetParam()); + MakeRefCounted(channel2.channel(), GetParam(), true); tracer.AddTraceEventReferencingChannel( ChannelTrace::Severity::Info, grpc_slice_from_static_string("LB channel two created"), sc2); @@ -204,7 +202,7 @@ TEST_P(ChannelTracerTest, TestNesting) { ValidateChannelTrace(&tracer, 2, GetParam()); ChannelFixture channel1(GetParam()); RefCountedPtr sc1 = - MakeRefCounted(channel1.channel(), GetParam()); + MakeRefCounted(channel1.channel(), GetParam(), true); tracer.AddTraceEventReferencingChannel( ChannelTrace::Severity::Info, grpc_slice_from_static_string("subchannel one created"), sc1); @@ -212,7 +210,7 @@ TEST_P(ChannelTracerTest, TestNesting) { AddSimpleTrace(sc1->trace()); ChannelFixture channel2(GetParam()); RefCountedPtr conn1 = - MakeRefCounted(channel2.channel(), GetParam()); + MakeRefCounted(channel2.channel(), GetParam(), true); // nesting one level deeper. sc1->trace()->AddTraceEventReferencingSubchannel( ChannelTrace::Severity::Info, @@ -225,7 +223,7 @@ TEST_P(ChannelTracerTest, TestNesting) { ValidateChannelTrace(conn1->trace(), 1, GetParam()); ChannelFixture channel3(GetParam()); RefCountedPtr sc2 = - MakeRefCounted(channel3.channel(), GetParam()); + MakeRefCounted(channel3.channel(), GetParam(), true); tracer.AddTraceEventReferencingSubchannel( ChannelTrace::Severity::Info, grpc_slice_from_static_string("subchannel two created"), sc2); diff --git a/test/core/channel/channelz_registry_test.cc b/test/core/channel/channelz_registry_test.cc index 06363d00e2..990cd3d205 100644 --- a/test/core/channel/channelz_registry_test.cc +++ b/test/core/channel/channelz_registry_test.cc @@ -42,36 +42,9 @@ namespace grpc_core { namespace channelz { namespace testing { -namespace { -class ChannelFixture { - public: - ChannelFixture() { - grpc_arg client_a[1]; - client_a[0].type = GRPC_ARG_INTEGER; - client_a[0].key = const_cast(GRPC_ARG_ENABLE_CHANNELZ); - client_a[0].value.integer = true; - grpc_channel_args client_args = {GPR_ARRAY_SIZE(client_a), client_a}; - channel_ = - grpc_insecure_channel_create("fake_target", &client_args, nullptr); - } - - ~ChannelFixture() { grpc_channel_destroy(channel_); } - - grpc_channel* channel() { return channel_; } - - private: - grpc_channel* channel_; -}; - -} // namespace - -// Tests basic ChannelTrace functionality like construction, adding trace, and -// lookups by uuid. TEST(ChannelzRegistryTest, UuidStartsAboveZeroTest) { - ChannelFixture channel; - ChannelNode* channelz_channel = - grpc_channel_get_channelz_node(channel.channel()); + ChannelNode* channelz_channel = nullptr; intptr_t uuid = ChannelzRegistry::RegisterChannelNode(channelz_channel); EXPECT_GT(uuid, 0) << "First uuid chose must be greater than zero. Zero if " "reserved according to " @@ -81,9 +54,7 @@ TEST(ChannelzRegistryTest, UuidStartsAboveZeroTest) { } TEST(ChannelzRegistryTest, UuidsAreIncreasing) { - ChannelFixture channel; - ChannelNode* channelz_channel = - grpc_channel_get_channelz_node(channel.channel()); + ChannelNode* channelz_channel = nullptr; std::vector uuids; uuids.reserve(10); for (int i = 0; i < 10; ++i) { @@ -96,18 +67,14 @@ TEST(ChannelzRegistryTest, UuidsAreIncreasing) { } TEST(ChannelzRegistryTest, RegisterGetTest) { - ChannelFixture channel; - ChannelNode* channelz_channel = - grpc_channel_get_channelz_node(channel.channel()); + ChannelNode* channelz_channel = nullptr; intptr_t uuid = ChannelzRegistry::RegisterChannelNode(channelz_channel); ChannelNode* retrieved = ChannelzRegistry::GetChannelNode(uuid); EXPECT_EQ(channelz_channel, retrieved); } TEST(ChannelzRegistryTest, RegisterManyItems) { - ChannelFixture channel; - ChannelNode* channelz_channel = - grpc_channel_get_channelz_node(channel.channel()); + ChannelNode* channelz_channel = nullptr; for (int i = 0; i < 100; i++) { intptr_t uuid = ChannelzRegistry::RegisterChannelNode(channelz_channel); ChannelNode* retrieved = ChannelzRegistry::GetChannelNode(uuid); @@ -116,9 +83,7 @@ TEST(ChannelzRegistryTest, RegisterManyItems) { } TEST(ChannelzRegistryTest, NullIfNotPresentTest) { - ChannelFixture channel; - ChannelNode* channelz_channel = - grpc_channel_get_channelz_node(channel.channel()); + ChannelNode* channelz_channel = nullptr; intptr_t uuid = ChannelzRegistry::RegisterChannelNode(channelz_channel); // try to pull out a uuid that does not exist. ChannelNode* nonexistant = ChannelzRegistry::GetChannelNode(uuid + 1); diff --git a/test/core/channel/channelz_test.cc b/test/core/channel/channelz_test.cc index 1ed36acb61..d12f529726 100644 --- a/test/core/channel/channelz_test.cc +++ b/test/core/channel/channelz_test.cc @@ -80,7 +80,7 @@ void ValidateJsonArraySize(grpc_json* json, const char* key, for (grpc_json* child = arr->child; child != nullptr; child = child->next) { ++count; } - ASSERT_EQ(count, expected_size); + EXPECT_EQ(count, expected_size); } void ValidateGetTopChannels(size_t expected_channels) { @@ -91,7 +91,7 @@ void ValidateGetTopChannels(size_t expected_channels) { // tracked: https://github.com/grpc/grpc/issues/16019. ValidateJsonArraySize(parsed_json, "channel", expected_channels); grpc_json* end = GetJsonChild(parsed_json, "end"); - EXPECT_NE(end, nullptr); + ASSERT_NE(end, nullptr); EXPECT_EQ(end->type, GRPC_JSON_TRUE); grpc_json_destroy(parsed_json); gpr_free(json_str); @@ -101,13 +101,11 @@ class ChannelFixture { public: ChannelFixture(int max_trace_nodes = 0) { grpc_arg client_a[2]; - client_a[0].type = GRPC_ARG_INTEGER; - client_a[0].key = - const_cast(GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE); - client_a[0].value.integer = max_trace_nodes; - client_a[1].type = GRPC_ARG_INTEGER; - client_a[1].key = const_cast(GRPC_ARG_ENABLE_CHANNELZ); - client_a[1].value.integer = true; + client_a[0] = grpc_channel_arg_integer_create( + const_cast(GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE), + max_trace_nodes); + client_a[1] = grpc_channel_arg_integer_create( + const_cast(GRPC_ARG_ENABLE_CHANNELZ), true); grpc_channel_args client_args = {GPR_ARRAY_SIZE(client_a), client_a}; channel_ = grpc_insecure_channel_create("fake_target", &client_args, nullptr); @@ -255,13 +253,10 @@ TEST(ChannelzGetTopChannelsTest, InternalChannelTest) { (void)channels; // suppress unused variable error // create an internal channel grpc_arg client_a[2]; - client_a[0].type = GRPC_ARG_INTEGER; - client_a[0].key = - const_cast(GRPC_ARG_CHANNELZ_CHANNEL_IS_INTERNAL_CHANNEL); - client_a[0].value.integer = 1; - client_a[1].type = GRPC_ARG_INTEGER; - client_a[1].key = const_cast(GRPC_ARG_ENABLE_CHANNELZ); - client_a[1].value.integer = true; + client_a[0] = grpc_channel_arg_integer_create( + const_cast(GRPC_ARG_CHANNELZ_CHANNEL_IS_INTERNAL_CHANNEL), true); + client_a[1] = grpc_channel_arg_integer_create( + const_cast(GRPC_ARG_ENABLE_CHANNELZ), true); grpc_channel_args client_args = {GPR_ARRAY_SIZE(client_a), client_a}; grpc_channel* internal_channel = grpc_insecure_channel_create("fake_target", &client_args, nullptr); -- cgit v1.2.3