----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68712/#review208794 -----------------------------------------------------------
Hi Bogi, Thank you for improving the patch, it is much more concise now. However I think we could make it more concise by introducing parametrization since the 2 new test cases are almost the same the only real difference is the file format. What do you think? src/test/org/apache/sqoop/s3/TestS3ExternalHiveTableTextImport.java Lines 112 (patched) <https://reviews.apache.org/r/68712/#comment292991> This assert could be simplified if we introduce a convenience method to HiveServer2TestUtils, which would return the rows in CSV. For example: public List<String> loadCsvRowsFromTable(String tableName) { return loadRawRowsFromTable(tableName).stream() .map(list -> StringUtils.join(list, ",")) .collect(toList()); } If we have this method we can just use Assert.assertEquals to compare the 2 lists. - Szabolcs Vasas On Sept. 18, 2018, 2:24 p.m., Boglarka Egyed wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68712/ > ----------------------------------------------------------- > > (Updated Sept. 18, 2018, 2:24 p.m.) > > > Review request for Sqoop, daniel voros, Fero Szabo, and Szabolcs Vasas. > > > Bugs: SQOOP-3376 > https://issues.apache.org/jira/browse/SQOOP-3376 > > > Repository: sqoop-trunk > > > Description > ------- > > Testing the Text and Parquet imports into an external Hive table backed by S3. > > > Diffs > ----- > > src/test/org/apache/sqoop/s3/TestS3ExternalHiveTableParquetImport.java > PRE-CREATION > src/test/org/apache/sqoop/s3/TestS3ExternalHiveTableTextImport.java > PRE-CREATION > src/test/org/apache/sqoop/testutil/S3TestUtils.java > 0e6ef5bf001797aa70a7ad50d261c6fd384222fe > > > Diff: https://reviews.apache.org/r/68712/diff/2/ > > > Testing > ------- > > ./gradlew test -Ds3.bucket.url=<bucket-url> > -Ds3.generator.command=<credential-generator-command> > > > Thanks, > > Boglarka Egyed > >
