Xiaoyu Yao commented on HDFS-10922:

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

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

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
    Path user1Tmp = Path.mergePaths(TEST_ROOT, new Path("/test-del-u1"));

    Path user1Tmp = new Path(TEST_ROOT, "test-del-u1");

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

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

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