[ 
https://issues.apache.org/jira/browse/HADOOP-5687?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12712288#action_12712288
 ] 

Philip Zeyliger commented on HADOOP-5687:
-----------------------------------------

I like the patches.  Quick comments:

bq. @see <a 
href="https://issues.apache.org/jira/browse/HADOOP-5687";>HADOOP-5687</a>        
    | * @see <a 
href="https://issues.apache.org/jira/browse/HADOOP-5687";>HADOOP-5687</a>        
   

I was under the impression that we were discouraged from putting links to JIRA 
issues; is that less so the case in tests?

bq. return new URI(patchedfsuri);

I think the indentation here is wrong.

{noformat}
  public TestNameNodeAddress(String s) {
    super(s);
  }

  @Override
  protected void setUp() throws Exception {
  }
{noformat}
These are default, right?  You can skip them.

{noformat}
      if (!e.toString().contains(text)) {
        throw e;
      }
{noformat}
You could do "assertTrue(e.toString().contains(text), "Expected foo but got 
bar.")" instead of re-throwing.  Both end up failing the test, but the 
assertion failure makes it clearer that this isn't a random hard-to-diagnose 
exception.

{noformat}
[0]doorstop5687:hadoop-git(93063)$cat 
build/test/TEST-org.apache.hadoop.hdfs.TestNameNodeAddress.txt  | grep -B 1 
ERROR
Testcase: testNullURI took 0.142 sec
        Caused an ERROR
--
Testcase: testEmptyURI took 0.095 sec
        Caused an ERROR
--
Testcase: testFileURI took 0.044 sec
        Caused an ERROR
--
Testcase: testLocalMapsToFileURI took 0.037 sec
        Caused an ERROR
--
Testcase: testHttpNoHostURI took 0.048 sec
        Caused an ERROR
--
Testcase: testUnexpandedHdfsURI took 0.094 sec
        Caused an ERROR
{noformat}
I think some of the tests test that some exceptions are supposed to be thrown 
but aren't marked as such.



> Hadoop NameNode throws NPE if fs.default.name is the default value
> ------------------------------------------------------------------
>
>                 Key: HADOOP-5687
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5687
>             Project: Hadoop Core
>          Issue Type: Improvement
>          Components: dfs
>            Reporter: Philip Zeyliger
>            Assignee: Philip Zeyliger
>            Priority: Minor
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5687-extension-1.patch, 
> HADOOP-5687-extension-2.patch, HADOOP-5687-v3.patch, HADOOP-5687-v4.patch, 
> HADOOP-5687-v5.patch, HADOOP-5687-v5.patch, HADOOP-5687.patch, 
> HADOOP-5687.patch
>
>
> Throwing NPE is confusing; instead, an exception with a useful string 
> description could be thrown instead.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to