You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by jx...@apache.org on 2020/04/24 23:47:07 UTC

[helix] branch master updated: Fix failed tests in helix-rest (#966)

This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/helix.git


The following commit(s) were added to refs/heads/master by this push:
     new 2b40e1a  Fix failed tests in helix-rest (#966)
2b40e1a is described below

commit 2b40e1a5d9e02398691a97cf8dafbcc0e3443abe
Author: Huizhi Lu <ih...@gmail.com>
AuthorDate: Fri Apr 24 16:46:58 2020 -0700

    Fix failed tests in helix-rest (#966)
    
    In tests: TestZkRoutingDataWriter and TestZkRoutingDataReader, the zkClient is trying to read/write ZNRecords, however, zkClient's serializer is not a ZNRecordSerializer but a BasicZkSerializer. So when read/write a ZNRecord, a ZkMarshallingError is thrown and causes the tests failed.
---
 .../accessor/TestZkRoutingDataReader.java          | 32 ++++++++++------------
 .../accessor/TestZkRoutingDataWriter.java          | 30 ++++++++++----------
 2 files changed, 28 insertions(+), 34 deletions(-)

diff --git a/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/accessor/TestZkRoutingDataReader.java b/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/accessor/TestZkRoutingDataReader.java
index 4c95445..d9b521b 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/accessor/TestZkRoutingDataReader.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/accessor/TestZkRoutingDataReader.java
@@ -29,7 +29,6 @@ import org.apache.helix.msdcommon.constant.MetadataStoreRoutingConstants;
 import org.apache.helix.msdcommon.exception.InvalidRoutingDataException;
 import org.apache.helix.rest.server.AbstractTestClass;
 import org.apache.helix.zookeeper.datamodel.ZNRecord;
-import org.apache.helix.zookeeper.zkclient.ZkClient;
 import org.testng.Assert;
 import org.testng.annotations.AfterClass;
 import org.testng.annotations.AfterMethod;
@@ -39,11 +38,9 @@ import org.testng.annotations.Test;
 
 public class TestZkRoutingDataReader extends AbstractTestClass {
   private MetadataStoreRoutingDataReader _zkRoutingDataReader;
-  private ZkClient _zkClient;
 
   @BeforeClass
   public void beforeClass() throws Exception {
-    _zkClient = ZK_SERVER_MAP.get(_zkAddrTestNS).getZkClient();
     _zkRoutingDataReader = new ZkRoutingDataReader(TEST_NAMESPACE, _zkAddrTestNS, null);
     clearRoutingDataPath();
   }
@@ -76,14 +73,13 @@ public class TestZkRoutingDataReader extends AbstractTestClass {
         .setListField(MetadataStoreRoutingConstants.ZNRECORD_LIST_FIELD_KEY, testShardingKeys2);
 
     // Add both nodes as children nodes to ZkRoutingDataReader.ROUTING_DATA_PATH
-    _zkClient
-        .createPersistent(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/testRealmAddress1");
-    _zkClient.writeData(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/testRealmAddress1",
-        testZnRecord1);
-    _zkClient
-        .createPersistent(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/testRealmAddress2");
-    _zkClient.writeData(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/testRealmAddress2",
-        testZnRecord2);
+    _gZkClientTestNS
+        .createPersistent(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/testRealmAddress1",
+            testZnRecord1);
+
+    _gZkClientTestNS
+        .createPersistent(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/testRealmAddress2",
+            testZnRecord2);
 
     try {
       Map<String, List<String>> routingData = _zkRoutingDataReader.getRoutingData();
@@ -110,10 +106,10 @@ public class TestZkRoutingDataReader extends AbstractTestClass {
     ZNRecord testZnRecord1 = new ZNRecord("testZnRecord1");
     testZnRecord1.setListField(MetadataStoreRoutingConstants.ZNRECORD_LIST_FIELD_KEY,
         Collections.emptyList());
-    _zkClient
-        .createPersistent(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/testRealmAddress1");
-    _zkClient.writeData(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/testRealmAddress1",
-        testZnRecord1);
+    _gZkClientTestNS
+        .createPersistent(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/testRealmAddress1",
+            testZnRecord1);
+
     try {
       Map<String, List<String>> routingData = _zkRoutingDataReader.getRoutingData();
       Assert.assertEquals(routingData.size(), 1);
@@ -125,12 +121,12 @@ public class TestZkRoutingDataReader extends AbstractTestClass {
 
   private void clearRoutingDataPath() throws Exception {
     Assert.assertTrue(TestHelper.verify(() -> {
-      for (String zkRealm : _zkClient
+      for (String zkRealm : _gZkClientTestNS
           .getChildren(MetadataStoreRoutingConstants.ROUTING_DATA_PATH)) {
-        _zkClient.delete(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + zkRealm);
+        _gZkClientTestNS.delete(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + zkRealm);
       }
 
-      return _zkClient.getChildren(MetadataStoreRoutingConstants.ROUTING_DATA_PATH).isEmpty();
+      return _gZkClientTestNS.getChildren(MetadataStoreRoutingConstants.ROUTING_DATA_PATH).isEmpty();
     }, TestHelper.WAIT_DURATION), "Routing data path should be deleted after the tests.");
   }
 }
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/accessor/TestZkRoutingDataWriter.java b/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/accessor/TestZkRoutingDataWriter.java
index 217b13e..9cab562 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/accessor/TestZkRoutingDataWriter.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/accessor/TestZkRoutingDataWriter.java
@@ -30,7 +30,6 @@ import org.apache.helix.msdcommon.constant.MetadataStoreRoutingConstants;
 import org.apache.helix.rest.common.HttpConstants;
 import org.apache.helix.rest.server.AbstractTestClass;
 import org.apache.helix.zookeeper.datamodel.ZNRecord;
-import org.apache.helix.zookeeper.zkclient.ZkClient;
 import org.apache.http.client.methods.HttpUriRequest;
 import org.testng.Assert;
 import org.testng.annotations.AfterClass;
@@ -43,7 +42,6 @@ public class TestZkRoutingDataWriter extends AbstractTestClass {
   private static final String DUMMY_SHARDING_KEY = "/DUMMY/SHARDING/KEY";
 
   private MetadataStoreRoutingDataWriter _zkRoutingDataWriter;
-  private ZkClient _zkClient;
 
   // MockWriter is used for testing request forwarding features in non-leader situations
   class MockWriter extends ZkRoutingDataWriter {
@@ -63,7 +61,6 @@ public class TestZkRoutingDataWriter extends AbstractTestClass {
 
   @BeforeClass
   public void beforeClass() throws Exception {
-    _zkClient = ZK_SERVER_MAP.get(_zkAddrTestNS).getZkClient();
     System.setProperty(MetadataStoreRoutingConstants.MSDS_SERVER_HOSTNAME_KEY,
         getBaseUri().getHost());
     System.setProperty(MetadataStoreRoutingConstants.MSDS_SERVER_PORT_KEY,
@@ -83,23 +80,23 @@ public class TestZkRoutingDataWriter extends AbstractTestClass {
   @Test
   public void testAddMetadataStoreRealm() {
     _zkRoutingDataWriter.addMetadataStoreRealm(DUMMY_REALM);
-    ZNRecord znRecord =
-        _zkClient.readData(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + DUMMY_REALM);
+    ZNRecord znRecord = _gZkClientTestNS
+        .readData(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + DUMMY_REALM);
     Assert.assertNotNull(znRecord);
   }
 
   @Test(dependsOnMethods = "testAddMetadataStoreRealm")
   public void testDeleteMetadataStoreRealm() {
     _zkRoutingDataWriter.deleteMetadataStoreRealm(DUMMY_REALM);
-    Assert.assertFalse(
-        _zkClient.exists(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + DUMMY_REALM));
+    Assert.assertFalse(_gZkClientTestNS
+        .exists(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + DUMMY_REALM));
   }
 
   @Test(dependsOnMethods = "testDeleteMetadataStoreRealm")
   public void testAddShardingKey() {
     _zkRoutingDataWriter.addShardingKey(DUMMY_REALM, DUMMY_SHARDING_KEY);
-    ZNRecord znRecord =
-        _zkClient.readData(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + DUMMY_REALM);
+    ZNRecord znRecord = _gZkClientTestNS
+        .readData(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + DUMMY_REALM);
     Assert.assertNotNull(znRecord);
     Assert.assertTrue(znRecord.getListField(MetadataStoreRoutingConstants.ZNRECORD_LIST_FIELD_KEY)
         .contains(DUMMY_SHARDING_KEY));
@@ -108,8 +105,8 @@ public class TestZkRoutingDataWriter extends AbstractTestClass {
   @Test(dependsOnMethods = "testAddShardingKey")
   public void testDeleteShardingKey() {
     _zkRoutingDataWriter.deleteShardingKey(DUMMY_REALM, DUMMY_SHARDING_KEY);
-    ZNRecord znRecord =
-        _zkClient.readData(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + DUMMY_REALM);
+    ZNRecord znRecord = _gZkClientTestNS
+        .readData(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + DUMMY_REALM);
     Assert.assertNotNull(znRecord);
     Assert.assertFalse(znRecord.getListField(MetadataStoreRoutingConstants.ZNRECORD_LIST_FIELD_KEY)
         .contains(DUMMY_SHARDING_KEY));
@@ -120,8 +117,8 @@ public class TestZkRoutingDataWriter extends AbstractTestClass {
     Map<String, List<String>> testRoutingDataMap =
         ImmutableMap.of(DUMMY_REALM, Collections.singletonList(DUMMY_SHARDING_KEY));
     _zkRoutingDataWriter.setRoutingData(testRoutingDataMap);
-    ZNRecord znRecord =
-        _zkClient.readData(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + DUMMY_REALM);
+    ZNRecord znRecord = _gZkClientTestNS
+        .readData(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + DUMMY_REALM);
     Assert.assertNotNull(znRecord);
     Assert.assertEquals(
         znRecord.getListField(MetadataStoreRoutingConstants.ZNRECORD_LIST_FIELD_KEY).size(), 1);
@@ -212,12 +209,13 @@ public class TestZkRoutingDataWriter extends AbstractTestClass {
 
   private void clearRoutingDataPath() throws Exception {
     Assert.assertTrue(TestHelper.verify(() -> {
-      for (String zkRealm : _zkClient
+      for (String zkRealm : _gZkClientTestNS
           .getChildren(MetadataStoreRoutingConstants.ROUTING_DATA_PATH)) {
-        _zkClient.delete(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + zkRealm);
+        _gZkClientTestNS.delete(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + zkRealm);
       }
 
-      return _zkClient.getChildren(MetadataStoreRoutingConstants.ROUTING_DATA_PATH).isEmpty();
+      return _gZkClientTestNS.getChildren(MetadataStoreRoutingConstants.ROUTING_DATA_PATH)
+          .isEmpty();
     }, TestHelper.WAIT_DURATION), "Routing data path should be deleted after the tests.");
   }
 }