You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Jarek Cecho <ja...@apache.org> on 2013/02/04 06:55:31 UTC
Review Request: SQOOP-861 Sqoop2: Integration: Create basic integration
infrastructure
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9281/
-----------------------------------------------------------
Review request for Sqoop.
Description
-------
I've created basic infrastructure for integration tests. I'm using cargo for automatic installation and deployment of Sqoop server into Tomcat.
I've included first simple integration test that will connect to the Sqoop server and fetch the version.
This addresses bug SQOOP-861.
https://issues.apache.org/jira/browse/SQOOP-861
Diffs
-----
pom.xml 003ac43fe3dc7b8ecccf6d7e912901d02b5d0860
test/pom.xml PRE-CREATION
test/src/main/java/org/apache/sqoop/test/minicluster/InProcessSqoopMiniCluster.java PRE-CREATION
test/src/main/java/org/apache/sqoop/test/minicluster/SqoopMiniCluster.java PRE-CREATION
test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java PRE-CREATION
test/src/test/java/org/apache/sqoop/integration/TomcatTestCase.java PRE-CREATION
test/src/test/java/org/apache/sqoop/integration/server/VersionTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/9281/diff/
Testing
-------
mvn clean test # Run only unit tests
mvn clean integration-test # Run both unit and integration tests
Thanks,
Jarek Cecho
Re: Review Request: SQOOP-861 Sqoop2: Integration: Create basic integration
infrastructure
Posted by Cheolsoo Park <ch...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9281/#review16402
-----------------------------------------------------------
Ship it!
Ship It!
- Cheolsoo Park
On Feb. 9, 2013, 1:35 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9281/
> -----------------------------------------------------------
>
> (Updated Feb. 9, 2013, 1:35 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've created basic infrastructure for integration tests. I'm using cargo for automatic installation and deployment of Sqoop server into Tomcat.
>
> I've included first simple integration test that will connect to the Sqoop server and fetch the version.
>
>
> This addresses bug SQOOP-861.
> https://issues.apache.org/jira/browse/SQOOP-861
>
>
> Diffs
> -----
>
> pom.xml 003ac43fe3dc7b8ecccf6d7e912901d02b5d0860
> test/pom.xml PRE-CREATION
> test/src/main/java/org/apache/sqoop/test/minicluster/InProcessSqoopMiniCluster.java PRE-CREATION
> test/src/main/java/org/apache/sqoop/test/minicluster/SqoopMiniCluster.java PRE-CREATION
> test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java PRE-CREATION
> test/src/test/java/org/apache/sqoop/integration/TomcatTestCase.java PRE-CREATION
> test/src/test/java/org/apache/sqoop/integration/server/VersionTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9281/diff/
>
>
> Testing
> -------
>
> mvn clean test # Run only unit tests
> mvn clean integration-test # Run both unit and integration tests
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request: SQOOP-861 Sqoop2: Integration: Create basic integration
infrastructure
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9281/
-----------------------------------------------------------
(Updated Feb. 9, 2013, 1:35 a.m.)
Review request for Sqoop.
Changes
-------
I've incorporated Cheolsoo's changes.
Description
-------
I've created basic infrastructure for integration tests. I'm using cargo for automatic installation and deployment of Sqoop server into Tomcat.
I've included first simple integration test that will connect to the Sqoop server and fetch the version.
This addresses bug SQOOP-861.
https://issues.apache.org/jira/browse/SQOOP-861
Diffs (updated)
-----
pom.xml 003ac43fe3dc7b8ecccf6d7e912901d02b5d0860
test/pom.xml PRE-CREATION
test/src/main/java/org/apache/sqoop/test/minicluster/InProcessSqoopMiniCluster.java PRE-CREATION
test/src/main/java/org/apache/sqoop/test/minicluster/SqoopMiniCluster.java PRE-CREATION
test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java PRE-CREATION
test/src/test/java/org/apache/sqoop/integration/TomcatTestCase.java PRE-CREATION
test/src/test/java/org/apache/sqoop/integration/server/VersionTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/9281/diff/
Testing
-------
mvn clean test # Run only unit tests
mvn clean integration-test # Run both unit and integration tests
Thanks,
Jarek Cecho
Re: Review Request: SQOOP-861 Sqoop2: Integration: Create basic integration
infrastructure
Posted by Jarek Cecho <ja...@apache.org>.
> On Feb. 6, 2013, 8:34 p.m., Cheolsoo Park wrote:
> >
Hi Cheolsoo,
thank you very much for your feedback.
> On Feb. 6, 2013, 8:34 p.m., Cheolsoo Park wrote:
> > test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java, line 111
> > <https://reviews.apache.org/r/9281/diff/1/?file=255001#file255001line111>
> >
> > I can see that tomcat from a previous run gets deleted before running tests, so I am not too worried that stale files will be an issue.
> >
> > Nonetheless, wouldn't it better to clean up installed tomcat at the end of tests? I prefer that each test cleans up their mess after they're finished because stale files might effect other tests that happen to use cargo to install tomcat under /tmp although it's not likely.
Thank you for catching this, I was actually planning to fix that in follow up jira. I'm keeping entire "state" in tmp directory right now. Tomcat itself be cleaned up by cargo.
I do not want to nuke the tmp directory after each test as it contain very important logs (both tomcat and sqoop server). Instead, I'll move the temporary directory into test/target so it will get cleaned up on "mvn clean" (but preserved after the test
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9281/#review16200
-----------------------------------------------------------
On Feb. 4, 2013, 5:55 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9281/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2013, 5:55 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've created basic infrastructure for integration tests. I'm using cargo for automatic installation and deployment of Sqoop server into Tomcat.
>
> I've included first simple integration test that will connect to the Sqoop server and fetch the version.
>
>
> This addresses bug SQOOP-861.
> https://issues.apache.org/jira/browse/SQOOP-861
>
>
> Diffs
> -----
>
> pom.xml 003ac43fe3dc7b8ecccf6d7e912901d02b5d0860
> test/pom.xml PRE-CREATION
> test/src/main/java/org/apache/sqoop/test/minicluster/InProcessSqoopMiniCluster.java PRE-CREATION
> test/src/main/java/org/apache/sqoop/test/minicluster/SqoopMiniCluster.java PRE-CREATION
> test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java PRE-CREATION
> test/src/test/java/org/apache/sqoop/integration/TomcatTestCase.java PRE-CREATION
> test/src/test/java/org/apache/sqoop/integration/server/VersionTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9281/diff/
>
>
> Testing
> -------
>
> mvn clean test # Run only unit tests
> mvn clean integration-test # Run both unit and integration tests
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request: SQOOP-861 Sqoop2: Integration: Create basic integration
infrastructure
Posted by Cheolsoo Park <ch...@cloudera.com>.
> On Feb. 6, 2013, 8:34 p.m., Cheolsoo Park wrote:
> > test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java, line 111
> > <https://reviews.apache.org/r/9281/diff/1/?file=255001#file255001line111>
> >
> > I can see that tomcat from a previous run gets deleted before running tests, so I am not too worried that stale files will be an issue.
> >
> > Nonetheless, wouldn't it better to clean up installed tomcat at the end of tests? I prefer that each test cleans up their mess after they're finished because stale files might effect other tests that happen to use cargo to install tomcat under /tmp although it's not likely.
>
> Jarek Cecho wrote:
> Thank you for catching this, I was actually planning to fix that in follow up jira. I'm keeping entire "state" in tmp directory right now. Tomcat itself be cleaned up by cargo.
>
> I do not want to nuke the tmp directory after each test as it contain very important logs (both tomcat and sqoop server). Instead, I'll move the temporary directory into test/target so it will get cleaned up on "mvn clean" (but preserved after the test
Sounds fine to me. Thanks for the clarification.
- Cheolsoo
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9281/#review16200
-----------------------------------------------------------
On Feb. 9, 2013, 1:35 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9281/
> -----------------------------------------------------------
>
> (Updated Feb. 9, 2013, 1:35 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've created basic infrastructure for integration tests. I'm using cargo for automatic installation and deployment of Sqoop server into Tomcat.
>
> I've included first simple integration test that will connect to the Sqoop server and fetch the version.
>
>
> This addresses bug SQOOP-861.
> https://issues.apache.org/jira/browse/SQOOP-861
>
>
> Diffs
> -----
>
> pom.xml 003ac43fe3dc7b8ecccf6d7e912901d02b5d0860
> test/pom.xml PRE-CREATION
> test/src/main/java/org/apache/sqoop/test/minicluster/InProcessSqoopMiniCluster.java PRE-CREATION
> test/src/main/java/org/apache/sqoop/test/minicluster/SqoopMiniCluster.java PRE-CREATION
> test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java PRE-CREATION
> test/src/test/java/org/apache/sqoop/integration/TomcatTestCase.java PRE-CREATION
> test/src/test/java/org/apache/sqoop/integration/server/VersionTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9281/diff/
>
>
> Testing
> -------
>
> mvn clean test # Run only unit tests
> mvn clean integration-test # Run both unit and integration tests
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request: SQOOP-861 Sqoop2: Integration: Create basic integration
infrastructure
Posted by Cheolsoo Park <ch...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9281/#review16200
-----------------------------------------------------------
test/pom.xml
<https://reviews.apache.org/r/9281/#comment34630>
Do you need the version here?
test/src/main/java/org/apache/sqoop/test/minicluster/SqoopMiniCluster.java
<https://reviews.apache.org/r/9281/#comment34634>
Can you create constants for temporaryPath + "/config" and temporaryPath + "/log" and reuse them instead duplicate them in the rest of code?
test/src/main/java/org/apache/sqoop/test/minicluster/SqoopMiniCluster.java
<https://reviews.apache.org/r/9281/#comment34635>
Wouldn't it better to use writeLines() instead of doing a file I/O for every line?
For example,
String[] contents = {
"1st line",
"2nd line",
...
}
FileUtils.writeLines(f, null, Array.asList(contents), true);
test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java
<https://reviews.apache.org/r/9281/#comment34636>
I can see that tomcat from a previous run gets deleted before running tests, so I am not too worried that stale files will be an issue.
Nonetheless, wouldn't it better to clean up installed tomcat at the end of tests? I prefer that each test cleans up their mess after they're finished because stale files might effect other tests that happen to use cargo to install tomcat under /tmp although it's not likely.
- Cheolsoo Park
On Feb. 4, 2013, 5:55 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9281/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2013, 5:55 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've created basic infrastructure for integration tests. I'm using cargo for automatic installation and deployment of Sqoop server into Tomcat.
>
> I've included first simple integration test that will connect to the Sqoop server and fetch the version.
>
>
> This addresses bug SQOOP-861.
> https://issues.apache.org/jira/browse/SQOOP-861
>
>
> Diffs
> -----
>
> pom.xml 003ac43fe3dc7b8ecccf6d7e912901d02b5d0860
> test/pom.xml PRE-CREATION
> test/src/main/java/org/apache/sqoop/test/minicluster/InProcessSqoopMiniCluster.java PRE-CREATION
> test/src/main/java/org/apache/sqoop/test/minicluster/SqoopMiniCluster.java PRE-CREATION
> test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java PRE-CREATION
> test/src/test/java/org/apache/sqoop/integration/TomcatTestCase.java PRE-CREATION
> test/src/test/java/org/apache/sqoop/integration/server/VersionTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9281/diff/
>
>
> Testing
> -------
>
> mvn clean test # Run only unit tests
> mvn clean integration-test # Run both unit and integration tests
>
>
> Thanks,
>
> Jarek Cecho
>
>