sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jarek Cecho" <jar...@apache.org>
Subject Re: Review Request 34509: Sqoop2: Provide test infrastructure base class for server tests
Date Thu, 09 Jul 2015 16:26:23 GMT

-----------------------------------------------------------
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 <abe@apache.org>
> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message