[
https://issues.apache.org/jira/browse/HADOOP-13388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15425386#comment-15425386
]
Mingliang Liu commented on HADOOP-13388:
----------------------------------------
Thanks for the patch, [~boky01].
# As to the parameter order, can you double check the {{assertEquals(expected,
actual)}} pattern is used? Or else, the error message is confusing. Even
better, provide an error message. e.g.
{code}
112 assertEquals(copyPermission, initialPermission);
{code}
# Should we {{assertTrue()}} here? Or the unit test with null/empty group is
considered successful?
{code}
198 if (groups == null || groups.size() < 1) {
199 LOGGER.error("Cannot run test: need at least one group.
groups={}",
200 groups);
201 return;
202 }
{code}
# Hm...why _while_ loop is preferred to _for_?
{code}
for(StringTokenizer t = new StringTokenizer(s); t.hasMoreTokens(); ) {
a.add(t.nextToken());
}
{code}
> Clean up TestLocalFileSystemPermission
> --------------------------------------
>
> Key: HADOOP-13388
> URL: https://issues.apache.org/jira/browse/HADOOP-13388
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Reporter: Andras Bokor
> Assignee: Andras Bokor
> Priority: Trivial
> Fix For: 3.0.0-alpha2
>
> Attachments: HADOOP-13388.01.patch
>
>
> I see more problems with {{TestLocalFileSystemPermission}}:
> * Many checkstyle warnings
> * Relays on JUnit3 so Assume framework cannot be used for Windows checks.
> * In the tests in case of exception we get an error message but the test
> itself will pass (because of the return).
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
