> On July 20, 2015, 5:25 p.m., Darrel Schneider wrote:
> > gemfire-core/src/test/java/com/gemstone/gemfire/cache/RegionFactoryJUnitTest.java,
> >  line 1113
> > <https://reviews.apache.org/r/36594/diff/1/?file=1015167#file1015167line1113>
> >
> >     I'm not sure this unit test needs to create a region and then call 
> > assertBasicRegionFunctionality.
> >     Why don't you just make sure that the cache is closed after the try 
> > block?
> >     
> >     Inside the try block you could assert that !cache.isClosed().
> >     Outside the try block you could assert that cache.isClosed.
> >     You just need add a "Cache c1;" outside the try block and then "c1 = 
> > cache;" inside the try block.
> >     The after the try block you can assert that c1.isClosed().
> >     I'd get rid of all the RegionFactory code get rid of this test throwing 
> > CacheClosedException

Thanks Darrel! I was following some of the patterns at testAfterCacheClosed, 
testAfterConnect, testRegionFactoryAndCacheClose.. But I more than agree that 
it's not necessary. Fixed.


- William


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


On July 18, 2015, 5:18 a.m., William Markito wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36594/
> -----------------------------------------------------------
> 
> (Updated July 18, 2015, 5:18 a.m.)
> 
> 
> Review request for geode, Darrel Schneider and Kirk Lund.
> 
> 
> Bugs: GEODE-144
>     https://issues.apache.org/jira/browse/GEODE-144
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Adding try-with-resources for Cache
> 
> 
> Diffs
> -----
> 
>   gemfire-core/src/main/java/com/gemstone/gemfire/cache/RegionService.java 
> 9ea65c0 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/cache/RegionFactoryJUnitTest.java
>  151e9c8 
> 
> Diff: https://reviews.apache.org/r/36594/diff/
> 
> 
> Testing
> -------
> 
> Added JUnit test
> Ran test suite/build.
> 
> 
> Thanks,
> 
> William Markito
> 
>

Reply via email to