[
https://issues.apache.org/jira/browse/HDFS-4510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13583513#comment-13583513
]
Chris Nauroth commented on HDFS-4510:
-------------------------------------
Thank you for addressing the feedback, Vadim. I tested the new patch
successfully. Please disregard my earlier comments about not creating conf as
a static in {{TestNameNodeJspHelper}}. I can see now that it's safe for this
test.
Here are just a few more really minor things:
{code}
static final class ConfigurationForTestClusterJspHelper extends Configuration
{
static {
addDefaultResource("testClusterJspHelperProp.xml");
}
}
{code}
Is the subclass necessary? I think calling
{{Configuration#addDefaultResource}} from a static initialization block in
{{TestClusterJspHelper}} and then using {{new Configuration()}} would have the
same effect.
{code}
--- hadoop-hdfs-project/hadoop-hdfs/src/test/resources/hdfs-site.xml
+++ hadoop-hdfs-project/hadoop-hdfs/src/test/resources/hdfs-site.xml
@@ -24,6 +24,5 @@
<property>
<name>hadoop.security.authentication</name>
<value>simple</value>
- </property>
-
+ </property>
</configuration>
{code}
Is there actually a change in hdfs-site.xml? If not, could you remove it from
the patch?
{code}
<configuration>
<property>
<name>fs.defaultFS</name>
<value>hdfs://localhost.localdomain:45541</value>
<description></description>
</property>
<property>
<name>hadoop.security.authentication</name>
<value>simple</value>
</property>
</configuration>
{code}
Minor nitpicks: could you remove the empty <description> and indent the last
</property> 2 spaces instead of 4?
{quote}
-1 one of tests included doesn't have a timeout
{quote}
There was a change submitted just this morning to test-patch.sh to enforce that
all new tests must specify a timeout in the annotation, i.e.
@Test(timeout=30000), so let's add that to the new tests.
Thanks again!
> Cover classes ClusterJspHelper/NamenodeJspHelper with unit tests
> ----------------------------------------------------------------
>
> Key: HDFS-4510
> URL: https://issues.apache.org/jira/browse/HDFS-4510
> Project: Hadoop HDFS
> Issue Type: Test
> Affects Versions: 3.0.0, 2.0.3-alpha, 0.23.6
> Reporter: Vadim Bondarev
> Attachments: HADOOP-4510-branch-0.23-a.patch,
> HADOOP-4510-branch-0.23-b.patch, HADOOP-4510-branch-2-a.patch,
> HADOOP-4510-branch-2-b.patch, HADOOP-4510-trunk-a.patch,
> HADOOP-4510-trunk-b.patch
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
