> On Dec. 5, 2015, 2:28 p.m., Jarek Cecho wrote: > > test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromHDFSToKafkaTest.java, > > line 26 > > <https://reviews.apache.org/r/40895/diff/4/?file=1153632#file1153632line26> > > > > Why not run this test on real cluster with real Kafka instance?
that is something that we can definitely do, but i figured that it was beyond the scope of this jira and something we should tackle at a later date. > On Dec. 5, 2015, 2:28 p.m., Jarek Cecho wrote: > > test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromRDBMSToKafkaTest.java, > > line 28 > > <https://reviews.apache.org/r/40895/diff/4/?file=1153633#file1153633line28> > > > > Why not run this test on real cluster with real Kafka instance? see other comment on kafka test > On Dec. 5, 2015, 2:28 p.m., Jarek Cecho wrote: > > test/src/main/java/org/apache/sqoop/test/minicluster/RealSqoopCluster.java, > > lines 50-68 > > <https://reviews.apache.org/r/40895/diff/4/?file=1153629#file1153629line50> > > > > 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. whoops, forgot to clean that up :) > On Dec. 5, 2015, 2:28 p.m., Jarek Cecho wrote: > > pom.xml, lines 233-259 > > <https://reviews.apache.org/r/40895/diff/4/?file=1153628#file1153628line233> > > > > 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? i am sure that is something that maven supports. it just seemed to me that this seemed like a perfect use case for a profile. i think we only have two groups at the moment so i can not really see this getting out of hand any time soon. > On Dec. 5, 2015, 2:28 p.m., Jarek Cecho wrote: > > test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java, > > lines 107-111 > > <https://reviews.apache.org/r/40895/diff/4/?file=1153630#file1153630line107> > > > > Let's keep the "setMethodName" and create new BeforeMethod call to drop > > the getMapreduceDirectory()? my concern here is that getMapreduceDirectory is dependent on having the name set. so, considering these two methods seem to always be called together, i would rather not create the opportunity to have a bug in beforemethod order. > On Dec. 5, 2015, 2:28 p.m., Jarek Cecho wrote: > > test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java, > > lines 166-168 > > <https://reviews.apache.org/r/40895/diff/4/?file=1153630#file1153630line166> > > > > 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. so we already use the SqoopMiniClusterFactory in the SqoopInfrastructureProvider. I really do not see an easy way to run the JettyTestCase tests on a real cluster without it. is this change really worthy of its own jira? - Abraham ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40895/#review109085 ----------------------------------------------------------- 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 > >
