You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Colin Ma <ju...@intel.com> on 2015/01/28 09:44:12 UTC
Review Request 30360: SENTRY-625: Improve test cases in
"TestPrivilegesAtColumnScope"
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30360/
-----------------------------------------------------------
Review request for sentry.
Repository: sentry
Description
-------
Many test cases include the "join" case, this will submit a mapreduce job and take a lot of time. Remove the unnecessary test cases with "join" case.
Diffs
-----
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 689f5a6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtColumnScope.java 4e43046
Diff: https://reviews.apache.org/r/30360/diff/
Testing
-------
Thanks,
Colin Ma
Re: Review Request 30360: SENTRY-625: Improve test cases in
"TestPrivilegesAtColumnScope"
Posted by Xiaomeng Huang <xi...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30360/#review70891
-----------------------------------------------------------
Ship it!
Ship It!
- Xiaomeng Huang
On 二月 4, 2015, 1:36 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30360/
> -----------------------------------------------------------
>
> (Updated 二月 4, 2015, 1:36 a.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Many test cases include the "join" case, this will submit a mapreduce job and take a lot of time. Remove the unnecessary test cases with "join" case.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegesAtColumnScope.java a4de2c0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 689f5a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtColumnScope.java 4e43046
>
> Diff: https://reviews.apache.org/r/30360/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 30360: SENTRY-625: Improve test cases in
"TestPrivilegesAtColumnScope"
Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30360/
-----------------------------------------------------------
(Updated Feb. 4, 2015, 1:36 a.m.)
Review request for sentry.
Repository: sentry
Description
-------
Many test cases include the "join" case, this will submit a mapreduce job and take a lot of time. Remove the unnecessary test cases with "join" case.
Diffs (updated)
-----
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegesAtColumnScope.java a4de2c0
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 689f5a6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtColumnScope.java 4e43046
Diff: https://reviews.apache.org/r/30360/diff/
Testing
-------
Thanks,
Colin Ma
Re: Review Request 30360: SENTRY-625: Improve test cases in
"TestPrivilegesAtColumnScope"
Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30360/
-----------------------------------------------------------
(Updated Jan. 29, 2015, 8:39 a.m.)
Review request for sentry.
Repository: sentry
Description
-------
Many test cases include the "join" case, this will submit a mapreduce job and take a lot of time. Remove the unnecessary test cases with "join" case.
Diffs (updated)
-----
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegesAtColumnScope.java a4de2c0
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 689f5a6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtColumnScope.java 4e43046
Diff: https://reviews.apache.org/r/30360/diff/
Testing
-------
Thanks,
Colin Ma
Re: Review Request 30360: SENTRY-625: Improve test cases in
"TestPrivilegesAtColumnScope"
Posted by Colin Ma <ju...@intel.com>.
On Jan. 29, 2015, 7:36 a.m., Colin Ma wrote:
> > Hi, Colin. I see your patch has two part:
> > 1. refactor test case.
> > 2. remove join test case.
> >
> > As for "(1)refactor test case", I once refactored all test cases to reduce setup plicy file and clean DB. For example, set up policy file in @beforeClass, clean DB in @afterClass. After refactoring all test cases, I tested these modification have little effect on cutting down test time. Because clean DB and write policy file are very fact. So I think we don't need take effort on that point. I fact, for every test case, we setup policy in @before, and clean DB in @after, it will make we write test case more convenient and flexible. We don't need to handle what's privilege has had, and relation between two test case.
> >
> > As for "(2)remove join test case", I think for e2e test, we also should add these join tests, there are very common and important usages for customers. Shouldn't we test that?
hi, Xiaomeng, thanks for the comments:
1. I agree that Clear DB won't spend much time, but load data to table will spend a lot of time. It's better to load data once, and use the data in every test method.
2. The column authorization for the "join" and "where" should focus on the part of the condition, eg, "JOIN TAB_2 T2 *ON T1.B = T2.B*", "where *T1.B = T2.B*". The test of select part shouldn't be included in join test cases.
- Colin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30360/#review70172
-----------------------------------------------------------
On Jan. 29, 2015, 7:07 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30360/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2015, 7:07 a.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Many test cases include the "join" case, this will submit a mapreduce job and take a lot of time. Remove the unnecessary test cases with "join" case.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegesAtColumnScope.java a4de2c0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 689f5a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtColumnScope.java 4e43046
>
> Diff: https://reviews.apache.org/r/30360/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 30360: SENTRY-625: Improve test cases in
"TestPrivilegesAtColumnScope"
Posted by Xiaomeng Huang <xi...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30360/#review70172
-----------------------------------------------------------
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtColumnScope.java
<https://reviews.apache.org/r/30360/#comment115210>
Clear DB after testing will not cost mush time. In fact, if we have clearDB in @After, it makes us write test case more convenient and flexible.
Hi, Colin. I see your patch has two part:
1. refactor test case.
2. remove join test case.
As for "(1)refactor test case", I once refactored all test cases to reduce setup plicy file and clean DB. For example, set up policy file in @beforeClass, clean DB in @afterClass. After refactoring all test cases, I tested these modification have little effect on cutting down test time. Because clean DB and write policy file are very fact. So I think we don't need take effort on that point. I fact, for every test case, we setup policy in @before, and clean DB in @after, it will make we write test case more convenient and flexible. We don't need to handle what's privilege has had, and relation between two test case.
As for "(2)remove join test case", I think for e2e test, we also should add these join tests, there are very common and important usages for customers. Shouldn't we test that?
- Xiaomeng Huang
On 一月 29, 2015, 7:07 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30360/
> -----------------------------------------------------------
>
> (Updated 一月 29, 2015, 7:07 a.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Many test cases include the "join" case, this will submit a mapreduce job and take a lot of time. Remove the unnecessary test cases with "join" case.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegesAtColumnScope.java a4de2c0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 689f5a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtColumnScope.java 4e43046
>
> Diff: https://reviews.apache.org/r/30360/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 30360: SENTRY-625: Improve test cases in
"TestPrivilegesAtColumnScope"
Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30360/
-----------------------------------------------------------
(Updated Jan. 29, 2015, 7:07 a.m.)
Review request for sentry.
Repository: sentry
Description
-------
Many test cases include the "join" case, this will submit a mapreduce job and take a lot of time. Remove the unnecessary test cases with "join" case.
Diffs (updated)
-----
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegesAtColumnScope.java a4de2c0
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 689f5a6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtColumnScope.java 4e43046
Diff: https://reviews.apache.org/r/30360/diff/
Testing
-------
Thanks,
Colin Ma