You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Xiaomeng Huang <xi...@intel.com> on 2015/02/03 06:19:22 UTC
Re: Review Request 30490: SENTRY-633: Refactor
SentryServiceIntegrationBase to reduce test time
-----------------------------------------------------------
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.
Summary (updated)
-----------------
SENTRY-633: Refactor SentryServiceIntegrationBase to reduce test time
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
Re: Review Request 30490: SENTRY-633: Refactor
SentryServiceIntegrationBase to reduce test time
Posted by Xiaomeng Huang <xi...@intel.com>.
> 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
>
>
Re: Review Request 30490: SENTRY-633: Refactor
SentryServiceIntegrationBase to reduce test time
Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30490/#review70705
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java
<https://reviews.apache.org/r/30490/#comment116056>
Why remove the test case?
- Colin Ma
On Feb. 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 Feb. 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
>
>