You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by jc...@apache.org on 2019/01/29 01:28:11 UTC

[geode] branch feature/GEODE-6273 updated: Test improvements

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

jchen21 pushed a commit to branch feature/GEODE-6273
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/feature/GEODE-6273 by this push:
     new bc9b835  Test improvements
bc9b835 is described below

commit bc9b8356bd2eb14e70f5e768bf62a58178172d76
Author: Scott Jewell <sj...@pivotal.io>
AuthorDate: Mon Jan 28 17:27:09 2019 -0800

    Test improvements
    
    Co-authored-by: Scott Jewell <sj...@pivotal.io>
    Co-authored-by: Jianxia Chen <jc...@pivotal.io>
---
 .../cli/CreateMappingCommandDUnitTest.java         | 202 +++++++++++----------
 .../cli/DescribeMappingCommandDUnitTest.java       |  27 +--
 .../jdbc/internal/cli/CreateMappingCommand.java    |  21 ++-
 .../jdbc/internal/RegionMappingTest.java           |   1 -
 .../internal/cli/CreateMappingCommandTest.java     |   1 -
 .../internal/cli/CreateMappingFunctionTest.java    |   1 -
 .../jdbc/internal/cli/ListMappingCommandTest.java  |   1 -
 7 files changed, 129 insertions(+), 125 deletions(-)

diff --git a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandDUnitTest.java b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandDUnitTest.java
index 014809c..1f47c2d 100644
--- a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandDUnitTest.java
+++ b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandDUnitTest.java
@@ -93,15 +93,6 @@ public class CreateMappingCommandDUnitTest {
 
   }
 
-  public void cleanUp() throws Exception {
-    startupRule.stop(0);
-    startupRule.stop(1);
-    startupRule.stop(2);
-    startupRule.stop(3);
-    startupRule.stop(4);
-    gfsh.disconnect();
-  }
-
   private void setupReplicate(String regionName) {
     setupReplicate(regionName, false);
   }
@@ -114,7 +105,6 @@ public class CreateMappingCommandDUnitTest {
         .statusIsSuccess();
   }
 
-  // TODO: might need a parameter for server group names
   private void setupGroupReplicate(String regionName, String groupNames) {
     gfsh.executeAndAssertThat(
         "create region --name=" + regionName + " --type=REPLICATE --groups=" + groupNames)
@@ -239,12 +229,12 @@ public class CreateMappingCommandDUnitTest {
     // TEST_GROUP1 only contains server2 and server 4
     server2.invoke(() -> {
       RegionMapping mapping = getRegionMappingFromService(regionName);
-      assertValidMapping(mapping, regionName, false, false);
+      assertValidMappingOnServer(mapping, regionName, false, false);
     });
 
     server4.invoke(() -> {
       RegionMapping mapping = getRegionMappingFromService(regionName);
-      assertValidMapping(mapping, regionName, false, false);
+      assertValidMappingOnServer(mapping, regionName, false, false);
     });
 
     server1.invoke(() -> {
@@ -259,20 +249,13 @@ public class CreateMappingCommandDUnitTest {
 
     locator.invoke(() -> {
       RegionMapping regionMapping = getRegionMappingFromClusterConfig(regionName, TEST_GROUP1);
-      assertThat(regionMapping.getDataSourceName()).isEqualTo("connection");
-      assertThat(regionMapping.getTableName()).isEqualTo("myTable");
-      assertThat(regionMapping.getPdxName()).isEqualTo("myPdxClass");
-      assertThat(regionMapping.getIds()).isEqualTo("myId");
-      assertThat(regionMapping.getCatalog()).isEqualTo("myCatalog");
-      assertThat(regionMapping.getSchema()).isEqualTo("mySchema");
-      validateRegionAlteredInClusterConfig(regionName, TEST_GROUP1, false);
-      validateAsyncEventQueueCreatedInClusterConfig(regionName, TEST_GROUP1, false);
+      assertValidMappingOnLocator(regionMapping, regionName, TEST_GROUP1, false, false);
     });
   }
 
   @Test
   @Parameters({GROUP2_REGION, "/" + GROUP2_REGION})
-  public void createMappingParitionedUpdatesServiceAndClusterConfigForServerGroup(
+  public void createMappingPartitionedUpdatesServiceAndClusterConfigForServerGroup(
       String regionName) {
     setupGroupPartition(regionName, TEST_GROUP2);
     CommandStringBuilder csb = new CommandStringBuilder(CREATE_MAPPING);
@@ -290,12 +273,12 @@ public class CreateMappingCommandDUnitTest {
     // TEST_GROUP2 only contains server3 and server4
     server3.invoke(() -> {
       RegionMapping mapping = getRegionMappingFromService(regionName);
-      assertValidMapping(mapping, regionName, false, true);
+      assertValidMappingOnServer(mapping, regionName, false, true);
     });
 
     server4.invoke(() -> {
       RegionMapping mapping = getRegionMappingFromService(regionName);
-      assertValidMapping(mapping, regionName, false, true);
+      assertValidMappingOnServer(mapping, regionName, false, true);
     });
 
     server1.invoke(() -> {
@@ -310,14 +293,7 @@ public class CreateMappingCommandDUnitTest {
 
     locator.invoke(() -> {
       RegionMapping regionMapping = getRegionMappingFromClusterConfig(regionName, TEST_GROUP2);
-      assertThat(regionMapping.getDataSourceName()).isEqualTo("connection");
-      assertThat(regionMapping.getTableName()).isEqualTo("myTable");
-      assertThat(regionMapping.getPdxName()).isEqualTo("myPdxClass");
-      assertThat(regionMapping.getIds()).isEqualTo("myId");
-      assertThat(regionMapping.getCatalog()).isEqualTo("myCatalog");
-      assertThat(regionMapping.getSchema()).isEqualTo("mySchema");
-      validateRegionAlteredInClusterConfig(regionName, TEST_GROUP2, false);
-      validateAsyncEventQueueCreatedInClusterConfig(regionName, TEST_GROUP2, true);
+      assertValidMappingOnLocator(regionMapping, regionName, TEST_GROUP2, false, true);
     });
   }
 
@@ -341,17 +317,17 @@ public class CreateMappingCommandDUnitTest {
     // TEST_GROUP1 and TEST_GROUP2 only contains server 2, server 3, and server 4
     server2.invoke(() -> {
       RegionMapping mapping = getRegionMappingFromService(regionName);
-      assertValidMapping(mapping, regionName, false, false);
+      assertValidMappingOnServer(mapping, regionName, false, false);
     });
 
     server3.invoke(() -> {
       RegionMapping mapping = getRegionMappingFromService(regionName);
-      assertValidMapping(mapping, regionName, false, false);
+      assertValidMappingOnServer(mapping, regionName, false, false);
     });
 
     server4.invoke(() -> {
       RegionMapping mapping = getRegionMappingFromService(regionName);
-      assertValidMapping(mapping, regionName, false, false);
+      assertValidMappingOnServer(mapping, regionName, false, false);
     });
 
     server1.invoke(() -> {
@@ -361,14 +337,7 @@ public class CreateMappingCommandDUnitTest {
 
     locator.invoke(() -> {
       RegionMapping regionMapping = getRegionMappingFromClusterConfig(regionName, TEST_GROUP1);
-      assertThat(regionMapping.getDataSourceName()).isEqualTo("connection");
-      assertThat(regionMapping.getTableName()).isEqualTo("myTable");
-      assertThat(regionMapping.getPdxName()).isEqualTo("myPdxClass");
-      assertThat(regionMapping.getIds()).isEqualTo("myId");
-      assertThat(regionMapping.getCatalog()).isEqualTo("myCatalog");
-      assertThat(regionMapping.getSchema()).isEqualTo("mySchema");
-      validateRegionAlteredInClusterConfig(regionName, TEST_GROUP1, false);
-      validateAsyncEventQueueCreatedInClusterConfig(regionName, TEST_GROUP1, false);
+      assertValidMappingOnLocator(regionMapping, regionName, TEST_GROUP1, false, false);
     });
   }
 
@@ -392,17 +361,17 @@ public class CreateMappingCommandDUnitTest {
     // TEST_GROUP1 and TEST_GROUP2 only contains server 2, server 3, and server 4
     server2.invoke(() -> {
       RegionMapping mapping = getRegionMappingFromService(regionName);
-      assertValidMapping(mapping, regionName, false, true);
+      assertValidMappingOnServer(mapping, regionName, false, true);
     });
 
     server3.invoke(() -> {
       RegionMapping mapping = getRegionMappingFromService(regionName);
-      assertValidMapping(mapping, regionName, false, true);
+      assertValidMappingOnServer(mapping, regionName, false, true);
     });
 
     server4.invoke(() -> {
       RegionMapping mapping = getRegionMappingFromService(regionName);
-      assertValidMapping(mapping, regionName, false, true);
+      assertValidMappingOnServer(mapping, regionName, false, true);
     });
 
     server1.invoke(() -> {
@@ -412,27 +381,44 @@ public class CreateMappingCommandDUnitTest {
 
     locator.invoke(() -> {
       RegionMapping regionMapping = getRegionMappingFromClusterConfig(regionName, TEST_GROUP1);
-      assertThat(regionMapping.getDataSourceName()).isEqualTo("connection");
-      assertThat(regionMapping.getTableName()).isEqualTo("myTable");
-      assertThat(regionMapping.getPdxName()).isEqualTo("myPdxClass");
-      assertThat(regionMapping.getIds()).isEqualTo("myId");
-      assertThat(regionMapping.getCatalog()).isEqualTo("myCatalog");
-      assertThat(regionMapping.getSchema()).isEqualTo("mySchema");
-      validateRegionAlteredInClusterConfig(regionName, TEST_GROUP1, false);
-      validateAsyncEventQueueCreatedInClusterConfig(regionName, TEST_GROUP1, true);
+      assertValidMappingOnLocator(regionMapping, regionName, TEST_GROUP1, false, true);
     });
   }
 
-  private static void assertValidMapping(RegionMapping mapping, String regionName,
+  private static void assertValidMappingOnServer(RegionMapping mapping, String regionName,
+      boolean synchronous, boolean isParallel) {
+    assertValidMapping(mapping);
+    validateRegionAlteredOnServer(regionName, synchronous);
+    if (!synchronous) {
+      validateAsyncEventQueueCreatedOnServer(regionName, isParallel);
+    }
+  }
+
+  private static void assertValidMappingOnLocator(RegionMapping mapping, String regionName,
+      String groups,
       boolean synchronous, boolean isParallel) {
+    assertValidMapping(mapping);
+    validateRegionAlteredInClusterConfig(regionName, groups, synchronous);
+    if (!synchronous) {
+      validateAsyncEventQueueCreatedInClusterConfig(regionName, groups, isParallel);
+    }
+  }
+
+  private static void assertValidMapping(RegionMapping mapping) {
     assertThat(mapping.getDataSourceName()).isEqualTo("connection");
     assertThat(mapping.getTableName()).isEqualTo("myTable");
     assertThat(mapping.getPdxName()).isEqualTo("myPdxClass");
     assertThat(mapping.getIds()).isEqualTo("myId");
     assertThat(mapping.getCatalog()).isEqualTo("myCatalog");
     assertThat(mapping.getSchema()).isEqualTo("mySchema");
-    validateRegionAlteredOnServer(regionName, synchronous);
-    validateAsyncEventQueueCreatedOnServer(regionName, isParallel);
+  }
+
+  private static void assertValidMappingWithoutIds(RegionMapping mapping) {
+    assertThat(mapping.getDataSourceName()).isEqualTo("connection");
+    assertThat(mapping.getTableName()).isEqualTo("myTable");
+    assertThat(mapping.getPdxName()).isEqualTo("myPdxClass");
+    assertThat(mapping.getCatalog()).isEqualTo("myCatalog");
+    assertThat(mapping.getSchema()).isEqualTo("mySchema");
   }
 
   @Test
@@ -452,30 +438,22 @@ public class CreateMappingCommandDUnitTest {
 
     server1.invoke(() -> {
       RegionMapping mapping = getRegionMappingFromService(regionName);
-      assertValidMapping(mapping, regionName, false, false);
+      assertValidMappingOnServer(mapping, regionName, false, false);
     });
 
     // without specifying 'group/groups', the region and regionmapping will be created on all
     // servers
     server2.invoke(() -> {
       RegionMapping mapping = getRegionMappingFromService(regionName);
-      assertValidMapping(mapping, regionName, false, false);
+      assertValidMappingOnServer(mapping, regionName, false, false);
     });
 
     locator.invoke(() -> {
       RegionMapping regionMapping = getRegionMappingFromClusterConfig(regionName, null);
-      assertThat(regionMapping.getDataSourceName()).isEqualTo("connection");
-      assertThat(regionMapping.getTableName()).isEqualTo("myTable");
-      assertThat(regionMapping.getPdxName()).isEqualTo("myPdxClass");
-      assertThat(regionMapping.getIds()).isEqualTo("myId");
-      assertThat(regionMapping.getCatalog()).isEqualTo("myCatalog");
-      assertThat(regionMapping.getSchema()).isEqualTo("mySchema");
-      validateRegionAlteredInClusterConfig(regionName, null, false);
-      validateAsyncEventQueueCreatedInClusterConfig(regionName, null, false);
+      assertValidMappingOnLocator(regionMapping, regionName, null, false, false);
     });
   }
 
-  // TODO: need to have createSynchronousMappingUpdatesServiceAndClusterConfigForServerGroup
   @Test
   @Parameters({TEST_REGION, "/" + TEST_REGION})
   public void createSynchronousMappingUpdatesServiceAndClusterConfig(String regionName) {
@@ -494,41 +472,32 @@ public class CreateMappingCommandDUnitTest {
 
     server1.invoke(() -> {
       RegionMapping mapping = getRegionMappingFromService(regionName);
-      assertThat(mapping.getDataSourceName()).isEqualTo("connection");
-      assertThat(mapping.getTableName()).isEqualTo("myTable");
-      assertThat(mapping.getPdxName()).isEqualTo("myPdxClass");
-      assertThat(mapping.getIds()).isEqualTo("myId");
-      assertThat(mapping.getCatalog()).isEqualTo("myCatalog");
-      assertThat(mapping.getSchema()).isEqualTo("mySchema");
-      validateRegionAlteredOnServer(regionName, true);
+      assertValidMappingOnServer(mapping, regionName, true, false);
     });
 
     // without specifying 'group/groups', the region and regionmapping will be created on all
     // servers
     server2.invoke(() -> {
       RegionMapping mapping = getRegionMappingFromService(regionName);
-      assertThat(mapping.getDataSourceName()).isEqualTo("connection");
-      assertThat(mapping.getTableName()).isEqualTo("myTable");
-      assertThat(mapping.getPdxName()).isEqualTo("myPdxClass");
-      assertThat(mapping.getIds()).isEqualTo("myId");
-      assertThat(mapping.getCatalog()).isEqualTo("myCatalog");
-      assertThat(mapping.getSchema()).isEqualTo("mySchema");
-      validateRegionAlteredOnServer(regionName, true);
+      assertValidMappingOnServer(mapping, regionName, true, false);
+    });
+
+    server3.invoke(() -> {
+      RegionMapping mapping = getRegionMappingFromService(regionName);
+      assertValidMappingOnServer(mapping, regionName, true, false);
+    });
+
+    server4.invoke(() -> {
+      RegionMapping mapping = getRegionMappingFromService(regionName);
+      assertValidMappingOnServer(mapping, regionName, true, false);
     });
 
     locator.invoke(() -> {
-      RegionMapping regionMapping = getRegionMappingFromClusterConfig(regionName, null);
-      assertThat(regionMapping.getDataSourceName()).isEqualTo("connection");
-      assertThat(regionMapping.getTableName()).isEqualTo("myTable");
-      assertThat(regionMapping.getPdxName()).isEqualTo("myPdxClass");
-      assertThat(regionMapping.getIds()).isEqualTo("myId");
-      assertThat(regionMapping.getCatalog()).isEqualTo("myCatalog");
-      assertThat(regionMapping.getSchema()).isEqualTo("mySchema");
-      validateRegionAlteredInClusterConfig(regionName, null, true);
+      RegionMapping mapping = getRegionMappingFromClusterConfig(regionName, null);
+      assertValidMappingOnLocator(mapping, regionName, null, true, false);
     });
   }
 
-  // TODO: need to have a createMappingWithPartitionUpdatesServiceAndClusterConfigForServerGroup
   @Test
   @Parameters({TEST_REGION, "/" + TEST_REGION})
   public void createMappingWithPartitionUpdatesServiceAndClusterConfig(String regionName) {
@@ -561,6 +530,24 @@ public class CreateMappingCommandDUnitTest {
       validateAsyncEventQueueCreatedOnServer(regionName, true);
     });
 
+    server3.invoke(() -> {
+      RegionMapping mapping = getRegionMappingFromService(regionName);
+      assertThat(mapping.getDataSourceName()).isEqualTo("connection");
+      assertThat(mapping.getTableName()).isEqualTo("myTable");
+      assertThat(mapping.getPdxName()).isEqualTo("myPdxClass");
+      validateRegionAlteredOnServer(regionName, false);
+      validateAsyncEventQueueCreatedOnServer(regionName, true);
+    });
+
+    server4.invoke(() -> {
+      RegionMapping mapping = getRegionMappingFromService(regionName);
+      assertThat(mapping.getDataSourceName()).isEqualTo("connection");
+      assertThat(mapping.getTableName()).isEqualTo("myTable");
+      assertThat(mapping.getPdxName()).isEqualTo("myPdxClass");
+      validateRegionAlteredOnServer(regionName, false);
+      validateAsyncEventQueueCreatedOnServer(regionName, true);
+    });
+
     locator.invoke(() -> {
       RegionMapping regionMapping = getRegionMappingFromClusterConfig(regionName, null);
       assertThat(regionMapping.getDataSourceName()).isEqualTo("connection");
@@ -571,7 +558,6 @@ public class CreateMappingCommandDUnitTest {
     });
   }
 
-  // TODO: need to have createMappingWithNoTableForServerGroup
   @Test
   @Parameters({TEST_REGION, "/" + TEST_REGION})
   public void createMappingWithNoTable(String regionName) {
@@ -603,6 +589,24 @@ public class CreateMappingCommandDUnitTest {
       validateAsyncEventQueueCreatedOnServer(regionName, false);
     });
 
+    server3.invoke(() -> {
+      RegionMapping mapping = getRegionMappingFromService(regionName);
+      assertThat(mapping.getDataSourceName()).isEqualTo("connection");
+      assertThat(mapping.getTableName()).isNull();
+      assertThat(mapping.getPdxName()).isEqualTo("myPdxClass");
+      validateRegionAlteredOnServer(regionName, false);
+      validateAsyncEventQueueCreatedOnServer(regionName, false);
+    });
+
+    server4.invoke(() -> {
+      RegionMapping mapping = getRegionMappingFromService(regionName);
+      assertThat(mapping.getDataSourceName()).isEqualTo("connection");
+      assertThat(mapping.getTableName()).isNull();
+      assertThat(mapping.getPdxName()).isEqualTo("myPdxClass");
+      validateRegionAlteredOnServer(regionName, false);
+      validateAsyncEventQueueCreatedOnServer(regionName, false);
+    });
+
     locator.invoke(() -> {
       RegionMapping regionMapping = getRegionMappingFromClusterConfig(regionName, null);
       assertThat(regionMapping.getDataSourceName()).isEqualTo("connection");
@@ -613,7 +617,6 @@ public class CreateMappingCommandDUnitTest {
     });
   }
 
-  // TODO: need to have createExistingRegionMappingFailsForServerGroup
   @Test
   @Parameters({TEST_REGION, "/" + TEST_REGION})
   public void createExistingRegionMappingFails(String regionName) {
@@ -650,6 +653,20 @@ public class CreateMappingCommandDUnitTest {
       assertThat(mapping.getPdxName()).isEqualTo("myPdxClass");
     });
 
+    server3.invoke(() -> {
+      RegionMapping mapping = getRegionMappingFromService(regionName);
+      assertThat(mapping.getDataSourceName()).isEqualTo("connection");
+      assertThat(mapping.getTableName()).isEqualTo("myTable");
+      assertThat(mapping.getPdxName()).isEqualTo("myPdxClass");
+    });
+
+    server4.invoke(() -> {
+      RegionMapping mapping = getRegionMappingFromService(regionName);
+      assertThat(mapping.getDataSourceName()).isEqualTo("connection");
+      assertThat(mapping.getTableName()).isEqualTo("myTable");
+      assertThat(mapping.getPdxName()).isEqualTo("myPdxClass");
+    });
+
     locator.invoke(() -> {
       RegionMapping regionMapping = getRegionMappingFromClusterConfig(regionName, null);
       assertThat(regionMapping.getDataSourceName()).isEqualTo("connection");
@@ -658,7 +675,6 @@ public class CreateMappingCommandDUnitTest {
     });
   }
 
-  // TODO: need to add createMappingWithoutPdxNameFailsForServerGroup
   @Test
   @Parameters({TEST_REGION, "/" + TEST_REGION})
   public void createMappingWithoutPdxNameFails(String regionName) {
diff --git a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeMappingCommandDUnitTest.java b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeMappingCommandDUnitTest.java
index 7cc99d4..92dece5 100644
--- a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeMappingCommandDUnitTest.java
+++ b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeMappingCommandDUnitTest.java
@@ -30,7 +30,6 @@ import java.io.Serializable;
 
 import junitparams.JUnitParamsRunner;
 import junitparams.Parameters;
-import org.junit.After;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
@@ -59,7 +58,7 @@ public class DescribeMappingCommandDUnitTest implements Serializable {
   @Rule
   public SerializableTestName testName = new SerializableTestName();
 
-  private MemberVM locator, server1, server2;
+  private MemberVM locator;
 
   private static String convertRegionPathToName(String regionPath) {
     if (regionPath.startsWith("/")) {
@@ -68,19 +67,11 @@ public class DescribeMappingCommandDUnitTest implements Serializable {
     return regionPath;
   }
 
-  // TODO: need to add test for server1 group
-  @After
-  public void cleanUp() throws Exception {
-    startupRule.stop(0);
-    startupRule.stop(1);
-    gfsh.disconnect();
-  }
-
   @Test
   @Parameters({TEST_REGION, "/" + TEST_REGION})
   public void describesExistingSynchronousMapping(String regionName) throws Exception {
     locator = startupRule.startLocatorVM(0);
-    server1 = startupRule.startServerVM(1, locator.getPort());
+    startupRule.startServerVM(1, locator.getPort());
 
     gfsh.connectAndVerify(locator);
     gfsh.executeAndAssertThat("create region --name=" + regionName + " --type=REPLICATE")
@@ -116,7 +107,7 @@ public class DescribeMappingCommandDUnitTest implements Serializable {
   public void describesExistingSynchronousMappingWithGroups(String regionName) throws Exception {
     String groupName = "group1";
     locator = startupRule.startLocatorVM(0);
-    server1 = startupRule.startServerVM(1, groupName, locator.getPort());
+    startupRule.startServerVM(1, groupName, locator.getPort());
 
     gfsh.connectAndVerify(locator);
     gfsh.executeAndAssertThat(
@@ -153,7 +144,7 @@ public class DescribeMappingCommandDUnitTest implements Serializable {
   @Parameters({TEST_REGION, "/" + TEST_REGION})
   public void describesExistingAsyncMapping(String regionName) throws Exception {
     locator = startupRule.startLocatorVM(0);
-    server1 = startupRule.startServerVM(1, locator.getPort());
+    startupRule.startServerVM(1, locator.getPort());
 
     gfsh.connectAndVerify(locator);
     gfsh.executeAndAssertThat("create region --name=" + regionName + " --type=REPLICATE")
@@ -195,7 +186,7 @@ public class DescribeMappingCommandDUnitTest implements Serializable {
   public void describesExistingAsyncMappingWithGroup(String regionName) throws Exception {
     String groupName = "group1";
     locator = startupRule.startLocatorVM(0);
-    server1 = startupRule.startServerVM(1, groupName, locator.getPort());
+    startupRule.startServerVM(1, groupName, locator.getPort());
 
     gfsh.connectAndVerify(locator);
     gfsh.executeAndAssertThat(
@@ -241,8 +232,8 @@ public class DescribeMappingCommandDUnitTest implements Serializable {
     String groupName1 = "group1";
     String groupName2 = "group2";
     locator = startupRule.startLocatorVM(0);
-    server1 = startupRule.startServerVM(1, groupName1, locator.getPort());
-    server2 = startupRule.startServerVM(2, groupName2, locator.getPort());
+    startupRule.startServerVM(1, groupName1, locator.getPort());
+    startupRule.startServerVM(2, groupName2, locator.getPort());
 
     gfsh.connectAndVerify(locator);
     gfsh.executeAndAssertThat("create region --name=" + regionName + " --type=REPLICATE --group="
@@ -315,7 +306,7 @@ public class DescribeMappingCommandDUnitTest implements Serializable {
   @Test
   public void reportsNoRegionFound() throws Exception {
     locator = startupRule.startLocatorVM(0);
-    server1 = startupRule.startServerVM(1, locator.getPort());
+    startupRule.startServerVM(1, locator.getPort());
     gfsh.connectAndVerify(locator);
     gfsh.executeAndAssertThat("create region --name=" + TEST_REGION + " --type=REPLICATE")
         .statusIsSuccess();
@@ -333,7 +324,7 @@ public class DescribeMappingCommandDUnitTest implements Serializable {
   @Test
   public void reportsRegionButNoMappingFound() throws Exception {
     locator = startupRule.startLocatorVM(0);
-    server1 = startupRule.startServerVM(1, locator.getPort());
+    startupRule.startServerVM(1, locator.getPort());
     gfsh.connectAndVerify(locator);
     gfsh.executeAndAssertThat("create region --name=" + TEST_REGION + " --type=REPLICATE")
         .statusIsSuccess();
diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommand.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommand.java
index 5511317..ecc4b40 100644
--- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommand.java
+++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommand.java
@@ -14,7 +14,6 @@
  */
 package org.apache.geode.connectors.jdbc.internal.cli;
 
-
 import java.util.List;
 import java.util.Set;
 
@@ -120,15 +119,17 @@ public class CreateMappingCommand extends SingleGfshCommand {
     try {
       ConfigurationPersistenceService configurationPersistenceService =
           checkForClusterConfiguration();
-      // TODO: might need to review if 'null' can be something else
-      // we might need to iterate groups
-      String group = groups != null ? groups[0] : null;
-      CacheConfig cacheConfig = configurationPersistenceService.getCacheConfig(group);
-      RegionConfig regionConfig = checkForRegion(regionName, cacheConfig);
-      checkForExistingMapping(regionName, regionConfig);
-      checkForCacheLoader(regionName, regionConfig);
-      checkForCacheWriter(regionName, synchronous, regionConfig);
-      checkForAsyncQueue(regionName, synchronous, cacheConfig);
+      if (groups == null) {
+        groups = new String[] {"cluster"};
+      }
+      for (String group : groups) {
+        CacheConfig cacheConfig = configurationPersistenceService.getCacheConfig(group);
+        RegionConfig regionConfig = checkForRegion(regionName, cacheConfig);
+        checkForExistingMapping(regionName, regionConfig);
+        checkForCacheLoader(regionName, regionConfig);
+        checkForCacheWriter(regionName, synchronous, regionConfig);
+        checkForAsyncQueue(regionName, synchronous, cacheConfig);
+      }
     } catch (PreconditionException ex) {
       return ResultModel.createError(ex.getMessage());
     }
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/RegionMappingTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/RegionMappingTest.java
index aeaa859..cd29c4b 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/RegionMappingTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/RegionMappingTest.java
@@ -43,7 +43,6 @@ public class RegionMappingTest {
 
   private RegionMapping mapping;
 
-  // TODO add a test for server group
   @Before
   public void setUp() {
     name = "name";
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandTest.java
index 72ef2d4..5bd85b2 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandTest.java
@@ -70,7 +70,6 @@ public class CreateMappingCommandTest {
   RegionConfig matchingRegion;
   RegionAttributesType matchingRegionAttributes;
 
-  // TODO: add tests for server group
   @Before
   public void setup() {
     regionName = "regionName";
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingFunctionTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingFunctionTest.java
index 84e9ae4..e14ca8d 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingFunctionTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingFunctionTest.java
@@ -66,7 +66,6 @@ public class CreateMappingFunctionTest {
 
   private CreateMappingFunction function;
 
-  // TODO add a test for server group
   @Before
   public void setUp() {
     context = mock(FunctionContext.class);
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/ListMappingCommandTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/ListMappingCommandTest.java
index 36ed867..190ccc9 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/ListMappingCommandTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/ListMappingCommandTest.java
@@ -40,7 +40,6 @@ public class ListMappingCommandTest {
   public static final String COMMAND = "list jdbc-mappings";
   private ListMappingCommand command;
 
-  // TODO: add tests for server group
   @ClassRule
   public static GfshParserRule gfsh = new GfshParserRule();