> On 二月 3, 2015, 5:34 a.m., Colin Ma wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java,
> > line 823
> > <https://reviews.apache.org/r/30490/diff/1/?file=843274#file843274line823>
> >
> > Why remove the test case?
Good point! Because this test will broke thrift client. But after this test, I
still use the client in after().
In other words, the following code will failed too:
// null parameter name fails
try {
val = client.getConfigValue(null, null);
fail("null parameter succeeded");
} catch (SentryUserException e) {
// expected
}
+ // Basic success case
+ val = client.getConfigValue("sentry.service.admin.group", "xxx");
+ assertEquals(val, "admin_group");
In my opinion, this is a invalid test case and broke thrift, so I remove it.
- Xiaomeng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30490/#review70705
-----------------------------------------------------------
On 二月 3, 2015, 5:19 a.m., Xiaomeng Huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30490/
> -----------------------------------------------------------
>
> (Updated 二月 3, 2015, 5:19 a.m.)
>
>
> Review request for sentry and Colin Ma.
>
>
> Bugs: SENTRY-633
> https://issues.apache.org/jira/browse/SENTRY-633
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Currently some test cases extends SentryServiceIntegrationBase run every test
> will start services. We can start services at beforeClass to reduce test time.
> Before this patch, TestSentryServiceIntegration run 6:53.691s.
> After this patch, TestSentryServiceIntegration run 57.969s.
>
>
> Diffs
> -----
>
>
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceIntegration.java
> dfd9f10
>
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/SentryMiniKdcTestcase.java
> 79acb58
>
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestConnectionWithTicketTimeout.java
> af063cf
>
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerForHaWithoutKerberos.java
> 28233ee
>
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerWithoutKerberos.java
> cfa3f19
>
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java
> b97db4b
>
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceForHAWithKerberos.java
> 6b5cbf0
>
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java
> d4dfa23
>
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithKerberos.java
> ea666f1
>
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryWebServerWithKerberos.java
> 7997d6c
>
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryWebServerWithoutSecurity.java
> 0bcef1a
>
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java
> ca64ce1
>
> Diff: https://reviews.apache.org/r/30490/diff/
>
>
> Testing
> -------
>
> all test cases passed.
>
>
> Thanks,
>
> Xiaomeng Huang
>
>
