GeorgeJahad commented on a change in pull request #3183:
URL: https://github.com/apache/ozone/pull/3183#discussion_r824993158
##########
File path:
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestBucketManagerImpl.java
##########
@@ -120,91 +131,86 @@ public void testCreateBucket() throws Exception {
Mockito.when(kmsProvider.getMetadata(testBekName)).thenReturn(mockMetadata);
Mockito.when(mockMetadata.getCipher()).thenReturn(testCipherName);
- BucketManager bucketManager = new BucketManagerImpl(metaMgr,
- kmsProvider);
+ BucketManager bucketManager = omTestManagers.getBucketManager();
+
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.setBucketEncryptionKey(new
BucketEncryptionKeyInfo.Builder().setKeyName("key1").build())
.build();
- bucketManager.createBucket(bucketInfo);
- Assert.assertNotNull(bucketManager.getBucketInfo("sampleVol",
"bucketOne"));
+ writeClient.createBucket(bucketInfo);
+ Assert.assertNotNull(bucketManager.getBucketInfo("sample-vol",
+ "bucket-one"));
OmBucketInfo bucketInfoRead =
- bucketManager.getBucketInfo("sampleVol", "bucketOne");
+ bucketManager.getBucketInfo("sample-vol", "bucket-one");
Assert.assertTrue(bucketInfoRead.getEncryptionKeyInfo().getKeyName()
.equals(bucketInfo.getEncryptionKeyInfo().getKeyName()));
- metaMgr.getStore().close();
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testCreateEncryptedBucket() throws Exception {
- OmMetadataManagerImpl metaMgr = createSampleVol();
+ createSampleVol();
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
+ BucketManager bucketManager = omTestManagers.getBucketManager();
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.build();
- bucketManager.createBucket(bucketInfo);
- Assert.assertNotNull(bucketManager.getBucketInfo("sampleVol",
- "bucketOne"));
- metaMgr.getStore().close();
+ writeClient.createBucket(bucketInfo);
+ Assert.assertNotNull(bucketManager.getBucketInfo("sample-vol",
+ "bucket-one"));
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testCreateAlreadyExistingBucket() throws Exception {
thrown.expectMessage("Bucket already exist");
- OmMetadataManagerImpl metaMgr = createSampleVol();
+ createSampleVol();
try {
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.build();
- bucketManager.createBucket(bucketInfo);
- bucketManager.createBucket(bucketInfo);
+ writeClient.createBucket(bucketInfo);
+ writeClient.createBucket(bucketInfo);
} catch (OMException omEx) {
Assert.assertEquals(ResultCodes.BUCKET_ALREADY_EXISTS,
omEx.getResult());
throw omEx;
- } finally {
- metaMgr.getStore().close();
}
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testGetBucketInfoForInvalidBucket() throws Exception {
thrown.expectMessage("Bucket not found");
- OmMetadataManagerImpl metaMgr = createSampleVol();
- try {
+ createSampleVol();
+ try {
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
- bucketManager.getBucketInfo("sampleVol", "bucketOne");
+ BucketManager bucketManager = omTestManagers.getBucketManager();
+ bucketManager.getBucketInfo("sample-vol", "bucket-one");
} catch (OMException omEx) {
Assert.assertEquals(ResultCodes.BUCKET_NOT_FOUND,
omEx.getResult());
throw omEx;
- } finally {
- metaMgr.getStore().close();
}
}
@Test
public void testGetBucketInfo() throws Exception {
- final String volumeName = "sampleVol";
- final String bucketName = "bucketOne";
- OmMetadataManagerImpl metaMgr = createSampleVol();
Review comment:
FYI: We believe that, prior to our changes, the testGetBucketInfo() test
was actually incorrect, (even though it was passing.)
It would first create the volume, then do a getBucketInfo() expecting that
call to fail with a VOLUME_NOT_FOUND exception. It was generating that
exception because the call to create the volume updated the metaData manager
without updating the cache. When getBucketInfo() sees its not in the cache, it
throws the exception.
Now that the create volume call correctly uses the cache, this
createSampleVol() call actually breaks the test, so we removed it.
--
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]
