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



exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java (line 394)
<https://reviews.apache.org/r/35881/#comment141957>

    Should make this a named constant like EXPECTED_BATCH_COUNT_NOT_SET. While 
-1 is a common magic number for things like this it makes it a little more 
self-documenting.



exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java (line 397)
<https://reviews.apache.org/r/35881/#comment141958>

    This behavior to ignore empty batches might suprise some people using this 
feature. I don't know enough about when we actually produce empty batches for 
the client to consumer to know if will be a big issue though.


- Jason Altekruse


On June 25, 2015, 4:53 p.m., abdelhakim deneche wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35881/
> -----------------------------------------------------------
> 
> (Updated June 25, 2015, 4:53 p.m.)
> 
> 
> Review request for drill and Jason Altekruse.
> 
> 
> Bugs: DRILL-3354
>     https://issues.apache.org/jira/browse/DRILL-3354
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Unit tests can specifiy an expectedNumBatches when using the TestBuilder, 
> these tests will fail if they receive a different number of result batches.
> I also updated TestWindowFrame to use this feature
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java d4e7ed6 
>   exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java 666ecda 
>   
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java
>  1d6c517 
> 
> Diff: https://reviews.apache.org/r/35881/diff/
> 
> 
> Testing
> -------
> 
> unit tests are passing
> 
> 
> Thanks,
> 
> abdelhakim deneche
> 
>

Reply via email to