[ 
https://issues.apache.org/jira/browse/GEODE-3539?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16254280#comment-16254280
 ] 

ASF GitHub Bot commented on GEODE-3539:
---------------------------------------

PurelyApplied commented on a change in pull request #1052: GEODE-3539: Add 
missing test coverage for 'list regions' and 'describe region' commands
URL: https://github.com/apache/geode/pull/1052#discussion_r151263537
 
 

 ##########
 File path: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ListRegionCommand.java
 ##########
 @@ -110,4 +108,17 @@ public Result listRegion(
     }
     return result;
   }
+
+  public static class ListRegionInterceptor extends 
AbstractCliAroundInterceptor {
 
 Review comment:
   I think cleaner is better, too, but throwing inside the command server-side 
feels a little too close to the "exception as program flow" antipattern.  
Exceptions should be exceptional, right?
   
   There's also the issue that such an exception then gets wrapped a couple of 
times, to result in the output
   ```
   Could not process command due to error. Error occurred while fetching list 
of regions. : Please provide either "member" or "group" option.
   ```
   This could be improved by a better message in the root exception, but still 
would have two confusing layers of output for something that is ultimately a 
User error, not a server-error.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


> Add more test coverage for p2p commands
> ---------------------------------------
>
>                 Key: GEODE-3539
>                 URL: https://issues.apache.org/jira/browse/GEODE-3539
>             Project: Geode
>          Issue Type: Improvement
>          Components: gfsh
>            Reporter: Jinmei Liao
>
> Add more command tests that would eventually get rid of the legacy tests.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to