[ https://issues.apache.org/jira/browse/HDFS-4133?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13488506#comment-13488506 ]
Suresh Srinivas commented on HDFS-4133: --------------------------------------- Excellent tests. All the other changes look good. The following comments are for TestSnapshot.java: # File is missing the Apache License at the beginning. # Add javadoc to TestSnapshot class. Please mention that this test tests creation 1 or multiple snapshots. # #composeSnapshotRootPath could be named #getSnapshotRoot # #composeSnapshotFilePath could be named #getSnapshotRath. This method also should #composeSnapshotRootPath. Indentation in this method is messed up. # Please add javadoc to #prepareModifications method, expecially the parameter number # Instead of using names file1, file2, and file3, names such as fileToBeDeleted, fileToBeModified, fileToBeCreated better? # Make the following methods static and move it to a helper class say SnapshotTestHelper: #* createSnapshot, checkSnapshotCreation # Move some of the common utility methods in this class to a new class SnapshotTestHelper. The same utility methods could also be used by TestSnapshotPathInodes. Some methods that should move are: composeSnapshot* methods, createSnapshot, # A lot of member variables of Modification and its subclasses can be final. # Please add very brief javadoc to Modification class methods > Add testcases for testing basic snapshot functionalities > -------------------------------------------------------- > > Key: HDFS-4133 > URL: https://issues.apache.org/jira/browse/HDFS-4133 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: data-node, name-node > Reporter: Jing Zhao > Assignee: Jing Zhao > Attachments: HDFS-4133.001.patch > > > Add testcase for basic snapshot functionalities. In the test we keep creating > snapshots, modifying original files, and check previous snapshots. -- 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