[
https://issues.apache.org/jira/browse/HADOOP-6730?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12862517#action_12862517
]
Jakob Homan commented on HADOOP-6730:
-------------------------------------
Review of the patch:
* Lines 47-52: These notes aren't necessary since the test writers should know
about @Before and @After. Also, the provided methods (setUp() and tearDown())
conflict with the provided names.
* Lines 55 & 56: These values should be all capitalized and final.
Alternatively, it may be good to provide these values via functions so that
implementing classes may override as needed.
* Line 64: catch should be on the same line as closing brace, per our coding
style
* Line 70: fc should not be declared static. It is the responsibility of each
implementing class to provide an instance of it. Also, fc should probably be
protected
* Line 91: Please provide assert message for assertion failure
* Since this is a test that the copy method worked, we should prove it by
comparing the contents of the copied file with the original.
* Lines 135-138: This code: {co...@after
public void tearDown() throws Exception {
super.tearDown();
}{code} is a no-op and should be removed.
Nits:
* Rather than Assert.assertTrue() you can do a static import of
org.junit.Assert.{assertTrue|*}.
* Line 57: Remove extra line
* Line 47: "Since this a junit 4" ->"Since this is a junit 4 test"
> Bug in FileContext#copy
> -----------------------
>
> Key: HADOOP-6730
> URL: https://issues.apache.org/jira/browse/HADOOP-6730
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.21.0, 0.22.0
> Reporter: Eli Collins
> Assignee: Ravi Phulari
> Fix For: 0.21.0, 0.22.0
>
> Attachments: HADOOP-6730.patch
>
>
> Thanks to Eli, He noticed that there is no test for FileContext#Copy
> operation.
> On further investigation with the help of Sanjay we found that there is bug
> in FileContext#checkDest.
> {noformat}
> FileStatus dstFs = getFileStatus(dst);
> try {
> if (dstFs.isDir()) {
> if (null == srcNa
> {noformat}
> *FileStatus dstFs = getFileStatus(dst);* should be in try...catch block.
> {noformat}
> try {
> FileStatus dstFs = getFileStatus(dst);
> if (dstFs.isDir()) {
> if (null == srcNa
> {noformat}
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
