Merge pull request #17410 from ericgribkoff/channelz_stop_server

stop() server and enable skipped channelz test

src/python/grpcio_tests/tests/channelz/_channelz_servicer_test.py

@@ -69,32 +69,28 @@ class _ChannelServerPair(object):
 
     def __init__(self):
         # Server will enable channelz service
-        # Bind as attribute, so its `del` can be called explicitly, during
-        #   the destruction process. Otherwise, if the removal of server
-        #   rely on gc cycle, the test will become non-deterministic.
-        self._server = grpc.server(
+        self.server = grpc.server(
             futures.ThreadPoolExecutor(max_workers=3),
             options=_DISABLE_REUSE_PORT + _ENABLE_CHANNELZ)
-        port = self._server.add_insecure_port('[::]:0')
-        self._server.add_generic_rpc_handlers((_GenericHandler(),))
-        self._server.start()
+        port = self.server.add_insecure_port('[::]:0')
+        self.server.add_generic_rpc_handlers((_GenericHandler(),))
+        self.server.start()
 
         # Channel will enable channelz service...
         self.channel = grpc.insecure_channel('localhost:%d' % port,
                                              _ENABLE_CHANNELZ)
 
-    def __del__(self):
-        self._server.__del__()
-        self.channel.close()
-
 
 def _generate_channel_server_pairs(n):
     return [_ChannelServerPair() for i in range(n)]
 
 
-def _clean_channel_server_pairs(pairs):
+def _close_channel_server_pairs(pairs):
     for pair in pairs:
-        pair.__del__()
+        pair.server.stop(None)
+        # TODO(ericgribkoff) This del should not be required
+        del pair.server
+        pair.channel.close()
 
 
 class ChannelzServicerTest(unittest.TestCase):
@@ -147,9 +143,9 @@ class ChannelzServicerTest(unittest.TestCase):
         self._channelz_stub = channelz_pb2_grpc.ChannelzStub(self._channel)
 
     def tearDown(self):
-        self._server.__del__()
+        self._server.stop(None)
         self._channel.close()
-        _clean_channel_server_pairs(self._pairs)
+        _close_channel_server_pairs(self._pairs)
 
     def test_get_top_channels_basic(self):
         self._pairs = _generate_channel_server_pairs(1)
@@ -278,20 +274,12 @@ class ChannelzServicerTest(unittest.TestCase):
             self.assertEqual(gtc_resp.channel[i].data.calls_failed,
                              gsc_resp.subchannel.data.calls_failed)
 
-    @unittest.skip('Servers in core are not guaranteed to be destroyed ' \
-                   'immediately when the reference goes out of scope, so ' \
-                   'servers from multiple test cases are not hermetic. ' \
-                   'TODO(https://github.com/grpc/grpc/issues/17258)')
     def test_server_basic(self):
         self._pairs = _generate_channel_server_pairs(1)
         resp = self._channelz_stub.GetServers(
             channelz_pb2.GetServersRequest(start_server_id=0))
         self.assertEqual(len(resp.server), 1)
 
-    @unittest.skip('Servers in core are not guaranteed to be destroyed ' \
-                   'immediately when the reference goes out of scope, so ' \
-                   'servers from multiple test cases are not hermetic. ' \
-                   'TODO(https://github.com/grpc/grpc/issues/17258)')
     def test_get_one_server(self):
         self._pairs = _generate_channel_server_pairs(1)
         gss_resp = self._channelz_stub.GetServers(
@@ -303,10 +291,6 @@ class ChannelzServicerTest(unittest.TestCase):
         self.assertEqual(gss_resp.server[0].ref.server_id,
                          gs_resp.server.ref.server_id)
 
-    @unittest.skip('Servers in core are not guaranteed to be destroyed ' \
-                   'immediately when the reference goes out of scope, so ' \
-                   'servers from multiple test cases are not hermetic. ' \
-                   'TODO(https://github.com/grpc/grpc/issues/17258)')
     def test_server_call(self):
         self._pairs = _generate_channel_server_pairs(1)
         k_success = 23
@@ -401,10 +385,6 @@ class ChannelzServicerTest(unittest.TestCase):
         self.assertEqual(gs_resp.socket.data.messages_received,
                          test_constants.STREAM_LENGTH)
 
-    @unittest.skip('Servers in core are not guaranteed to be destroyed ' \
-                   'immediately when the reference goes out of scope, so ' \
-                   'servers from multiple test cases are not hermetic. ' \
-                   'TODO(https://github.com/grpc/grpc/issues/17258)')
     def test_server_sockets(self):
         self._pairs = _generate_channel_server_pairs(1)
         self._send_successful_unary_unary(0)
@@ -423,10 +403,6 @@ class ChannelzServicerTest(unittest.TestCase):
         # If the RPC call failed, it will raise a grpc.RpcError
         # So, if there is no exception raised, considered pass
 
-    @unittest.skip('Servers in core are not guaranteed to be destroyed ' \
-                   'immediately when the reference goes out of scope, so ' \
-                   'servers from multiple test cases are not hermetic. ' \
-                   'TODO(https://github.com/grpc/grpc/issues/17258)')
     def test_server_listen_sockets(self):
         self._pairs = _generate_channel_server_pairs(1)