xBis7 commented on code in PR #4134:
URL: https://github.com/apache/ozone/pull/4134#discussion_r1087767041
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFsSnapshot.java:
##########
@@ -159,63 +141,53 @@ public void testCreateSnapshotDuplicateName() throws
Exception {
Assertions.assertEquals(1, res);
}
- @Test
- public void testCreateSnapshotInvalidName() throws Exception {
- String snapshotName = "snapa?b";
-
- int res = ToolRunner.run(shell,
- new String[]{"-createSnapshot", bucketPath, snapshotName});
- // Asserts that create request failed since invalid name passed
- Assertions.assertEquals(1, res);
- }
-
- @Test
- public void testCreateSnapshotOnlyNumericName() throws Exception {
- String snapshotName = "1234";
-
+ /**
+ * Create snapshot should succeed.
+ * 1st case: valid snapshot name
+ * 2nd case: snapshot name length is less than 64 chars
+ */
+ @ParameterizedTest
+ @ValueSource(strings = {"snap-1",
+ "snap75795657617173401188448010125899089001363595171500499231286"})
+ public void testCreateSnapshotSuccess(String snapshotName)
+ throws Exception {
int res = ToolRunner.run(shell,
new String[]{"-createSnapshot", bucketPath, snapshotName});
- // Asserts that create request failed since only numeric name passed
- Assertions.assertEquals(1, res);
- }
-
- @Test
- public void testCreateSnapshotInvalidURI() throws Exception {
-
- int res = ToolRunner.run(shell,
- new String[]{"-createSnapshot", "invalidURI"});
- // Asserts that create request failed since
- // invalid volume-bucket URI passed
- Assertions.assertEquals(1, res);
- }
-
- @Test
- public void testCreateSnapshotNameLength() throws Exception {
- String name63 =
- "snap75795657617173401188448010125899089001363595171500499231286";
- String name64 =
- "snap156808943643007724443266605711479126926050896107709081166294";
-
- int res = ToolRunner.run(shell,
- new String[]{"-createSnapshot", bucketPath, name63});
- // Asserts that create request succeeded since name length
- // less than 64 char
+ // Asserts that create request succeeded
Assertions.assertEquals(0, res);
- res = ToolRunner.run(shell,
- new String[]{"-createSnapshot", bucketPath, name64});
- // Asserts that create request fails since name length
- // more than 64 char
- Assertions.assertEquals(1, res);
-
SnapshotInfo snapshotInfo = ozoneManager
.getMetadataManager()
.getSnapshotInfoTable()
- .get(SnapshotInfo.getTableKey(volume, bucket, name63));
+ .get(SnapshotInfo.getTableKey(volume, bucket, snapshotName));
+ // Assert that snapshot exists in RocksDB.
+ // We can't use list or valid if snapshot directory exists because DB
+ // transaction might not be flushed by the time.
Assertions.assertNotNull(snapshotInfo);
}
+ /**
+ * Create snapshot should fail.
+ * 1st case: snapshot name contains invalid char
+ * 2nd case: snapshot name consists only of numbers
+ * 3rd case: bucket path is invalid
+ * 4th case: snapshot name length is more than 64 chars
+ */
+ @ParameterizedTest
+ @ValueSource(strings = {"snapa?b", "1234", "invalidURI",
+ "snap156808943643007724443266605711479126926050896107709081166294"})
+ public void testCreateSnapshotFailure(String snapshotName)
+ throws Exception {
+ if (snapshotName.equals("invalidURI")) {
Review Comment:
I tried it both ways and neither seem a clean approach, so I restored the
original method that checks that the creation fails due to the volume-bucket
URI. After all, this has to do with the path and not the snapshot name so it
makes sense to have it in a seperate method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
