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 table without
updating the cache. When getBucketInfo() saw the volume was not in the cache,
it would throw the exception.
Now that the createSampleVol call correctly updates the cache, it actually
breaks the test, so we removed it.
Now the getBucketInfo() call generates the VOLUME_NOT_FOUND error because
the volume really isn't there.
--
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]
