> On Nov. 25, 2015, 3:18 a.m., Colin Ma wrote:
> > Thanks Dian for the patch, Overall looks great.
> > A few comments here, I think the there should be some new test cases for 
> > KdcInfrastructureProvider, not add KdcInfrastructureProvider to the exist 
> > case. This will reduce the test coverage, eg, some test cases without 
> > kerberos are disappear.
> > Just for reference:
> > 
> > @Infrastructure(dependencies =...............)
> > public class InformalObjectNameWithKerbosTest extends 
> > InformalObjectNameTest {
> >    ........
> > }

Hi Colin,
Thanks a lot for the review.
1) Adding KdcInfrastructureProvider to the existing test cases will ensure that 
the existing test cases running in kerberos environment and they will pass if 
and only if the kerberos authentication passed and the tested functionality 
passed. 
2) Adding a wrapper for each test case to make it both tested in "kerberos" 
environment and "simple" environment is not necessary from my point of view. If 
a test case passes in "kerberos" environment, it should also pass in "simple" 
test unless the test case is writen specially to test the "simple" 
authentication. In this case, we can disable the "kerberos" test for that 
specific test case.
3) Actually the kerberos tests can be disabled by configure environment 
variable "sqoop.kdc.runner.class" as "org.apache.sqoop.test.kdc.RealKdcRunner". 
But I understand that it's not easy to integrate into the jenkins.


- Dian


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


On Nov. 25, 2015, 4:50 a.m., Dian Fu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40635/
> -----------------------------------------------------------
> 
> (Updated Nov. 25, 2015, 4:50 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2705
>     https://issues.apache.org/jira/browse/SQOOP-2705
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> At the first step of providing kerberos support for integration tests, I'd 
> like to add kerberos support for SqoopMiniCluster make sure the communication 
> between sqoop client and sqoop server are kerberos enabled.
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/sqoop/client/SqoopClient.java 3d3425d 
>   
> client/src/main/java/org/apache/sqoop/client/request/SqoopResourceRequests.java
>  0987703 
>   pom.xml 91721ce 
>   test/pom.xml 4e1e197 
>   test/src/main/java/org/apache/sqoop/test/infrastructure/SqoopTestCase.java 
> 4c5d3a8 
>   
> test/src/main/java/org/apache/sqoop/test/infrastructure/providers/KdcInfrastructureProvider.java
>  PRE-CREATION 
>   test/src/main/java/org/apache/sqoop/test/kdc/KdcRunner.java PRE-CREATION 
>   test/src/main/java/org/apache/sqoop/test/kdc/KdcRunnerFactory.java 
> PRE-CREATION 
>   test/src/main/java/org/apache/sqoop/test/kdc/MiniKdcRunner.java 
> PRE-CREATION 
>   test/src/main/java/org/apache/sqoop/test/kdc/RealKdcRunner.java 
> PRE-CREATION 
>   
> test/src/main/java/org/apache/sqoop/test/minicluster/JettySqoopMiniCluster.java
>  325a790 
>   test/src/main/java/org/apache/sqoop/test/minicluster/SqoopMiniCluster.java 
> 9ae941f 
>   test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java 
> bd4ba6a 
>   test/src/main/java/org/apache/sqoop/test/utils/KerberosTestUtils.java 
> PRE-CREATION 
>   
> test/src/test/java/org/apache/sqoop/integration/server/InformalObjectNameTest.java
>  920679f 
>   
> test/src/test/java/org/apache/sqoop/integration/server/ShowJobInOrderTest.java
>  cbf1e90 
>   
> test/src/test/java/org/apache/sqoop/integration/server/SubmissionWithDisabledModelObjectsTest.java
>  9e682bc 
>   test/src/test/java/org/apache/sqoop/integration/server/VersionTest.java 
> e86a6cc 
>   
> test/src/test/java/org/apache/sqoop/integration/server/rest/LinkRestTest.java 
> 99959ac 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/RestTest.java 
> 4ac564c 
> 
> Diff: https://reviews.apache.org/r/40635/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dian Fu
> 
>

Reply via email to