[ https://issues.apache.org/jira/browse/HADOOP-13388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15426746#comment-15426746 ]
Andras Bokor commented on HADOOP-13388: --------------------------------------- Thanks for the review [~anu] and [~liuml07]. [~anu]: # The import changes was done by my IDE. The Assert got wildcard when I imported assertFalse. I think wildcard is reasonable there since assertFalse the 4th method from the class # I thought assertFalse is more readable here because we have an asserTrue followed by a delete so assertFalse seemed more straightforward. # Good point. I put them into the try. I had to add some null check to the {{cleanup}} method because if we have an exception while creating the file the cleanup will throw NPE which will hide the real exception. # Yes, I do not really understand the purpose of that check. {{getGroups}} cannot return with null and on Unix users have to belong to at least one group. I would remove the whole block. [~liuml07] # Yes, the order was wrong. Fixed. # Please check 4th point from above # Replacing for by while only has readability purposes. While seems more appropriate here for me (Idea also suggests to do). What do you guys think about the 2nd patch? > 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: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org