GEODE-6765: Gfsh list* commands do not return error when no results f…

…ound (apache#3634) - This also includes not returning an error when no members are found to execute against. Authored-by: Jens Deppe <[email protected]>
shjgiser · May 30, 2019 · 47e44ff · 47e44ff
1 parent 1680b2b
commit 47e44ff
Show file tree

Hide file tree

Showing 19 changed files with 76 additions and 44 deletions.
diff --git a/...g/apache/geode/management/internal/cli/commands/ListAsyncEventQueuesCommandDUnitTest.java b/...g/apache/geode/management/internal/cli/commands/ListAsyncEventQueuesCommandDUnitTest.java
@@ -51,9 +51,6 @@ public static void beforeClass() {
 
   @Test
   public void list() {
-    gfsh.executeAndAssertThat("list async-event-queue").statusIsSuccess()
-        .containsOutput(CliStrings.LIST_ASYNC_EVENT_QUEUES__NO_QUEUES_FOUND_MESSAGE);
-
     gfsh.executeAndAssertThat("create async-event-queue --id=queue1 --group=group1 --listener="
         + MyAsyncEventListener.class.getName()).statusIsSuccess();
 
@@ -78,5 +75,14 @@ public void list() {
         .tableHasRowWithValues("Member", "ID", "server-1", "queue")
         .tableHasRowWithValues("Member", "ID", "server-2", "queue");
 
+    gfsh.executeAndAssertThat("destroy async-event-queue --id=queue").statusIsSuccess();
+    gfsh.executeAndAssertThat("destroy async-event-queue --id=queue1").statusIsSuccess();
+    gfsh.executeAndAssertThat("destroy async-event-queue --id=queue2").statusIsSuccess();
+  }
+
+  @Test
+  public void ensureNoResultIsSuccess() {
+    gfsh.executeAndAssertThat("list async-event-queue").statusIsSuccess()
+        .containsOutput(CliStrings.LIST_ASYNC_EVENT_QUEUES__NO_QUEUES_FOUND_MESSAGE);
   }
 }
diff --git a/...butedTest/java/org/apache/geode/management/internal/cli/commands/ListRegionDUnitTest.java b/...butedTest/java/org/apache/geode/management/internal/cli/commands/ListRegionDUnitTest.java
@@ -123,6 +123,13 @@ public void listRegionsInGroup2() throws Exception {
         REGION2, REGION3, SUBREGION1A);
   }
 
+  @Test
+  public void listNoMembersIsSuccess() {
+    CommandStringBuilder csb = new CommandStringBuilder(LIST_REGION);
+    csb.addOption(GROUP, "unknown");
+    gfsh.executeAndAssertThat(csb.toString()).statusIsSuccess();
+  }
+
   private static void createLocalRegion(final String regionName) {
     final Cache cache = CacheFactory.getAnyInstance();
     // Create the data region

diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java
@@ -551,7 +551,7 @@ public static void runLessCommandAsExternalViewer(Result commandResult) {
           null, workingDir);
       p.waitFor();
     } catch (IOException | InterruptedException e) {
-      Gfsh.printlnErr(e.getMessage());
+      Gfsh.println(e.getMessage());
     } finally {
       if (file != null)
         file.delete();

diff --git a/...n/java/org/apache/geode/management/internal/cli/commands/ListAsyncEventQueuesCommand.java b/...n/java/org/apache/geode/management/internal/cli/commands/ListAsyncEventQueuesCommand.java
@@ -48,7 +48,7 @@ public class ListAsyncEventQueuesCommand extends GfshCommand {
   public ResultModel listAsyncEventQueues() {
     Set<DistributedMember> targetMembers = getAllNormalMembers();
     if (targetMembers.isEmpty()) {
-      return ResultModel.createError(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
+      return ResultModel.createInfo(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
     }
 
     // Each (successful) member returns a list of AsyncEventQueueDetails.

diff --git a/...re/src/main/java/org/apache/geode/management/internal/cli/commands/ListClientCommand.java b/...re/src/main/java/org/apache/geode/management/internal/cli/commands/ListClientCommand.java
@@ -51,7 +51,7 @@ public ResultModel listClient() throws Exception {
     ObjectName[] cacheServers = service.getDistributedSystemMXBean().listCacheServerObjectNames();
 
     if (cacheServers.length == 0) {
-      return ResultModel.createError(
+      return ResultModel.createInfo(
           CliStrings.format(CliStrings.LIST_CLIENT_COULD_NOT_RETRIEVE_SERVER_LIST));
     }
 
@@ -81,7 +81,7 @@ public ResultModel listClient() throws Exception {
     }
 
     if (clientServerMap.size() == 0) {
-      return ResultModel.createError(
+      return ResultModel.createInfo(
           CliStrings.format(CliStrings.LIST_COULD_NOT_RETRIEVE_CLIENT_LIST));
     }
 

diff --git a/.../src/main/java/org/apache/geode/management/internal/cli/commands/ListDeployedCommand.java b/.../src/main/java/org/apache/geode/management/internal/cli/commands/ListDeployedCommand.java
@@ -51,7 +51,7 @@ public ResultModel listDeployed(@CliOption(key = {CliStrings.GROUP, CliStrings.G
 
     Set<DistributedMember> targetMembers = findMembers(group, null);
     if (targetMembers.isEmpty()) {
-      return ResultModel.createError(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
+      return ResultModel.createInfo(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
     }
 
     ResultModel result = new ResultModel();

diff --git a/...n/java/org/apache/geode/management/internal/cli/commands/ListDurableClientCQsCommand.java b/...n/java/org/apache/geode/management/internal/cli/commands/ListDurableClientCQsCommand.java
@@ -56,7 +56,7 @@ public ResultModel listDurableClientCQs(
     Set<DistributedMember> targetMembers = findMembers(group, memberNameOrId);
 
     if (targetMembers.isEmpty()) {
-      return ResultModel.createError(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
+      return ResultModel.createInfo(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
     }
 
     final ResultCollector<?, ?> rc =
@@ -72,7 +72,7 @@ public ResultModel listDurableClientCQs(
         table.accumulate("Status", oneResult.getStatus());
         table.accumulate("CQ Name", oneResult.getStatusMessage());
 
-        if (!oneResult.isSuccessful()) {
+        if (!(oneResult.isSuccessful() || oneResult.isIgnorableFailure())) {
           result.setStatus(Result.Status.ERROR);
         }
       }

diff --git a/.../src/main/java/org/apache/geode/management/internal/cli/commands/ListFunctionCommand.java b/.../src/main/java/org/apache/geode/management/internal/cli/commands/ListFunctionCommand.java
@@ -53,7 +53,7 @@ public ResultModel listFunction(
     Set<DistributedMember> targetMembers = findMembers(groups, members);
 
     if (targetMembers.isEmpty()) {
-      return ResultModel.createError(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
+      return ResultModel.createInfo(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
     }
 
     List<CliFunctionResult> results = executeAndGetFunctionResult(this.listFunctionFunction,

diff --git a/...e/src/main/java/org/apache/geode/management/internal/cli/commands/ListGatewayCommand.java b/...e/src/main/java/org/apache/geode/management/internal/cli/commands/ListGatewayCommand.java
@@ -62,7 +62,7 @@ public ResultModel listGateway(
     Set<DistributedMember> dsMembers = findMembers(onGroup, onMember);
 
     if (dsMembers.isEmpty()) {
-      return ResultModel.createError(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
+      return ResultModel.createInfo(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
     }
 
     Map<String, Map<String, GatewaySenderMXBean>> gatewaySenderBeans = new TreeMap<>();
@@ -104,7 +104,7 @@ public ResultModel listGateway(
       }
     }
     if (gatewaySenderBeans.isEmpty() && gatewayReceiverBeans.isEmpty()) {
-      return ResultModel.createError(CliStrings.GATEWAYS_ARE_NOT_AVAILABLE_IN_CLUSTER);
+      return ResultModel.createInfo(CliStrings.GATEWAYS_ARE_NOT_AVAILABLE_IN_CLUSTER);
     }
 
     accumulateListGatewayResult(result, gatewaySenderBeans, gatewayReceiverBeans);

diff --git a/...re/src/main/java/org/apache/geode/management/internal/cli/commands/ListRegionCommand.java b/...re/src/main/java/org/apache/geode/management/internal/cli/commands/ListRegionCommand.java
@@ -63,7 +63,7 @@ public ResultModel listRegion(
     Set<DistributedMember> targetMembers = findMembers(group, memberNameOrId);
 
     if (targetMembers.isEmpty()) {
-      return ResultModel.createError(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
+      return ResultModel.createInfo(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
     }
 
     TabularResultModel resultData = result.addTable("regionInfo");

diff --git a/...n/java/org/apache/geode/management/internal/cli/functions/ListDurableCqNamesFunction.java b/...n/java/org/apache/geode/management/internal/cli/functions/ListDurableCqNamesFunction.java
@@ -68,28 +68,28 @@ private List<CliFunctionResult> createFunctionResult(String memberNameOrId,
     try {
       CacheClientNotifier ccn = CacheClientNotifier.getInstance();
       if (ccn == null) {
-        results.add(new CliFunctionResult(memberNameOrId, CliFunctionResult.StatusState.ERROR,
+        results.add(new CliFunctionResult(memberNameOrId, CliFunctionResult.StatusState.IGNORABLE,
             CliStrings.NO_CLIENT_FOUND));
         return results;
       }
 
       CacheClientProxy ccp = ccn.getClientProxy(durableClientId);
       if (ccp == null) {
-        results.add(new CliFunctionResult(memberNameOrId, CliFunctionResult.StatusState.ERROR,
+        results.add(new CliFunctionResult(memberNameOrId, CliFunctionResult.StatusState.IGNORABLE,
             CliStrings.format(CliStrings.NO_CLIENT_FOUND_WITH_CLIENT_ID, durableClientId)));
         return results;
       }
 
       CqService cqService = ccp.getCache().getCqService();
       if (cqService == null || !cqService.isRunning()) {
-        results.add(new CliFunctionResult(memberNameOrId, CliFunctionResult.StatusState.ERROR,
+        results.add(new CliFunctionResult(memberNameOrId, CliFunctionResult.StatusState.IGNORABLE,
             CliStrings.LIST_DURABLE_CQS__NO__CQS__REGISTERED));
         return results;
       }
 
       List<String> durableCqNames = cqService.getAllDurableClientCqs(ccp.getProxyID());
       if (durableCqNames == null || durableCqNames.isEmpty()) {
-        results.add(new CliFunctionResult(memberNameOrId, CliFunctionResult.StatusState.ERROR,
+        results.add(new CliFunctionResult(memberNameOrId, CliFunctionResult.StatusState.IGNORABLE,
             CliStrings
                 .format(CliStrings.LIST_DURABLE_CQS__NO__CQS__FOR__CLIENT, durableClientId)));
         return results;

diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
@@ -272,15 +272,15 @@ public static void println() {
     gfshout.println();
   }
 
-  public static <T> void println(T toPrint) {
+  public static void println(Object toPrint) {
     gfshout.println(toPrint);
   }
 
-  public static <T> void print(T toPrint) {
+  public static void print(Object toPrint) {
     gfshout.print(toPrint);
   }
 
-  public static <T> void printlnErr(T toPrint) {
+  public static void printlnErr(Object toPrint) {
     gfsherr.println(toPrint);
   }
 

diff --git a/...test/java/org/apache/geode/management/internal/cli/commands/ListAsyncEventQueuesTest.java b/...test/java/org/apache/geode/management/internal/cli/commands/ListAsyncEventQueuesTest.java
@@ -66,7 +66,7 @@ public void noMembersFound() {
     doReturn(Collections.emptySet()).when(command).getAllNormalMembers();
 
     // Command should succeed with one row of data
-    gfsh.executeAndAssertThat(command, COMMAND).statusIsError()
+    gfsh.executeAndAssertThat(command, COMMAND).statusIsSuccess()
         .containsOutput(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
   }
 

diff --git a/...ava/org/apache/geode/management/internal/cli/commands/DurableClientCommandsDUnitTest.java b/...ava/org/apache/geode/management/internal/cli/commands/DurableClientCommandsDUnitTest.java
@@ -121,8 +121,9 @@ public void testListDurableClientCqsForOneGroup() {
     csb.addOption(CliStrings.LIST_DURABLE_CQS__DURABLECLIENTID, CLIENT_NAME);
     commandString = csb.toString();
 
-    gfsh.executeAndAssertThat(commandString).statusIsError()
+    gfsh.executeAndAssertThat(commandString).statusIsSuccess()
         .hasTableSection()
+        .hasColumn("Status").containsExactly("IGNORED", "IGNORED")
         .hasColumn("CQ Name")
         .containsExactlyInAnyOrder(
             CliStrings.format(CliStrings.LIST_DURABLE_CQS__NO__CQS__FOR__CLIENT, CLIENT_NAME),
@@ -135,10 +136,10 @@ public void testListDurableClientCqsWhenNoneExist() {
     csb.addOption(CliStrings.LIST_DURABLE_CQS__DURABLECLIENTID, CLIENT_NAME);
     String commandString = csb.toString();
 
-    gfsh.executeAndAssertThat(commandString).statusIsError()
+    gfsh.executeAndAssertThat(commandString).statusIsSuccess()
         .hasTableSection()
-        .hasColumn("CQ Name")
-        .containsExactlyInAnyOrder(
+        .hasColumn("Status").containsExactlyInAnyOrder("IGNORED", "IGNORED")
+        .hasColumn("CQ Name").containsExactlyInAnyOrder(
             CliStrings.format(CliStrings.NO_CLIENT_FOUND_WITH_CLIENT_ID, CLIENT_NAME),
             CliStrings.format(CliStrings.NO_CLIENT_FOUND_WITH_CLIENT_ID, CLIENT_NAME));
   }
@@ -151,8 +152,9 @@ public void testListDurableClientCqsWithMixedResults() {
     csb.addOption(CliStrings.LIST_DURABLE_CQS__DURABLECLIENTID, CLIENT_NAME);
     String commandString = csb.toString();
 
-    gfsh.executeAndAssertThat(commandString).statusIsError()
+    gfsh.executeAndAssertThat(commandString).statusIsSuccess()
         .hasTableSection()
+        .hasColumn("Status").containsExactlyInAnyOrder("IGNORED", "OK", "OK", "OK")
         .hasColumn("CQ Name")
         .containsExactlyInAnyOrder("cq1", "cq2", "cq3", "No client found with client-id : dc1");
 

diff --git a/...st/java/org/apache/geode/management/internal/cli/commands/ListClientCommandDUnitTest.java b/...st/java/org/apache/geode/management/internal/cli/commands/ListClientCommandDUnitTest.java
@@ -22,8 +22,8 @@
 import java.util.Properties;
 
 import org.apache.commons.lang3.exception.ExceptionUtils;
-import org.junit.BeforeClass;
-import org.junit.ClassRule;
+import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
@@ -43,11 +43,11 @@
 
 @Category({GfshTest.class})
 public class ListClientCommandDUnitTest {
-  @ClassRule
-  public static ClusterStartupRule cluster = new ClusterStartupRule(6);
+  @Rule
+  public ClusterStartupRule cluster = new ClusterStartupRule(6);
 
-  @ClassRule
-  public static GfshCommandRule gfsh = new GfshCommandRule();
+  @Rule
+  public GfshCommandRule gfsh = new GfshCommandRule();
 
   private static final String REGION_NAME = "stocks";
 
@@ -57,22 +57,36 @@ public class ListClientCommandDUnitTest {
 
   private static ClientVM client1, client2;
 
-  @BeforeClass
-  public static void setup() throws Exception {
+  @Before
+  public void setup() throws Exception {
     locator = cluster.startLocatorVM(locatorID);
-    int locatorPort = locator.getPort();
+
+    gfsh.connectAndVerify(locator);
+  }
+
+  private void startServers(MemberVM locator) {
     server1 = cluster.startServerVM(server1ID,
         r -> r.withRegion(RegionShortcut.REPLICATE, REGION_NAME)
-            .withConnectionToLocator(locatorPort));
+            .withConnectionToLocator(locator.getPort()));
     server2 = cluster.startServerVM(server2ID,
         r -> r.withRegion(RegionShortcut.REPLICATE, REGION_NAME)
-            .withConnectionToLocator(locatorPort));
+            .withConnectionToLocator(locator.getPort()));
+  }
 
-    gfsh.connectAndVerify(locator);
+  @Test
+  public void noResultIsSuccess() {
+    startServers(locator);
+    gfsh.executeAndAssertThat("list clients").statusIsSuccess();
+  }
+
+  @Test
+  public void noMembersIsSuccess() {
+    gfsh.executeAndAssertThat("list clients").statusIsSuccess();
   }
 
   @Test
   public void testTwoClientsConnectToOneServer() throws Exception {
+    startServers(locator);
     int server1port = server1.getPort();
     Properties client1props = new Properties();
     client1props.setProperty("name", "client-1");
@@ -123,6 +137,7 @@ public void testTwoClientsConnectToOneServer() throws Exception {
 
   @Test
   public void oneClientConnectToTwoServers() throws Exception {
+    startServers(locator);
     int server1port = server1.getPort();
     int server2port = server2.getPort();
     Properties client1props = new Properties();

diff --git a/geode-junit/src/main/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java b/geode-junit/src/main/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java
@@ -281,7 +281,9 @@ public InfoResultModelAssert hasInfoSection(String sectionName) {
   // convenience method to get the first table section. if more than one table section
   // use the sectionName to get it
   public TabularResultModelAssert hasTableSection() {
-    return new TabularResultModelAssert(getResultModel().getTableSections().get(0));
+    List<TabularResultModel> table = getResultModel().getTableSections();
+    assertThat(table.size()).isGreaterThan(0);
+    return new TabularResultModelAssert(table.get(0));
   }
 
   public TabularResultModelAssert hasTableSection(String sectionName) {

diff --git a/.../org/apache/geode/internal/cache/wan/wancommand/DestroyGatewaySenderCommandDUnitTest.java b/.../org/apache/geode/internal/cache/wan/wancommand/DestroyGatewaySenderCommandDUnitTest.java
@@ -82,7 +82,7 @@ public void testCreateDestroySerialGatewaySenderWithDefault() throws Exception {
 
     locatorSite1.waitUntilGatewaySendersAreReadyOnExactlyThisManyServers(0);
 
-    gfsh.executeAndAssertThat("list gateways").statusIsError()
+    gfsh.executeAndAssertThat("list gateways").statusIsSuccess()
         .containsOutput("GatewaySenders or GatewayReceivers are not available in cluster");
   }
 
@@ -100,7 +100,7 @@ public void testCreateDestroyParallellGatewaySenderWithDefault() throws Exceptio
 
     locatorSite1.waitUntilGatewaySendersAreReadyOnExactlyThisManyServers(0);
 
-    gfsh.executeAndAssertThat("list gateways").statusIsError()
+    gfsh.executeAndAssertThat("list gateways").statusIsSuccess()
         .containsOutput("GatewaySenders or GatewayReceivers are not available in cluster");
   }
 }
diff --git a/...est/java/org/apache/geode/internal/cache/wan/wancommand/ListGatewaysCommandDUnitTest.java b/...est/java/org/apache/geode/internal/cache/wan/wancommand/ListGatewaysCommandDUnitTest.java
@@ -89,7 +89,7 @@ public void testListGatewayWithNoSenderReceiver() {
     locatorSite1.invoke(() -> validateMemberMXBeanProxy(getMember(server3.getVM())));
 
     String command = CliStrings.LIST_GATEWAY;
-    gfsh.executeAndAssertThat(command).statusIsError();
+    gfsh.executeAndAssertThat(command).statusIsSuccess();
   }
 
   @Test

diff --git a/...ationTest/java/org/apache/geode/management/internal/cli/commands/CommandOverHttpTest.java b/...ationTest/java/org/apache/geode/management/internal/cli/commands/CommandOverHttpTest.java
@@ -54,7 +54,7 @@ public void before() throws Exception {
   @Test
   public void testListClient() throws Exception {
     gfshRule.executeAndAssertThat("list clients")
-        .statusIsError()
+        .statusIsSuccess()
         .hasInfoSection().hasOutput()
         .isEqualTo("No clients were retrieved for cache-servers.");
   }