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



gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/ChunkWithHeapFormJUnitTest.java
 (line 42)
<https://reviews.apache.org/r/41405/#comment170448>

    Seems like it would be better to use "assertSame" here to make clear that 
we want identity to be checked.



gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/ChunkWithHeapFormJUnitTest.java
 (line 60)
<https://reviews.apache.org/r/41405/#comment170452>

    Should you also do this:
    assertNotSame(valueInBytes, chunkWithOutHeapForm.getRawBytes());



gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/GemFireChunkFactoryJUnitTest.java
 (line 44)
<https://reviews.apache.org/r/41405/#comment170457>

    should you also have a @AfterClass that calls
      SimpleMemoryAllocatorImpl.freeOffHeapMemory()



gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/GemFireChunkFactoryJUnitTest.java
 (line 73)
<https://reviews.apache.org/r/41405/#comment170473>

    Should chunk.release by in a finally block?



gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/GemFireChunkFactoryJUnitTest.java
 (line 99)
<https://reviews.apache.org/r/41405/#comment170474>

    In reviewboard your indentation does not look correct? Did you end up 
having some tabs instead of just spaces?



gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/GemFireChunkJUnitTest.java
 (line 58)
<https://reviews.apache.org/r/41405/#comment170478>

    What does this do? Is it like adding -ea on the command line? I think we 
want unit tests to run with assertions enabled but this looks like something 
that gets set and never unset. That is probably ok but wanted it to be 
discussed.



gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/GemFireChunkJUnitTest.java
 (line 737)
<https://reviews.apache.org/r/41405/#comment170481>

    Should this test validate that it was able to retain the expected number of 
times and then have the next retain fail?


- Darrel Schneider


On Dec. 15, 2015, 10:27 a.m., Sai Boorlagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41405/
> -----------------------------------------------------------
> 
> (Updated Dec. 15, 2015, 10:27 a.m.)
> 
> 
> Review request for geode and Darrel Schneider.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Added chunck classes tests
> 
> 
> Diffs
> -----
> 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/AbstractStoredObjectTestBase.java
>  a5fac482cc7abb7f88582114514ba97c24b4579f 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/ChunkWithHeapFormJUnitTest.java
>  PRE-CREATION 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/GemFireChunkFactoryJUnitTest.java
>  PRE-CREATION 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/GemFireChunkJUnitTest.java
>  PRE-CREATION 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/GemFireChunkSliceJUnitTest.java
>  PRE-CREATION 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/offheap/StoredObjectTestSuite.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41405/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Sai Boorlagadda
> 
>

Reply via email to