[GitHub] [geode] BenjaminPerryRoss commented on a change in pull request #4818: GEODE-7667: Add a 'clear' gfsh command for PR and RR clear

[GitBox]

BenjaminPerryRoss commented on a change in pull request #4818:
URL: https://github.com/apache/geode/pull/4818#discussion_r412492480



##########
File path: 
geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ClearCommandDUnitTest.java
##########
@@ -70,43 +72,49 @@ private static void populateTestRegions() {
 
     Region<String, String> replicateRegion = 
cache.getRegion(REPLICATE_REGION_NAME);
     replicateRegion.put(EMPTY_STRING, "valueForEmptyKey");
-    replicateRegion.put("key1", "value1");
-    replicateRegion.put("key2", "value2");
+    for(int i = 0; i < NUM_ENTRIES; i++) {
+      replicateRegion.put("key" + i, "value" + i);
+    }
 
     Region<String, String> partitionedRegion = 
cache.getRegion(PARTITIONED_REGION_NAME);
-    partitionedRegion.put("key1", "value1");
-    partitionedRegion.put("key2", "value2");
+    replicateRegion.put(EMPTY_STRING, "valueForEmptyKey");
+    for(int i = 0; i < NUM_ENTRIES; i++) {
+      partitionedRegion.put("key" + i, "value" + i);
+    }
   }
 
   @Test
-  public void clearInvalidRegion() {
-    String command = commandString + " --region=NotAValidRegion";
-
+  public void clearFailsWhenRegionIsNotFound() {
+    String invalidRegionName = "NotAValidRegion";
+    String command = new CommandStringBuilder(CliStrings.CLEAR)
+        .addOption(CliStrings.CLEAR_REGION_NAME, 
invalidRegionName).getCommandString();
     gfsh.executeAndAssertThat(command).statusIsError()
-        .containsOutput(String.format(REGION_NOT_FOUND, "/NotAValidRegion"));
+        .containsOutput(String.format(REGION_NOT_FOUND, "/" + 
invalidRegionName));
   }
 
   @Test
-  public void clearReplicateRegion() {
-    String command = commandString + " --region=" + REPLICATE_REGION_NAME;
+  public void clearSucceedsWithValidReplicateRegion() {
+    String command = new CommandStringBuilder(CliStrings.CLEAR)
+        .addOption(CliStrings.CLEAR_REGION_NAME, 
REPLICATE_REGION_NAME).getCommandString();
 
-    gfsh.executeAndAssertThat("list regions").statusIsSuccess();
+    gfsh.executeAndAssertThat("list regions");
     gfsh.executeAndAssertThat(command).statusIsSuccess();
 
-    assertThat(gfsh.getGfshOutput()).contains("Cleared all keys in the 
region");
+    assertThat(gfsh.getGfshOutput()).contains(REMOVE__MSG__CLEARED_ALL_CLEARS);
 
     server1.invoke(() -> verifyAllKeysAreRemoved(REPLICATE_REGION_NAME));
     server2.invoke(() -> verifyAllKeysAreRemoved(REPLICATE_REGION_NAME));
   }
 
 
   @Test
-  public void clearPartitionedRegion() {
-    String command = commandString + " --region=" + PARTITIONED_REGION_NAME;
+  public void clearSucceedsWithValidPartitionedRegion() {
+    String command = new CommandStringBuilder(CliStrings.CLEAR)
+        .addOption(CliStrings.CLEAR_REGION_NAME, 
PARTITIONED_REGION_NAME).getCommandString();
 
-    gfsh.executeAndAssertThat(command).statusIsSuccess();
+    gfsh.executeAndAssertThat(command);

Review comment:
       This was removed by mistake, it should be back in there now.

##########
File path: 
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java
##########
@@ -276,7 +276,7 @@ public DataCommandResult remove(String key, String 
keyClass, String regionName,
 
     if (StringUtils.isEmpty(removeAllKeys) && (key == null)) {
       return DataCommandResult.createRemoveResult(null, null, null,
-          CliStrings.REMOVE__MSG__KEY_EMPTY, false);
+          "BR" + CliStrings.REMOVE__MSG__KEY_EMPTY, false);

Review comment:
       Done




----------------------------------------------------------------
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:
us...@infra.apache.org