[
https://issues.apache.org/jira/browse/HADOOP-6261?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12757321#action_12757321
]
Ravi Phulari commented on HADOOP-6261:
--------------------------------------
Cos,thanks for valuable comments and reviewing patch.
bq. indentation: it supposes to be 2 spaces, not 4
Corrected in next patch.
{quote}
what is the point of making the parent class abstract. If you want to emphasize
the fact that some of the methods are going to be overridden you can
{...@override} annotation where it's needed
I'd suggest to change the name of FileContextURIBaseTest to FileContextURIBase
because the first reaction is to suggest to rename the class into Test... so
it'd be picked up by a hardness
do you want to make this a @BeforeClass method? Otherwise it kinda stands there
along
{
for (int i = 0; i < data.length; i++) {
data[i] = (byte) (i % 10);
}
}
I think createFile has been written line 137 times already - I think you don't
need an extra one
{quote}
As per our discussion you suggested to disregard above comments .
{quote}
in qualifiedPath(String pathString, FileContext fc you, perhaps, should avoid
using a full type name as a part of the var. name. You might want to use
hungarian notation, e.g. prefix a String variable with 's', etc. Also, what if
fc would be equals to null?
{quote}
I haven't seen hungarian notation used in most of Hadoop code and there is
strong opposition to use hungarian notation.
bq.it isn't necessary to prefix your test methods with word 'test' since you
have the annotation in place. Although, there's no single rule about this
Changed .
bq. tearDown takes care about cleaning of fs2. Do you need to do the same for
fs1?
I guess you meant fc2 and fc1. These 2 are file context pointing to same
Dir/Files so cleaning fc2 in tearDown is enough, no need to clean fc1.
{quote}
there's a number of places in the code where you have things like
String fileName = "testFile";
Path testPath = qualifiedPath(fileName, fc2);
and which is an only one usage of fileName variable. It's better be declared
final in this case or simply use the literal constant in the call directly.
{quote}
This was intentional for clarity.
I agree with your way of writing testListStatus, I will update that in patch.
> Junit tests for FileContextURI
> ------------------------------
>
> Key: HADOOP-6261
> URL: https://issues.apache.org/jira/browse/HADOOP-6261
> Project: Hadoop Common
> Issue Type: Test
> Components: test
> Reporter: Ravi Phulari
> Assignee: Ravi Phulari
> Attachments: HADOOP-6261.patch
>
>
> FileContextURI unit tests.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
