> On 二月 13, 2015, 11:54 p.m., Abraham Elmahrek wrote:
> > test/src/test/java/org/apache/sqoop/integration/repository/derby/upgrade/DerbyRepositoryUpgradeTest.java,
> >  line 52
> > <https://reviews.apache.org/r/30980/diff/1/?file=863011#file863011line52>
> >
> >     Leave as tomcat

Ok, I will fix it. Thanks for your advice


> On 二月 13, 2015, 11:54 p.m., Abraham Elmahrek wrote:
> > test/src/main/java/org/apache/sqoop/test/minicluster/JettySqoopRunner.java, 
> > line 259
> > <https://reviews.apache.org/r/30980/diff/1/?file=863008#file863008line259>
> >
> >     I don't think this is used?

Ok, I will fix it. Thanks for your advice.


> On 二月 13, 2015, 11:54 p.m., Abraham Elmahrek wrote:
> > test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java, 
> > line 58
> > <https://reviews.apache.org/r/30980/diff/1/?file=863009#file863009line58>
> >
> >     Extra space?

I am sorry for my mistake. I will fix it.


> On 二月 13, 2015, 11:54 p.m., Abraham Elmahrek wrote:
> > test/src/main/java/org/apache/sqoop/test/minicluster/JettySqoopMiniCluster.java,
> >  line 99
> > <https://reviews.apache.org/r/30980/diff/1/?file=863007#file863007line99>
> >
> >     Let's make this configurable so that strange dev. setups are supported.

Good advice. I will fix it.


> On 二月 13, 2015, 11:54 p.m., Abraham Elmahrek wrote:
> > test/src/main/java/org/apache/sqoop/test/minicluster/JettySqoopRunner.java, 
> > line 73
> > <https://reviews.apache.org/r/30980/diff/1/?file=863008#file863008line73>
> >
> >     This doesn't need to be volatile I think. "start" should be called from 
> > a single thread.

Thanks, I will fix it.


> On 二月 13, 2015, 11:54 p.m., Abraham Elmahrek wrote:
> > test/src/main/java/org/apache/sqoop/test/minicluster/JettySqoopRunner.java, 
> > line 166
> > <https://reviews.apache.org/r/30980/diff/1/?file=863008#file863008line166>
> >
> >     In our case, the port the server listens on will always be the same? 
> > Perhaps I'm misunderstanding something here.

Yeah, I will fix it.


> On 二月 13, 2015, 11:54 p.m., Abraham Elmahrek wrote:
> > test/src/main/java/org/apache/sqoop/test/minicluster/JettySqoopRunner.java, 
> > line 209
> > <https://reviews.apache.org/r/30980/diff/1/?file=863008#file863008line209>
> >
> >     Copy/paste error?
> >     
> >     Also, I'd add some doc describing what the arguments do here.

I am sorry for this simple mistake.


> On 二月 13, 2015, 11:54 p.m., Abraham Elmahrek wrote:
> > test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java, 
> > lines 181-214
> > <https://reviews.apache.org/r/30980/diff/1/?file=863010#file863010line181>
> >
> >     Let's pull these out into shared utils somewhere.

Good advice. I will fix it.


- shen


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30980/#review72452
-----------------------------------------------------------


On 二月 13, 2015, 7:35 a.m., shen guoquan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30980/
> -----------------------------------------------------------
> 
> (Updated 二月 13, 2015, 7:35 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> Currenly when the Tomcat Sqoop miniCluster started, it at firstly will need 
> to download the tomcat.tar.gz. When the Tomcat tar package has been 
> successfully downloaded, it will load the sqoop war package. That's is the 
> problem. For example, When the sentry want to do some integration tests with 
> sqoop, Sentry want to use the Tomcat Sqoop miniCluster to test, it must 
> download the sqoop war package in its project. It is not the correct way. The 
> correct method is that when Sentry want to do integration tests with Sqoop, 
> it only to do is add a maven dependency in the pom, then it can use a 
> independent sqoop miniCluster to test.
> 
> 
> Diffs
> -----
> 
>   dist/pom.xml e0bf0d0 
>   pom.xml d48389e 
>   server/pom.xml ee409cb 
>   test/pom.xml f743d25 
>   
> test/src/main/java/org/apache/sqoop/test/minicluster/JettySqoopMiniCluster.java
>  PRE-CREATION 
>   test/src/main/java/org/apache/sqoop/test/minicluster/JettySqoopRunner.java 
> PRE-CREATION 
>   test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java 
> 398a051 
>   test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java 
> PRE-CREATION 
>   
> test/src/test/java/org/apache/sqoop/integration/repository/derby/upgrade/DerbyRepositoryUpgradeTest.java
>  a687c16 
>   test/src/test/java/org/apache/sqoop/integration/server/VersionTest.java 
> a54492e 
> 
> Diff: https://reviews.apache.org/r/30980/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> shen guoquan
> 
>

Reply via email to