----------------------------------------------------------- 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 <[email protected]> > 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 > >
