hemantk-12 commented on code in PR #4236:
URL: https://github.com/apache/ozone/pull/4236#discussion_r1097992377
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java:
##########
@@ -1351,6 +1353,80 @@ public void testGetFileStatusWithFakeDir() throws
IOException {
Assert.assertTrue(ozoneFileStatus.isFile());
}
+ @Test
+ public void testGetFileStatusWithFakeDirFalsePositive() throws IOException {
Review Comment:
```suggestion
private static Stream<Arguments>
getFileStatusWithFakeDirFalsePositiveScenarios() {
String dir0 = "dir0";
String dir1 = "dir1";
String dir2 = "dir2";
String dir3 = "dir3";
String keyName1 = dir1 + OZONE_URI_DELIMITER + "file1";
String keyName2 = dir2 + OZONE_URI_DELIMITER + "file2";
return Stream.of(
Arguments.of("Case 1: volume1/bucket1/dir1/file1", // You can
explain more here about the test case like input and out
BUCKET_NAME,
keyName1,
Collections.singletonList(dir1),
Arrays.asList(dir0, dir2, dir3)
),
Arguments.of("Case 2: volume1/bucket2/dir2/file2", // You can
explain more here about the test case like input and out
BUCKET_2_NAME,
keyName2,
Collections.singletonList(dir2),
Arrays.asList(dir0, dir1, dir3)
)
);
}
@ParameterizedTest(name = "{0}")
@MethodSource("getFileStatusWithFakeDirFalsePositiveScenarios")
public void testGetFileStatusWithFakeDirFalsePositive(String description,
String bucketName,
String keyName,
List<String>
expectedDirs,
List<String>
notExpectedDirs)
throws IOException {
createFile(bucketName, keyName);
Assert.assertNotNull(metadataManager.getKeyTable(getDefaultBucketLayout())
.get(metadataManager.getOzoneKey(VOLUME_NAME, BUCKET_NAME,
keyName)));
// Assert volumeName, bucketName and keyName as well;
for (String notExpectedDir: notExpectedDirs) {
assertFileNotFound(BUCKET_NAME, notExpectedDir);
}
for (String expectedDir: expectedDirs) {
assertIsDirectory(BUCKET_NAME, expectedDir);
}
}
```
I think it would be easier to read and understand the test scenarios if you
change the test to parameterize in Junit-5. And it can be a single test instead
of `testGetFileStatusWithFakeDirFalsePositive` and
`testGetFileStatusWithFakeDirFalseNegative` as separate tests.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java:
##########
@@ -1351,6 +1353,80 @@ public void testGetFileStatusWithFakeDir() throws
IOException {
Assert.assertTrue(ozoneFileStatus.isFile());
}
+ @Test
+ public void testGetFileStatusWithFakeDirFalsePositive() throws IOException {
+ String dir0 = "dir0";
+ String dir1 = "dir1";
+ String dir2 = "dir2";
+ String dir3 = "dir3";
+ String keyName1 = dir1 + OZONE_URI_DELIMITER + "file1";
+ String keyName2 = dir2 + OZONE_URI_DELIMITER + "file2";
+
+ // create a key "dir1/file1" in bucket1
+ createFile(BUCKET_NAME, keyName1);
+ // create a key "dir2/file2" in bucket2
+ createFile(BUCKET2_NAME, keyName2);
+
+ // Verify the following dbKeys in key table:
+ // 1. "volume1/bucket1/dir1/file1"
+ // 2. "volume1/bucket2/dir2/file2"
+ Assert.assertNotNull(metadataManager.getKeyTable(getDefaultBucketLayout())
Review Comment:
I'll suggestion to asset volume, bucket and key names as well.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java:
##########
@@ -1587,6 +1663,31 @@ private List<String> createFiles(String parent,
return keyNames;
}
+ private void createFile(String bucketName, String keyName)
+ throws IOException {
+ OmKeyArgs keyArgs = createBuilder(bucketName).setKeyName(keyName).build();
+ OpenKeySession keySession = writeClient.openKey(keyArgs);
+ keyArgs.setLocationInfoList(
+ keySession.getKeyInfo().getLatestVersionLocations().getLocationList());
+ writeClient.commitKey(keyArgs, keySession.getId());
+ }
+
+ private void assertFileNotFound(String bucketName, String keyName)
+ throws IOException {
+ OmKeyArgs keyArgs = createBuilder(bucketName).setKeyName(keyName).build();
+ OMException ex = Assert.assertThrows(OMException.class,
+ () -> keyManager.getFileStatus(keyArgs));
+ Assert.assertEquals(OMException.ResultCodes.FILE_NOT_FOUND,
ex.getResult());
+ }
+
+ private void assertIsDirectory(String bucketName, String keyName)
+ throws IOException {
+ OmKeyArgs keyArgs = createBuilder(bucketName).setKeyName(keyName).build();
+ OzoneFileStatus ozoneFileStatus = keyManager.getFileStatus(keyArgs);
+ Assert.assertEquals(keyName, ozoneFileStatus.getKeyInfo().getFileName());
Review Comment:
We should assert `volumeName` and `bucketName`.
Run the test few times and make sure that it passes all the time.
--
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]
