> On Sept. 26, 2014, 5:55 p.m., Timothy Chen wrote: > > exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java, line > > 201 > > <https://reviews.apache.org/r/26088/diff/1/?file=706172#file706172line201> > > > > Why do you want to remove from the list instead of just iterating it? > > Not sure if anyone will assume the list passed in will get elements removed.
This is to allow unordered matching. We could instead keep a track of indecies that have been 'used up' and not allow those to be matched against future records, however by actually removing them we have a smaller and smaller list to iterate over to match the rest of the records. > On Sept. 26, 2014, 5:55 p.m., Timothy Chen wrote: > > exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java, line > > 161 > > <https://reviews.apache.org/r/26088/diff/1/?file=706172#file706172line161> > > > > Wierd formatting just for clarity, would you prefer fewer spaces anywhere (before the bang?) or just an extra space before the last parenthesis? > On Sept. 26, 2014, 5:55 p.m., Timothy Chen wrote: > > exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java, line 79 > > <https://reviews.apache.org/r/26088/diff/1/?file=706172#file706172line79> > > > > I think although the comments are clear, I will assume using a enum is > > much clearer what the different states are and also more flexible. These aren't all mutually exclusive states, so I do not beleive an ENUM is the best option. Are you thinking we would have a list of ENUMs for all of the options that are currently booleans? > On Sept. 26, 2014, 5:55 p.m., Timothy Chen wrote: > > exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java, line 82 > > <https://reviews.apache.org/r/26088/diff/1/?file=706171#file706171line82> > > > > Is this being used anywhere? It is used in the comparison method to decide whether or not to print all of the values, including those that match. This can be useful for debugging when the pattern of the data can help drive interpretation of the error state. Leaving all of the logging statements on be default would take up a lot of extra time. I don't want this enabled by any of the tests explicitly. I just want it available while Devs are debugging individual tests. I'll add a comment to clarify the intended use. > On Sept. 26, 2014, 5:55 p.m., Timothy Chen wrote: > > common/src/main/java/org/apache/drill/common/types/Types.java, line 412 > > <https://reviews.apache.org/r/26088/diff/1/?file=706168#file706168line412> > > > > This seems odd, why can we assume LATE is json? I'll change it to throw an exception, we shouldn't ever see the LATE type during execution or in a query. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26088/#review54701 ----------------------------------------------------------- On Sept. 26, 2014, 5:45 p.m., Jason Altekruse wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26088/ > ----------------------------------------------------------- > > (Updated Sept. 26, 2014, 5:45 p.m.) > > > Review request for drill and Mehant Baid. > > > Repository: drill-git > > > Description > ------- > > new unit test framework to ease the process of creating unit tests with full > result verification > > > Diffs > ----- > > common/src/main/java/org/apache/drill/common/types/Types.java 8ae3edd > > exec/java-exec/src/main/java/org/apache/drill/exec/store/ClassPathFileSystem.java > 5e00f08 > exec/java-exec/src/main/resources/bootstrap-storage-plugins.json 31df303 > exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java 99c4da5 > exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java > PRE-CREATION > exec/java-exec/src/test/java/org/apache/drill/TestAltSortQueries.java > 5768908 > exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java PRE-CREATION > exec/java-exec/src/test/java/org/apache/drill/TestTestFramework.java > PRE-CREATION > exec/java-exec/src/test/java/org/apache/drill/TestValidationFramework.java > PRE-CREATION > > exec/java-exec/src/test/java/org/apache/drill/exec/HyperVectorValueIterator.java > PRE-CREATION > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestParquetWriter.java > ac5673d > exec/java-exec/src/test/resources/sort/testSelectWithLimitOffset.json > PRE-CREATION > exec/java-exec/src/test/resources/sort/testSelectWithLimitOffset.tsv > PRE-CREATION > > exec/java-exec/src/test/resources/sort/testSelectWithLimitOffset_reordered.tsv > PRE-CREATION > pom.xml 3cc4f38 > > Diff: https://reviews.apache.org/r/26088/diff/ > > > Testing > ------- > > Added both positive an negative unit test for varioud functionalities in the > unit test framework. All previous unit tests are passing. > > > Thanks, > > Jason Altekruse > >
