----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34509/#review85028 -----------------------------------------------------------
It seems that by moving a lot of code from ConnectorTestCase to standalone Util classes, one have to do much more to achieve the same functionality (initialize the util classes, call methods on the instances, ...). Overall I feel that the readability is rather decreasing. Hence I'm wondering if it would make sense to let those methods live in the parent test case or perhaps provide some facilities to avoid the decreased redability and copy&paste that will be required? Perhaps we can initalize the util classes in parent class and rather then instantiating the util class we can have all it's method static and import all static methods? We're doing something similar in shell package with ShellEnvironment class: https://github.com/apache/sqoop/blob/sqoop2/shell/src/main/java/org/apache/sqoop/shell/ShellEnvironment.java test/src/main/java/org/apache/sqoop/test/minicluster/SqoopMiniCluster.java (lines 214 - 216) <https://reviews.apache.org/r/34509/#comment136502> I'm wondering if it would made sense to define this method as abstract as it doesn't make sense without "concrete" implementation that will actually fill where the server is running? - Jarek Cecho On May 20, 2015, 11:04 p.m., Abraham Elmahrek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34509/ > ----------------------------------------------------------- > > (Updated May 20, 2015, 11:04 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-2364 > https://issues.apache.org/jira/browse/SQOOP-2364 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > commit d5117886e7be68c6a40fb28cd4c0020fab977052 > Author: Abraham Elmahrek <[email protected]> > Date: Fri May 15 16:10:17 2015 -0700 > > SQOOP-2364: Sqoop2: Provide test infrastructure base class for server > tests > > :100644 100644 758eb2f... 6a286ae... M > test/src/main/java/org/apache/sqoop/test/minicluster/SqoopMiniCluster.java > :100644 100644 5a6773d... 38444c0... M > test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java > :100644 100644 79700a6... 786cf40... M > test/src/main/java/org/apache/sqoop/test/utils/HdfsUtils.java > :000000 100644 0000000... f4eeb88... A > test/src/main/java/org/apache/sqoop/test/utils/RdbmsUtils.java > :000000 100644 0000000... 2aeeb72... A > test/src/main/java/org/apache/sqoop/test/utils/SqoopUtils.java > :100644 100644 40dc7d7... a1858d7... M > test/src/test/java/org/apache/sqoop/integration/server/SubmissionWithDisabledModelObjectsTest.java > :100644 100644 a54492e... a14d12e... M > test/src/test/java/org/apache/sqoop/integration/server/VersionTest.java > > > Diffs > ----- > > test/src/main/java/org/apache/sqoop/test/minicluster/SqoopMiniCluster.java > 758eb2f > > test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java > 5a6773d > test/src/main/java/org/apache/sqoop/test/utils/HdfsUtils.java 79700a6 > test/src/main/java/org/apache/sqoop/test/utils/RdbmsUtils.java PRE-CREATION > test/src/main/java/org/apache/sqoop/test/utils/SqoopUtils.java PRE-CREATION > > test/src/test/java/org/apache/sqoop/integration/server/SubmissionWithDisabledModelObjectsTest.java > 40dc7d7 > test/src/test/java/org/apache/sqoop/integration/server/VersionTest.java > a54492e > > Diff: https://reviews.apache.org/r/34509/diff/ > > > Testing > ------- > > mvn clean integration-test (but only for the server tests) > > > Thanks, > > Abraham Elmahrek > >
