[ 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.