[ https://issues.apache.org/jira/browse/HDFS-7738?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14306822#comment-14306822 ] Konstantin Shvachko commented on HDFS-7738: ------------------------------------------- Hey Nicholas, agreed more test cases is a good idea. Few comments on the patch: # I would wrap the {{op}} parameter in {{recoverLeaseInternal()}} as {{enum}} rather than passing an arbitrary string. # The if-esle statement in {{testBasicTruncate()}} can be replaced with a single assert {code} assertEquals("File is expected to be closed only for truncates to the block boundary", isReady, (toTruncate == 0 || newLength % BLOCK_SIZE == 0)); {code} I think comments in asserts are important. # Why extra bracket blocks in {{testTruncateFailure()}}? I don't think freeing local variables worth it. # In {{testTruncateFailure()}} you should probably handle {{InterruptedException}} rather than passing it through the test case. > Add more negative tests for truncate > ------------------------------------ > > Key: HDFS-7738 > URL: https://issues.apache.org/jira/browse/HDFS-7738 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: test > Reporter: Tsz Wo Nicholas Sze > Assignee: Tsz Wo Nicholas Sze > Priority: Minor > Fix For: 2.7.0 > > Attachments: h7738_20150204.patch > > > The following are negative test cases for truncate. > - new length > old length > - truncating a directory > - truncating a non-existing file > - truncating a file without write permission > - truncating a file opened for append > - truncating a file in safemode -- This message was sent by Atlassian JIRA (v6.3.4#6332)