[GitHub] [helix] Marcosrico commented on a diff in pull request #2290: Code cleanup and FederatedZkClient Multi test.

[GitBox <gi...@apache.org>]

Marcosrico commented on code in PR #2290:
URL: https://github.com/apache/helix/pull/2290#discussion_r1029484374


##########
helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkConnectionConfig.java:
##########
@@ -0,0 +1,190 @@
+package org.apache.helix.integration.multizk;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.helix.HelixAdmin;
+import org.apache.helix.HelixManagerProperty;
+import org.apache.helix.TestHelper;
+import org.apache.helix.integration.manager.ClusterControllerManager;
+import org.apache.helix.integration.manager.MockParticipantManager;
+import org.apache.helix.integration.task.MockTask;
+import org.apache.helix.manager.zk.ZKHelixAdmin;
+import org.apache.helix.model.InstanceConfig;
+import org.apache.helix.participant.StateMachineEngine;
+import org.apache.helix.task.TaskFactory;
+import org.apache.helix.task.TaskStateModelFactory;
+import org.apache.helix.tools.ClusterSetup;
+import org.apache.helix.zookeeper.api.client.RealmAwareZkClient;
+import org.apache.helix.zookeeper.constant.RoutingDataReaderType;
+import org.apache.helix.zookeeper.datamodel.serializer.ZNRecordSerializer;
+import org.apache.helix.zookeeper.impl.client.FederatedZkClient;
+import org.apache.helix.zookeeper.routing.RoutingDataManager;
+import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+
+/**
+ * Some Helix user do not have multizk routing configs in jvm system config but pass
+ * in through RealAwareZkConnectionConfig.
+ * This test class will not set jvm routing zk config and test Helix functionality.
+ * Tests were similar to TestMultiZkHelixJavaApis but without "MSDS_SERVER_ENDPOINT_KEY"
+ * in system property
+ */
+public class TestMultiZkConnectionConfig extends TestMultiZkHelixJavaApis {
+
+  @BeforeClass
+  public void beforeClass() throws Exception {
+    super.beforeClass();
+    // Routing data may be set by other tests using the same endpoint; reset() for good measure
+    RoutingDataManager.getInstance().reset();
+    // Create a FederatedZkClient for admin work
+
+    try {
+      _zkClient =
+              new FederatedZkClient(new RealmAwareZkClient.RealmAwareZkConnectionConfig.Builder()
+                      .setRoutingDataSourceEndpoint(_msdsEndpoint + "," + ZK_PREFIX + ZK_START_PORT)
+                      .setRoutingDataSourceType(RoutingDataReaderType.HTTP_ZK_FALLBACK.name()).build(),
+                      new RealmAwareZkClient.RealmAwareZkClientConfig());
+      _zkClient.setZkSerializer(new ZNRecordSerializer());
+    } catch (Exception ex) {
+      for (StackTraceElement elm : ex.getStackTrace()) {
+        System.out.println(elm);
+      }
+    }
+    System.out.println("end start");
+  }
+
+  @Override
+  public void setupCluster() {
+    // Create two ClusterSetups using two different constructors
+    // Note: ZK Address here could be anything because multiZk mode is on (it will be ignored)
+    _clusterSetupZkAddr = new ClusterSetup(_zkClient);
+    _clusterSetupBuilder = new ClusterSetup.Builder().setRealmAwareZkConnectionConfig(
+                    new RealmAwareZkClient.RealmAwareZkConnectionConfig.Builder()
+                            .setRoutingDataSourceEndpoint(_msdsEndpoint + "," + ZK_PREFIX + ZK_START_PORT)
+                            .setRoutingDataSourceType(RoutingDataReaderType.HTTP_ZK_FALLBACK.name()).build())
+            .build();
+  }
+
+
+  /**
+   * Test Helix Participant creation and addition.
+   * Helix Java APIs tested in this method are:
+   * ZkHelixAdmin and ZKHelixManager (mock participant/controller)
+   */
+  @Test(dependsOnMethods = "testCreateClusters")
+  public void testCreateParticipants() throws Exception {
+    // Create two ClusterSetups using two different constructors
+    // Note: ZK Address here could be anything because multiZk mode is on (it will be ignored)
+    //HelixAdmin helixAdminZkAddr = new ZKHelixAdmin(ZK_SERVER_MAP.keySet().iterator().next());

Review Comment:
   There is no reason for these comments, thank you for pointing it out! Both test cases in TestMultiZkConnectionConfig and TestMultiZkHelixJavaApis were very similar (the reason for this PR). I'm guessing they were copy pasted and commented out for a specific purpose and went under my radar when creating the the testbase.



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org