----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33570/#review81633 -----------------------------------------------------------
test/src/main/java/org/apache/sqoop/test/testcases/TomcatTestCase.java <https://reviews.apache.org/r/33570/#comment132062> Nit: I'm wondering why removing the more detailed descrption? Is it because it's no longer valid? test/src/main/java/org/apache/sqoop/test/testcases/TomcatTestCase.java <https://reviews.apache.org/r/33570/#comment132063> I'm wondering why we are changing the visilibyt from private to protected? (I understand the addition of "static") test/src/main/java/org/apache/sqoop/test/testcases/TomcatTestCase.java <https://reviews.apache.org/r/33570/#comment132064> I'm wondering why we are changing the visilibyt from private to protected? (I understand the addition of "static") test/src/test/java/org/apache/sqoop/integration/repository/derby/upgrade/DerbyRepositoryUpgradeTest.java <https://reviews.apache.org/r/33570/#comment132067> I do not particularly like this pattern when we are prohibit parent class to do something that it was designed to do. Do you think that it would make sense to split the TomcatTestCase to two classes: * TomcatTestCaseBase that will contain all variables/help methods but won't do any action. Then this DerbyRepositoryUpgradeTest class will inherit from the new base class. * TomatTestCase that will inherit from Base and will add the automatic actions of starting/stopping required runners. I know that you're thinking about improving the integration test suite by using annotations to start/stop only those runners that are required, so perhaps my note is not so much relevant as this is just a "temporary" workaround. - Jarek Cecho On April 26, 2015, 7 p.m., Abraham Elmahrek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33570/ > ----------------------------------------------------------- > > (Updated April 26, 2015, 7 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1953 > https://issues.apache.org/jira/browse/SQOOP-1953 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > commit 1250abe418f343ab0a2ffa010a2a15a97befb385 > Author: Abraham Elmahrek <[email protected]> > Date: Fri Apr 17 17:34:15 2015 -0700 > > SQOOP-1953: Tomcat in suite > > :100644 100644 bc2bec7... 6e5e038... M pom.xml > :100644 100644 98a60f6... a9502d2... M test/pom.xml > :100644 100644 4d27886... 5a6773d... M > test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java > :100644 100644 6729cc7... 197dab4... M > test/src/main/java/org/apache/sqoop/test/testcases/TomcatTestCase.java > :100644 100644 a687c16... 7ad3dc2... M > test/src/test/java/org/apache/sqoop/integration/repository/derby/upgrade/DerbyRepositoryUpgrade > :100644 100644 101b6ec... f0dd905... M > test/src/test/resources/integration-tests-suite.xml > :000000 100644 0000000... 2856556... A > test/src/test/resources/upgrade-tests-suite.xml > > > Diffs > ----- > > pom.xml bc2bec7 > test/pom.xml 98a60f6 > > test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java > 4d27886 > test/src/main/java/org/apache/sqoop/test/testcases/TomcatTestCase.java > 6729cc7 > > test/src/test/java/org/apache/sqoop/integration/repository/derby/upgrade/DerbyRepositoryUpgradeTest.java > a687c16 > test/src/test/resources/integration-tests-suite.xml 101b6ec > test/src/test/resources/upgrade-tests-suite.xml PRE-CREATION > > Diff: https://reviews.apache.org/r/33570/diff/ > > > Testing > ------- > > > Thanks, > > Abraham Elmahrek > >
