-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69070/#review209744
-----------------------------------------------------------


Fix it, then Ship it!




Hi Bogi,

Thanks for fixing this test cases, I have left few minor comments.


src/test/org/apache/sqoop/testutil/S3TestUtils.java
Line 90 (original), 90 (patched)
<https://reviews.apache.org/r/69070/#comment294302>

    The name of this method is a bit misleading, since it does not set a value 
but rather generates one.
    Can we call it generateUniqueDirName or similar?



src/test/org/apache/sqoop/testutil/S3TestUtils.java
Line 131 (original), 139 (patched)
<https://reviews.apache.org/r/69070/#comment294303>

    This line assumes that the targetDirName field is still equals to 
TARGET_DIR_NAME_PREFIX constant so can we just replace the parameter to 
TARGET_DIR_NAME_PREFIX?



src/test/org/apache/sqoop/testutil/S3TestUtils.java
Lines 168 (patched)
<https://reviews.apache.org/r/69070/#comment294304>

    This line assumes that the targetDirName field is still equals to 
HIVE_EXTERNAL_DIR_NAME_PREFIX constant so can we just replace the parameter to 
HIVE_EXTERNAL_DIR_NAME_PREFIX?


- Szabolcs Vasas


On Oct. 18, 2018, 3:11 p.m., Boglarka Egyed wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69070/
> -----------------------------------------------------------
> 
> (Updated Oct. 18, 2018, 3:11 p.m.)
> 
> 
> Review request for Sqoop, Fero Szabo and Szabolcs Vasas.
> 
> 
> Bugs: SQOOP-3394
>     https://issues.apache.org/jira/browse/SQOOP-3394
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> Current external Hive table tests on S3 uses the same external directory name 
> in every unit test cases which can cause problems during running them in an 
> automated environment. These names should be unique in every test cases.
> 
> 
> Diffs
> -----
> 
>   src/test/org/apache/sqoop/testutil/S3TestUtils.java 
> 97d53bbaa7c72d2ad1b890d7a8367c45a3e2b95c 
> 
> 
> Diff: https://reviews.apache.org/r/69070/diff/1/
> 
> 
> Testing
> -------
> 
> ./gradlew test -Ds3.bucket.url=<bucket-url> 
> -Ds3.generator.command=<credential-generator-command>
> 
> 
> Thanks,
> 
> Boglarka Egyed
> 
>

Reply via email to