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

Xiaoyu Yao commented on HDFS-10922:
-----------------------------------

[~cheersyang], thanks for the update. It looks good to me overall. Here are my 
comments:

1. Can we replace the "import static org.junit.Assert.*" with
{code}
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
{code}

2. NIT: can we replace usage of "DFSConfigKeys.FS_DEFAULT_NAME_KEY" with 
"CommonConfigurationKeys.FS_TRASH_INTERVAL_KEY" for consistency. 

3. Many of the mergePaths call is not necessary. It can be simplified like below
{code}
    Path user1Tmp = Path.mergePaths(TEST_ROOT, new Path("/test-del-u1"));
{code}

to 
{code}
    Path user1Tmp = new Path(TEST_ROOT, "test-del-u1");
{code}

4. Can we remove the {{e.printStackTrace();}} from 
testMoveUnprivilegedDirToTrash()?

5. TestHDFSTrash#login() can we move it into DFSTestUtil  for reusing in both 
TestHDFSTrash and TestDFSPermission?
The conf will need to be a parameter as we usually change configuration with a 
new configuration instance during the test. 

6. TestHDFSTrash#verifyFilePermission can be moved into DFSTestUtil.



> Adding additional unit tests for Trash (II)
> -------------------------------------------
>
>                 Key: HDFS-10922
>                 URL: https://issues.apache.org/jira/browse/HDFS-10922
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: test
>            Reporter: Xiaoyu Yao
>            Assignee: Weiwei Yang
>         Attachments: HDFS-10922.02.patch, HDFS-10922.03.patch, 
> HDFS-10922.04.patch, HDFS-10922.05.patch, HDFS-10922.06.patch, 
> HDFS-10922.07.patch, HDFS-10922.08.patch, HDFS-10922.09.patch, 
> HDFS-10922.10.patch, HDFS-10922.11.patch
>
>
> This ticket is opened to track adding unit tests for Trash.



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

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to