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 > >
