HDDS-14627. Code cleanup in TestVolumeSet#9769
Conversation
Gargi-jais11
left a comment
Gargi-jais11
left a comment
There was a problem hiding this comment.
Thanks @Russole for the patch. Overall LGTM!
Just one comment to resolve.
| assertEquals(2, volumeSet.getVolumesList().size()); | ||
|
|
||
| // Add a volume to VolumeSet | ||
| String volume3 = baseDir + "disk3"; |
There was a problem hiding this comment.
Nit: Let's make this consistent with how other volumes are added.
| String volume3 = baseDir + "disk3"; | |
| String volume3 = baseDir.resolve("disk3").toString(); |
|
Thanks @Gargi-jais11 for the review. |
| File volume = new File(volume3); | ||
| FileUtils.deleteDirectory(volume); |
There was a problem hiding this comment.
nit: Since this variable is only used when calling deleteDirectory, we can avoid creating a separate variable
| File volume = new File(volume3); | |
| FileUtils.deleteDirectory(volume); | |
| FileUtils.deleteDirectory(new File(volume3)); |
There was a problem hiding this comment.
@sreejasahithi Please ensure the suggested changes follow the correct indentation, as applying them directly may introduce formatting issues.
| System.out.println("new volume root: " + newVolume); | ||
| assertTrue(newVolume.mkdirs()); | ||
| assertTrue(newVolume.exists(), "Failed to create new volume root"); | ||
| assertTrue(newVolume.exists()); |
There was a problem hiding this comment.
mkdirs() returns true on success, so the exists() check immediately after is redundant
|
Thanks @sarvekshayr, @sreejasahithi for the review. |
| assertTrue(newVolume.exists(), "Failed to create new volume root"); | ||
| File dataDir = new File(newVolume, "chunks"); | ||
| assertTrue(dataDir.mkdirs()); | ||
| assertTrue(dataDir.exists()); |
There was a problem hiding this comment.
Let's remove the redundant check here as well.
| assertTrue(dataDir.exists()); |
|
Thanks @Russole for the patch, @Gargi-jais11 and @sreejasahithi for the review. |
4 participants
What changes were proposed in this pull request?
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-14627
How was this patch tested?