[GitHub] [accumulo] cshannon commented on a diff in pull request #3001: WIP - Address TODOs in PermissionsIT

[GitBox <gi...@apache.org>]

cshannon commented on code in PR #3001:
URL: https://github.com/apache/accumulo/pull/3001#discussion_r990133359


##########
test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java:
##########
@@ -419,7 +421,14 @@ private void testMissingSystemPermission(String tableNamePrefix, AccumuloClient
         break;
       case OBTAIN_DELEGATION_TOKEN:
         if (saslEnabled()) {

Review Comment:
   It doesn't appear that SASL is actually enabled in this test so this code never executes which I'm sure is why the TODO is there in the first place and why the test seems to work.
   
   SASL is enabled by setting static system properties that are checked in `AccumuloClusterHarness` so there may be some work here to only enable it for that one check or maybe it's simplest to create a new test class (or one that extends this) that sets the property to enable it so the `TestingKdc` is initialized and this can be tested.



##########
test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java:
##########
@@ -592,7 +601,16 @@ private void testGrantedSystemPermission(String tableNamePrefix, AccumuloClient
         break;
       case OBTAIN_DELEGATION_TOKEN:
         if (saslEnabled()) {

Review Comment:
   Same comment as above



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org