> On June 25, 2015, 5:12 p.m., Jason Altekruse wrote: > > exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java, line > > 397 > > <https://reviews.apache.org/r/35881/diff/1/?file=992189#file992189line397> > > > > 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.
you are right. I did this because I noticed in TestWindowFrame I was getting one extra "empty" batch for each query, I guess it's the first batch returned with the schema to the client. I guess we can just count all batches and let each specific unit test to explicitely account for the "empty" batches by adding +1 to the expected number > On June 25, 2015, 5:12 p.m., Jason Altekruse wrote: > > exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java, line > > 394 > > <https://reviews.apache.org/r/35881/diff/1/?file=992189#file992189line394> > > > > 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. fixing this - abdelhakim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35881/#review89378 ----------------------------------------------------------- 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 > >
