kaisun2000 commented on a change in pull request #1534:
URL: https://github.com/apache/helix/pull/1534#discussion_r525486813



##########
File path: 
helix-core/src/test/java/org/apache/helix/participant/TestDistControllerElection.java
##########
@@ -197,35 +218,42 @@ public void testCompeteLeadership() throws Exception {
 
     // Create controller leaders
     final Map<String, ZKHelixManager> managerList = new HashMap<>();
+    final List<GenericHelixController> controllers = new ArrayList<>();
     for (int i = 0; i < managerCount; i++) {
       String controllerName = "controller_" + i;
       ZKHelixManager manager =
           new ZKHelixManager(clusterName, controllerName, 
InstanceType.CONTROLLER, ZK_ADDR);
       GenericHelixController controller0 = new GenericHelixController();
       DistributedLeaderElection election =
           new DistributedLeaderElection(manager, controller0, 
Collections.EMPTY_LIST);
+      controllers.add(controller0);
       manager.connect();
       managerList.put(manager.getInstanceName(), manager);
     }
 
-    // Remove leader manager one by one, and verify if the leader node exists
-    while(!managerList.isEmpty()) {
-      // Ensure a controller successfully acquired leadership.
-      Assert.assertTrue(TestHelper.verify(new TestHelper.Verifier() {
-        @Override
-        public boolean verify() {
-          LiveInstance liveInstance = 
accessor.getProperty(keyBuilder.controllerLeader());
-          if (liveInstance != null) {
-            // disconnect the current leader manager
-            managerList.remove(liveInstance.getInstanceName()).disconnect();
-            return true;
-          } else {
-            return false;
+    try {
+      // Remove leader manager one by one, and verify if the leader node exists
+      while (!managerList.isEmpty()) {
+        // Ensure a controller successfully acquired leadership.
+        Assert.assertTrue(TestHelper.verify(new TestHelper.Verifier() {
+          @Override
+          public boolean verify() {
+            LiveInstance liveInstance = 
accessor.getProperty(keyBuilder.controllerLeader());
+            if (liveInstance != null) {
+              // disconnect the current leader manager
+              managerList.remove(liveInstance.getInstanceName()).disconnect();
+              return true;
+            } else {
+              return false;
+            }
           }
-        }
-      }, 1000));
+        }, 1000));
+      }
+    } finally {
+      for (GenericHelixController controller : controllers) {
+        controller.shutdown();

Review comment:
       good point. added.

##########
File path: 
helix-core/src/test/java/org/apache/helix/participant/TestDistControllerElection.java
##########
@@ -72,36 +73,45 @@ public void testController() throws Exception {
     DistributedLeaderElection election =
         new DistributedLeaderElection(manager, controller0, timerTasks);
     NotificationContext context = new NotificationContext(manager);
-    context.setType(NotificationContext.Type.INIT);
-    election.onControllerChange(context);
-
-    // path = PropertyPathConfig.getPath(PropertyType.LEADER, clusterName);
-    // ZNRecord leaderRecord = _gZkClient.<ZNRecord> readData(path);
-    LiveInstance liveInstance = 
accessor.getProperty(keyBuilder.controllerLeader());
-    AssertJUnit.assertEquals(controllerName, liveInstance.getInstanceName());
-    // AssertJUnit.assertNotNull(election.getController());
-    // AssertJUnit.assertNull(election.getLeader());
+    try {
+      context.setType(NotificationContext.Type.INIT);
+      election.onControllerChange(context);
+
+      // path = PropertyPathConfig.getPath(PropertyType.LEADER, clusterName);
+      // ZNRecord leaderRecord = _gZkClient.<ZNRecord> readData(path);
+      LiveInstance liveInstance = 
accessor.getProperty(keyBuilder.controllerLeader());
+      AssertJUnit.assertEquals(controllerName, liveInstance.getInstanceName());
+      // AssertJUnit.assertNotNull(election.getController());
+      // AssertJUnit.assertNull(election.getLeader());
+    } finally {
+      manager.disconnect();
+      controller0.shutdown();

Review comment:
       changed.

##########
File path: 
helix-core/src/test/java/org/apache/helix/participant/TestDistControllerElection.java
##########
@@ -72,36 +73,45 @@ public void testController() throws Exception {
     DistributedLeaderElection election =
         new DistributedLeaderElection(manager, controller0, timerTasks);
     NotificationContext context = new NotificationContext(manager);
-    context.setType(NotificationContext.Type.INIT);
-    election.onControllerChange(context);
-
-    // path = PropertyPathConfig.getPath(PropertyType.LEADER, clusterName);
-    // ZNRecord leaderRecord = _gZkClient.<ZNRecord> readData(path);
-    LiveInstance liveInstance = 
accessor.getProperty(keyBuilder.controllerLeader());
-    AssertJUnit.assertEquals(controllerName, liveInstance.getInstanceName());
-    // AssertJUnit.assertNotNull(election.getController());
-    // AssertJUnit.assertNull(election.getLeader());
+    try {
+      context.setType(NotificationContext.Type.INIT);
+      election.onControllerChange(context);
+
+      // path = PropertyPathConfig.getPath(PropertyType.LEADER, clusterName);
+      // ZNRecord leaderRecord = _gZkClient.<ZNRecord> readData(path);
+      LiveInstance liveInstance = 
accessor.getProperty(keyBuilder.controllerLeader());
+      AssertJUnit.assertEquals(controllerName, liveInstance.getInstanceName());
+      // AssertJUnit.assertNotNull(election.getController());
+      // AssertJUnit.assertNull(election.getLeader());
+    } finally {
+      manager.disconnect();
+      controller0.shutdown();
+    }
 
     manager =
         new MockZKHelixManager(clusterName, "controller_1", 
InstanceType.CONTROLLER, _gZkClient);
     GenericHelixController controller1 = new GenericHelixController();
     election = new DistributedLeaderElection(manager, controller1, timerTasks);
     context = new NotificationContext(manager);
     context.setType(NotificationContext.Type.INIT);
-    election.onControllerChange(context);
-    // leaderRecord = _gZkClient.<ZNRecord> readData(path);
-    liveInstance = accessor.getProperty(keyBuilder.controllerLeader());
-    AssertJUnit.assertEquals(controllerName, liveInstance.getInstanceName());
-    // AssertJUnit.assertNull(election.getController());
-    // AssertJUnit.assertNull(election.getLeader());
-
-    accessor.removeProperty(keyBuilder.controllerLeader());
-    TestHelper.dropCluster(clusterName, _gZkClient);
+    try {
+      election.onControllerChange(context);
+      // leaderRecord = _gZkClient.<ZNRecord> readData(path);
+      LiveInstance liveInstance = 
accessor.getProperty(keyBuilder.controllerLeader());
+      AssertJUnit.assertEquals(controllerName, liveInstance.getInstanceName());
+      // AssertJUnit.assertNull(election.getController());
+      // AssertJUnit.assertNull(election.getLeader());
+    } finally {
+      manager.disconnect();
+      controller1.shutdown();

Review comment:
       changed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to