----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40895/#review109085 -----------------------------------------------------------
Thanks for uploading the patch Abe! Few notes below: pom.xml (lines 233 - 259) <https://reviews.apache.org/r/40895/#comment168509> I'm not a big fan of the current approach of using maven profiles for all the various groups we have - it's hard to mantain and get oriented in. Would it make sense to simply document how to specify the given groups on command line or is that too hard to do? test/src/main/java/org/apache/sqoop/test/minicluster/RealSqoopCluster.java (lines 50 - 68) <https://reviews.apache.org/r/40895/#comment168510> Can we create a util class that for given SqoopClient will drop all jobs/links? It seems that we're having this code sniplet a lot, so it would be great to reuse the code - happy to review this as part of separate JIRA if needed. test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java (lines 102 - 106) <https://reviews.apache.org/r/40895/#comment168511> Let's keep the "setMethodName" and create new BeforeMethod call to drop the getMapreduceDirectory()? test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java (lines 161 - 163) <https://reviews.apache.org/r/40895/#comment168512> I'm generally +1 on migrating to the MiniClusterFactory to be honest, but let's make that as standalone patch - very likely a subtask of SQOOP-2329. test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromHDFSToKafkaTest.java (line 26) <https://reviews.apache.org/r/40895/#comment168513> Why not run this test on real cluster with real Kafka instance? test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromRDBMSToKafkaTest.java (line 28) <https://reviews.apache.org/r/40895/#comment168514> Why not run this test on real cluster with real Kafka instance? test/src/test/java/org/apache/sqoop/integration/server/ShowJobInOrderTest.java (lines 101 - 104) <https://reviews.apache.org/r/40895/#comment168515> I've just committed SQOOP-2715 that cleans up failures of this test, so let's clean up this change. Jarcec - Jarek Cecho On Dec. 4, 2015, 7:46 a.m., Abraham Fine wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40895/ > ----------------------------------------------------------- > > (Updated Dec. 4, 2015, 7:46 a.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-2720 > https://issues.apache.org/jira/browse/SQOOP-2720 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > Sqoop2: Allow the integration tests to be run against a real cluster > > > Diffs > ----- > > pom.xml 91721ce > test/src/main/java/org/apache/sqoop/test/minicluster/RealSqoopCluster.java > 325a6a9 > test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java > bd4ba6a > > test/src/test/java/org/apache/sqoop/integration/connector/hive/FromRDBMSToKiteHiveTest.java > 0e46bf3 > > test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromHDFSToKafkaTest.java > aa062fb > > test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromRDBMSToKafkaTest.java > 6e78a13 > > test/src/test/java/org/apache/sqoop/integration/connectorloading/BlacklistedConnectorTest.java > 6228b0d > > test/src/test/java/org/apache/sqoop/integration/connectorloading/ClasspathTest.java > 4a2e7a4 > > test/src/test/java/org/apache/sqoop/integration/repository/derby/upgrade/Derby1_99_3UpgradeTest.java > 8d02e24 > > test/src/test/java/org/apache/sqoop/integration/repository/derby/upgrade/Derby1_99_4UpgradeTest.java > b88940a > > test/src/test/java/org/apache/sqoop/integration/repository/derby/upgrade/Derby1_99_5UpgradeTest.java > 1f3563d > > test/src/test/java/org/apache/sqoop/integration/repository/derby/upgrade/Derby1_99_6UpgradeTest.java > a57a420 > > test/src/test/java/org/apache/sqoop/integration/server/ShowJobInOrderTest.java > cbf1e90 > > Diff: https://reviews.apache.org/r/40895/diff/ > > > Testing > ------- > > yes > > > Thanks, > > Abraham Fine > >
