----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34063/#review83318 -----------------------------------------------------------
I think it would be good to round out the tests will cases that go into a little deeper in terms of nesting, just to be complete. I think at least do a repetition of the same complex type followed by the other example: map,map,list and list,list,map as well as going back and forth at least once map,list,map,list would be good to confirm all code paths. I know these cases might not all be related to empty population, but they will confirm that changes in these other areas do not impact this codepath. exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/TestEmptyPopulation.java <https://reviews.apache.org/r/34063/#comment134261> A minor comment, but as assertEquals will already print out something like: expecting <0> but recieved <other val>, This message doesn't add anything to that, I would make it more descriptive or just remove it. - Jason Altekruse On May 11, 2015, 11:40 p.m., Hanifi Gunes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34063/ > ----------------------------------------------------------- > > (Updated May 11, 2015, 11:40 p.m.) > > > Review request for drill, abdelhakim deneche, Jason Altekruse, Mehant Baid, > and Venki Korukanti. > > > Repository: drill-git > > > Description > ------- > > DRILL-2776: add extensive tests for empty population coverage > > > Diffs > ----- > > > exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/TestEmptyPopulation.java > PRE-CREATION > > exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/TestEmptyPopulator.java > 23cc316aac7e7d64fea2a2549a42d0a9c4d401bb > exec/java-exec/src/test/resources/vector/complex/map-empty-between.json > PRE-CREATION > exec/java-exec/src/test/resources/vector/complex/map-empty-first.json > PRE-CREATION > exec/java-exec/src/test/resources/vector/complex/map-empty-last.json > PRE-CREATION > > exec/java-exec/src/test/resources/vector/complex/repeated-list-empty-between.json > PRE-CREATION > > exec/java-exec/src/test/resources/vector/complex/repeated-list-empty-first.json > PRE-CREATION > > exec/java-exec/src/test/resources/vector/complex/repeated-list-empty-last.json > PRE-CREATION > > exec/java-exec/src/test/resources/vector/complex/repeated-map-empty-between.json > PRE-CREATION > > exec/java-exec/src/test/resources/vector/complex/repeated-map-empty-first.json > PRE-CREATION > > exec/java-exec/src/test/resources/vector/complex/repeated-map-empty-last.json > PRE-CREATION > > exec/java-exec/src/test/resources/vector/complex/repeated-scalar-empty-between.json > PRE-CREATION > > exec/java-exec/src/test/resources/vector/complex/repeated-scalar-empty-first.json > PRE-CREATION > > exec/java-exec/src/test/resources/vector/complex/repeated-scalar-empty-last.json > PRE-CREATION > > Diff: https://reviews.apache.org/r/34063/diff/ > > > Testing > ------- > > TestEmptyPopulation > > > Thanks, > > Hanifi Gunes > >
