----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69199/#review210215 -----------------------------------------------------------
src/test/org/apache/sqoop/importjob/SplitByImportTest.java Lines 21 (patched) <https://reviews.apache.org/r/69199/#comment294834> Unused import. src/test/org/apache/sqoop/importjob/SplitByImportTest.java Lines 29 (patched) <https://reviews.apache.org/r/69199/#comment294835> Unused import. src/test/org/apache/sqoop/importjob/SplitByImportTest.java Lines 137 (patched) <https://reviews.apache.org/r/69199/#comment294837> Since the num-mappers is 1 the split-by logic does not get invoked, can you please set it to 2? src/test/org/apache/sqoop/importjob/SplitByImportTest.java Lines 152 (patched) <https://reviews.apache.org/r/69199/#comment294836> Since the readAll* methods of ParquetReader close the reader this method could be simplified to something like this: private void verifyParquetFile() { ParquetReader reader = new ParquetReader(new Path(getWarehouseDir() + "/" + getTableName()), getConf()); assertEquals(asList(configuration.getExpectedResultsForParquet()), reader.readAllInCsv()); } src/test/org/apache/sqoop/importjob/configuration/GenericImportJobSplitByTestConfiguration.java Lines 60 (patched) <https://reviews.apache.org/r/69199/#comment294833> I suggest simplifying this method by using streams: return data.stream() .map(element -> StringUtils.join(element, SEPARATOR)) .toArray(String[]::new); src/test/org/apache/sqoop/importjob/configuration/GenericImportJobSplitByTestConfiguration.java Lines 61 (patched) <https://reviews.apache.org/r/69199/#comment294832> We should not hardcode the size of the String[] here. src/test/org/apache/sqoop/importjob/configuration/GenericImportJobSplitByTestConfiguration.java Lines 63 (patched) <https://reviews.apache.org/r/69199/#comment294831> This variable is unused. - Szabolcs Vasas On Oct. 30, 2018, 4:26 p.m., Fero Szabo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69199/ > ----------------------------------------------------------- > > (Updated Oct. 30, 2018, 4:26 p.m.) > > > Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas. > > > Bugs: SQOOP-3400 > https://issues.apache.org/jira/browse/SQOOP-3400 > > > Repository: sqoop-trunk > > > Description > ------- > > Integration tests fro SQOOP-2949. > > > Diffs > ----- > > src/java/org/apache/sqoop/mapreduce/db/TextSplitter.java 22bbfe68 > src/test/org/apache/sqoop/importjob/SplitByImportTest.java PRE-CREATION > > src/test/org/apache/sqoop/importjob/configuration/GenericImportJobSplitByTestConfiguration.java > PRE-CREATION > src/test/org/apache/sqoop/testutil/BaseSqoopTestCase.java fe6ba831 > > > Diff: https://reviews.apache.org/r/69199/diff/3/ > > > Testing > ------- > > This is the testing part for a fix that lacked testing. > gradle test and gradle 3rdpartytests. > > > Thanks, > > Fero Szabo > >
