reviewer feedback

3 files changed, 21 insertions, 63 deletions

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 <grpc/support/string_util.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -157,7 +155,7 @@ TEST_P(ChannelTracerTest, ComplexTest) {
   AddSimpleTrace(&tracer);
   ChannelFixture channel1(GetParam());
   RefCountedPtr<ChannelNode> sc1 =
-      MakeRefCounted<ChannelNode>(channel1.channel(), GetParam());
+      MakeRefCounted<ChannelNode>(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<ChannelNode> sc2 =
-      MakeRefCounted<ChannelNode>(channel2.channel(), GetParam());
+      MakeRefCounted<ChannelNode>(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<ChannelNode> sc1 =
-      MakeRefCounted<ChannelNode>(channel1.channel(), GetParam());
+      MakeRefCounted<ChannelNode>(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<ChannelNode> conn1 =
-      MakeRefCounted<ChannelNode>(channel2.channel(), GetParam());
+      MakeRefCounted<ChannelNode>(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<ChannelNode> sc2 =
-      MakeRefCounted<ChannelNode>(channel3.channel(), GetParam());
+      MakeRefCounted<ChannelNode>(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<char*>(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<intptr_t> 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<char*>(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<char*>(GRPC_ARG_ENABLE_CHANNELZ);
-    client_a[1].value.integer = true;
+    client_a[0] = grpc_channel_arg_integer_create(
+        const_cast<char*>(GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE),
+        max_trace_nodes);
+    client_a[1] = grpc_channel_arg_integer_create(
+        const_cast<char*>(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<char*>(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<char*>(GRPC_ARG_ENABLE_CHANNELZ);
-  client_a[1].value.integer = true;
+  client_a[0] = grpc_channel_arg_integer_create(
+      const_cast<char*>(GRPC_ARG_CHANNELZ_CHANNEL_IS_INTERNAL_CHANNEL), true);
+  client_a[1] = grpc_channel_arg_integer_create(
+      const_cast<char*>(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);