----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23917/#review50918 -----------------------------------------------------------
Looks good to me. Minor comments below. sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java <https://reviews.apache.org/r/23917/#comment88801> Thanks for adding check in the catch block Colin! That helps. Minor suggestion: Would be good to assert that exception was indeed thrown here. Otherwise test would pass even if the exception was not thrown. Like: try { statement.execute("CREATE ROLE " + roleName); fail("Exception should have been thrown"); } sentry-tests/sentry-tests-hive/src/test/resources/log4j.properties <https://reviews.apache.org/r/23917/#comment88802> These changes are not required in log4j.properties file, as you are setting the appenders etc from the test itself. - Sravya Tirukkovalur On Aug. 18, 2014, 1:45 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23917/ > ----------------------------------------------------------- > > (Updated Aug. 18, 2014, 1:45 a.m.) > > > Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya > Tirukkovalur. > > > Repository: sentry > > > Description > ------- > > The patch is for end to end test cases of audit log. > > > Diffs > ----- > > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java > PRE-CREATION > sentry-tests/sentry-tests-hive/src/test/resources/log4j.properties d941816 > > Diff: https://reviews.apache.org/r/23917/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
