> On May 11, 2015, 11:52 p.m., Jason Altekruse wrote: > > 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.
RepeatedList is child agnostic so order of nesting should not exhibit any weirder results. I like the idea of empties all around at different levels adding couple of tests to cover this. > On May 11, 2015, 11:52 p.m., Jason Altekruse wrote: > > exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/TestEmptyPopulation.java, > > line 69 > > <https://reviews.apache.org/r/34063/diff/3/?file=955845#file955845line69> > > > > 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. "value" indicates result coming out of accessor.get(#). Error message is actually helpful in giving more context about a possible mismatch whether it is offset or value mismatch. - Hanifi ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34063/#review83318 ----------------------------------------------------------- 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 > >
