> On July 20, 2017, 2:52 p.m., Ken Howe wrote:
> > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/QueryCommandUnitTest.java
> > Lines 24 (patched)
> > <https://reviews.apache.org/r/60985/diff/1/?file=1780086#file1780086line24>
> >
> >     Seems there's opportunity for more tests in here, for instance, 
> > queryWithInvalidRegionName, queryInvalidExceptionThrown, etc.
> >     
> >     Have you checked coverage, in particular for the new classes 
> > QueryCommand, and QueryInterceptor

I've updated with a revision to add some tests for the sad paths as you 
suggested.  Coverage is as follows: QueryCommand (100% method, 89% line), 
QueryInterceptor (100% method, 92%line).  Most of the lines without coverage 
are things like an extra-cautious 
```
} catch (IOException e) { 
throw new RuntimeException(e);
}
```


- Jared


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60985/#review181043
-----------------------------------------------------------


On July 20, 2017, 5:54 p.m., Jared Stewart wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60985/
> -----------------------------------------------------------
> 
> (Updated July 20, 2017, 5:54 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-3217: Reimplement gfsh query as a single-step command
> 
> 
> Diffs
> -----
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DataCommands.java
>  3f4397b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/QueryCommand.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/QueryInterceptor.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/DataCommandResult.java
>  fe88fc9 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java
>  41cc171 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
>  8ab7c93 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java
>  fb63184 
>   
> geode-core/src/test/java/org/apache/geode/management/DataCommandMBeanTest.java
>  e5d6ce8 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/QueryCommandTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/QueryCommandUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/result/ResultBuilderTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60985/diff/2/
> 
> 
> Testing
> -------
> 
> Precheckin passed (except for some flaky failures that appear to be unrelated)
> 
> 
> Thanks,
> 
> Jared Stewart
> 
>

Reply via email to