[ 
https://issues.apache.org/jira/browse/HDFS-9339?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14980906#comment-14980906
 ] 

Zhe Zhang commented on HDFS-9339:
---------------------------------

Thanks Daniel! The patch looks pretty good. A few minor comments:
# Maybe we should improve the criteria in assertions?
{code}
try {
  setup(conf);

  assertTrue("Exception during key creation with correct config"
      + " using whitelist key ACLs", createKey(realUgi, KEY1, conf));
} finally {
  teardown();
}
{code}
The test will pass if {{createKey}} throws an exception. A simple fix is to 
{{fail()}} in the {{finally}} statement. But maybe there's a way to avoid 
adding it in every {{finally}} statement.
# Since the above {{try-finally}} logic repeats many times we can also abstract 
it out as a method.
# Looks like the {{// Correct config with blacklist}} {{//Missing GET_METADATA 
KMS ACL}} sections are both repeated twice?

> Extend full test of KMS ACLs
> ----------------------------
>
>                 Key: HDFS-9339
>                 URL: https://issues.apache.org/jira/browse/HDFS-9339
>             Project: Hadoop HDFS
>          Issue Type: Test
>          Components: HDFS
>    Affects Versions: 2.7.1
>            Reporter: Daniel Templeton
>            Assignee: Daniel Templeton
>         Attachments: HDFS-9339.001.patch
>
>
> HDFS-9295 adds an end-to-end test for KMS, but it is missing a dimension.  
> The tests added in that JIRA hold the configuration constant and test that 
> all operations succeed or fail as expected.  More tests are needed that hold 
> the operation constant and test that all possible configurations cause the 
> operations to succeed or fail as expected.  This JIRA is to add those tests.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to