hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jakob Homan (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HADOOP-6730) Bug in FileContext#copy
Date Fri, 30 Apr 2010 02:52:54 GMT

    [ 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: {code}@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.


Mime
View raw message