[
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: [email protected]
For additional commands, e-mail: [email protected]
