Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )
Change subject: IMPALA-7764: Improve SentryProxy test coverage ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11880/1/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java File fe/src/test/java/org/apache/impala/util/SentryProxyTest.java: http://gerrit.cloudera.org:8080/#/c/11880/1/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@89 PS1, Line 89: public void testRefreshRolePrivileges() throws ImpalaException { this is a great improvement to make the proxy easier to test. currently, there seems to be a fair bit of boiler-plate to setup state on the catalog, on sentry, then check. perhaps this can be abstracted a bit, something like this: setCatalogRoles( [ {ADD, updateRole, ["functional"]}, {ADD, removeRole, ["functional"]} ]) setSentryStateRoles( [ {ADD, addRole, ["functional"]}, {ADD, updateRole, ["functional", "functional_kudu"]}, {DROP, removeRole}]) refresh() checkCatalogRoles([{updateRole, ["functional", "functional_kudu"]}, {addRole, ["functional"]}]) representation can differ a bit, but the idea is to make it easier to try different combinations. examples: - explicit test for empty case - explicit tests for add, delete, update - non-additive updates - non-empty state to empty state to non-empty state - sentry drop of a role not in the catalog - any limits for number of roles or privs? this operation is basically trying to merge two collections, so coverage of these various states would be useful. -- To view, visit http://gerrit.cloudera.org:8080/11880 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db Gerrit-Change-Number: 11880 Gerrit-PatchSet: 1 Gerrit-Owner: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Adam Holley <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Thu, 08 Nov 2018 00:51:02 +0000 Gerrit-HasComments: Yes
