----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63958/#review191638 -----------------------------------------------------------
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingBase.java Lines 54 (patched) <https://reviews.apache.org/r/63958/#comment269478> could these functions be in base class TestHDFSIntegrationBase? They are in TestDbPrivilegeCleanupOnDrop also sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingBase.java Lines 91 (patched) <https://reviews.apache.org/r/63958/#comment269479> Can we move this function to base too? sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingBase.java Lines 117 (patched) <https://reviews.apache.org/r/63958/#comment269480> We can combine these two into assertEqual(count, privilegeCount). If they are not equal, the exception will show the expected value and actual value. sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutHdfsSync.java Lines 90 (patched) <https://reviews.apache.org/r/63958/#comment269482> If HDFS sync is not enabled, HDFS does not get ACL from sync with sentry. how could HDFS gets acl in this case? sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutSyncOnCreate.java Lines 81 (patched) <https://reviews.apache.org/r/63958/#comment269483> do you still need this? The previous grant is not removed. sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java Lines 27-29 (patched) <https://reviews.apache.org/r/63958/#comment269484> We can avoid most duplicated code if you call "TestHDFSIntegrationBase.setup();" first, and then change the settings for each child class. - Na Li On Nov. 21, 2017, 12:27 a.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63958/ > ----------------------------------------------------------- > > (Updated Nov. 21, 2017, 12:27 a.m.) > > > Review request for sentry, Na Li and Sergio Pena. > > > Bugs: SENTRY-2034 > https://issues.apache.org/jira/browse/SENTRY-2034 > > > Repository: sentry > > > Description > ------- > > Currently, there are no e2e tests that test the functionality of pulling the > notifications from HMS and processing them. Which include updating the update > the permissions based on the HMS updates. > > > Diffs > ----- > > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java > 2138304 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessing.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingBase.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutHdfsSync.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutSyncOnCreate.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutSyncOnDrop.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java > 33ace57 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java > 25a678b > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java > 645fc35 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java > a911e2f > > > Diff: https://reviews.apache.org/r/63958/diff/2/ > > > Testing > ------- > > Made sure that new tests added and all the older tests pass. > > > Thanks, > > kalyan kumar kalvagadda > >
