sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jarek Cecho" <jar...@apache.org>
Subject Re: Review Request 32429: SQOOP-2016: Sqoop2: Create integration test for JDBC to Hive
Date Tue, 24 Mar 2015 15:57:39 GMT

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


Nice, good work Abe! The test is passing on my box, so +1. I have couple of comments, mostly
nits:


test/src/main/java/org/apache/sqoop/test/hive/HiveServerRunner.java
<https://reviews.apache.org/r/32429/#comment125691>

    Seems as excellent use case for our LoggerWriter?
    
    https://github.com/apache/sqoop/blob/sqoop2/common-test/src/main/java/org/apache/sqoop/common/test/utils/LoggerWriter.java



test/src/main/java/org/apache/sqoop/test/hive/InternalHiveServerRunner.java
<https://reviews.apache.org/r/32429/#comment125692>

    Missing license header.



test/src/main/java/org/apache/sqoop/test/hive/MetastoreServerRunner.java
<https://reviews.apache.org/r/32429/#comment125693>

    Nit: It seems that we should do s/Hive server/Hive Metastore Server/g



test/src/main/java/org/apache/sqoop/test/hive/MetastoreServerRunner.java
<https://reviews.apache.org/r/32429/#comment125694>

    Same note as before - seems as good use case for LoggerWriter.



test/src/main/java/org/apache/sqoop/test/testcases/HiveConnectorTestCase.java
<https://reviews.apache.org/r/32429/#comment125697>

    I believe that we should randomize the path a bit so that we can retain test outpus and
possibly run multiple test cases at the same time right? Probably putting the warehouse directory
to subdirectory of getTemporaryPath()?



test/src/test/java/org/apache/sqoop/integration/connector/kite/FromRDBMSToKiteTest.java
<https://reviews.apache.org/r/32429/#comment125698>

    I'm thinking about putting the Hive specific tests to a separate test class, so that we
don't have one uber test case with all a lot of long running tests and rather have bigger
number of "faster" test cases ("faster" is relative term in our integration tests). I'm wondering
what are your toughts?



test/src/test/java/org/apache/sqoop/integration/connector/kite/FromRDBMSToKiteTest.java
<https://reviews.apache.org/r/32429/#comment125699>

    I think that this subset can be replaced with fillRdbmsFromConfig:
    
    https://github.com/apache/sqoop/blob/sqoop2/test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java#L148


Jarcec

- Jarek Cecho


On March 24, 2015, 9:01 a.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32429/
> -----------------------------------------------------------
> 
> (Updated March 24, 2015, 9:01 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2016
>     https://issues.apache.org/jira/browse/SQOOP-2016
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 1704a3d4ba8bc3b6c6a968c7485374b4c893e453
> Author: Abraham Elmahrek <abe@apache.org>
> Date:   Thu Mar 19 12:57:18 2015 -0700
> 
>     SQOOP-2016: Sqoop2: Create integration test for JDBC to Hive
> 
> :000000 100644 0000000... dfe0f43... A  common-test/src/main/java/org/apache/sqoop/common/test/db/HiveProvider.java
> :100644 100644 87534c7... 7f0f750... M  common-test/src/main/java/org/apache/sqoop/common/test/utils/NetworkUtils.java
> :100644 100644 db8d4e6... 6aa28be... M  connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteDatasetExecutor.java
> :100644 100644 44721a8... 0e797f8... M  connector/connector-kite/src/test/java/org/apache/sqoop/connector/kite/TestKiteExecutor.java
> :100644 100644 e3431bd... c608ca7... M  pom.xml
> :100644 100644 f743d25... 4af668e... M  test/pom.xml
> :000000 100644 0000000... 8b355bd... A  test/src/main/java/org/apache/sqoop/test/hive/HiveServerRunner.java
> :000000 100644 0000000... fa786e2... A  test/src/main/java/org/apache/sqoop/test/hive/HiveServerRunnerFactory.java
> :000000 100644 0000000... 67b39d4... A  test/src/main/java/org/apache/sqoop/test/hive/InternalHiveServerRunner.java
> :000000 100644 0000000... 5cc3f8b... A  test/src/main/java/org/apache/sqoop/test/hive/InternalMetastoreServerRunner.java
> :000000 100644 0000000... c561c59... A  test/src/main/java/org/apache/sqoop/test/hive/MetastoreServerRunner.java
> :000000 100644 0000000... 127ebdf... A  test/src/main/java/org/apache/sqoop/test/hive/MetastoreServerRunnerFactory.java
> :100644 100644 648e2f6... 4d27886... M  test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java
> :000000 100644 0000000... 4388650... A  test/src/main/java/org/apache/sqoop/test/testcases/HiveConnectorTestCase.java
> :100644 100644 8ca1c3a... 2698fe4... M  test/src/test/java/org/apache/sqoop/integration/connector/kite/FromRDBMSToKiteTest.java
> 
> 
> Diffs
> -----
> 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/HiveProvider.java PRE-CREATION

>   common-test/src/main/java/org/apache/sqoop/common/test/utils/NetworkUtils.java 87534c7

>   connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteDatasetExecutor.java
db8d4e6 
>   connector/connector-kite/src/test/java/org/apache/sqoop/connector/kite/TestKiteExecutor.java
44721a8 
>   pom.xml e3431bd 
>   test/pom.xml f743d25 
>   test/src/main/java/org/apache/sqoop/test/hive/HiveServerRunner.java PRE-CREATION 
>   test/src/main/java/org/apache/sqoop/test/hive/HiveServerRunnerFactory.java PRE-CREATION

>   test/src/main/java/org/apache/sqoop/test/hive/InternalHiveServerRunner.java PRE-CREATION

>   test/src/main/java/org/apache/sqoop/test/hive/InternalMetastoreServerRunner.java PRE-CREATION

>   test/src/main/java/org/apache/sqoop/test/hive/MetastoreServerRunner.java PRE-CREATION

>   test/src/main/java/org/apache/sqoop/test/hive/MetastoreServerRunnerFactory.java PRE-CREATION

>   test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java 648e2f6

>   test/src/main/java/org/apache/sqoop/test/testcases/HiveConnectorTestCase.java PRE-CREATION

>   test/src/test/java/org/apache/sqoop/integration/connector/kite/FromRDBMSToKiteTest.java
8ca1c3a 
> 
> Diff: https://reviews.apache.org/r/32429/diff/
> 
> 
> Testing
> -------
> 
> mvn clean integration-test -Dtest=org.apache.sqoop.integration.connector.kite.FromRDBMSToKiteTest#testHiveCities
-DfailIfNoTests=false
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message