You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "Xiaoyu Yao (JIRA)" <ji...@apache.org> on 2016/10/10 21:29:20 UTC
[jira] [Comment Edited] (HADOOP-13686) Adding additional unit test
for Trash (I)
[ https://issues.apache.org/jira/browse/HADOOP-13686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15563561#comment-15563561 ]
Xiaoyu Yao edited comment on HADOOP-13686 at 10/10/16 9:29 PM:
---------------------------------------------------------------
Thanks [~Weiwei Yang] for working on this. The patch looks good to me overall. Here are some early feedbacks.
1. testMoveEmptyDirToTrash
Can we add a helper method by passing a FileSystem obj as parameter
so that this test can be used to test Trash with not only raw file system but also
other HCFS?
Can we further verify that the only directory under trash is the empty directory?
verifyDefaultPolicyIntervalValues
{{FileSystem fs = null;}} can be removed.
2. testTrashPermission
Can we add a helper method by passing a FileSystem obj as parameter
so that this test can be used to test Trash with not only raw file system but also
other HCFS?
3. NIT: Can we use try with resource to simplify the logic?
{code}
try {
} finally {
698 if(fs != null) {
699 fs.close();
700 }
701 }
{code}
4. Let's move AuditableTrashPolicy/AuditableCheckpoints in a separate file for reusing with HDFS-10922.
5. NIT: AuditableCheckpoints: can be a static class. But I would suggest we
declare the members var/methods to be non-static. This can avoid issues when
running multiple AuditableTrashPolicy instances.
was (Author: xyao):
Thanks [~Weiwei Yang] for working on this. The patch looks good to me overall. Here are some early feedbacks.
1. testMoveEmptyDirToTrash
Can we add a helper method by passing a FileSystem obj as parameter
so that this test can be used to test Trash with not only raw file system but also
other HCFS?
Can we further verify that the only directory under trash is the empty directory?
verifyDefaultPolicyIntervalValues
{{FileSystem fs = null;}} can be removed.
2. testTrashPermission
Can we add a helper method by passing a FileSystem obj as parameter
so that this test can be used to test Trash with not only raw file system but also
other HCFS?
3. NIT: Can we use try with resource to simplify the logic?
{code}
try {
} finally {
698 if(fs != null) {
699 fs.close();
700 }
701 }
{code}
4. NIT: AuditableCheckpoints: can be a static inner class. But I would suggest we
declare the members var/methods to be non-static. This can avoid issues when
running multiple AuditableTrashPolicy instances.
> Adding additional unit test for Trash (I)
> -----------------------------------------
>
> Key: HADOOP-13686
> URL: https://issues.apache.org/jira/browse/HADOOP-13686
> Project: Hadoop Common
> Issue Type: Test
> Reporter: Xiaoyu Yao
> Assignee: Weiwei Yang
> Attachments: HADOOP-13686.01.patch
>
>
> This ticket is opened to track adding the forllowing unit test in hadoop-common.
> #test users can delete their own trash directory
> #test users can delete an empty directory and the directory is moved to trash
> #test fs.trash.interval with invalid values such as 0 or negative
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org