Skip to content

Commit

Permalink
HDDS-10033. Improve assertTrue assertions in ozone-manager (apache#5901)
Browse files Browse the repository at this point in the history
  • Loading branch information
wzhallright authored Jan 3, 2024
1 parent cb980a0 commit b9e74f6
Show file tree
Hide file tree
Showing 57 changed files with 230 additions and 211 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/
package org.apache.hadoop.ozone.om;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
Expand Down Expand Up @@ -68,9 +69,9 @@ public void testStampedLockBehavior() throws InterruptedException {

// Case 2: Read lock is reentrant; Write lock is exclusive
long readLockStamp1 = authorizerLock.tryReadLock(100);
assertTrue(readLockStamp1 > 0L);
assertThat(readLockStamp1).isGreaterThan(0L);
long readLockStamp2 = authorizerLock.tryReadLock(100);
assertTrue(readLockStamp2 > 0L);
assertThat(readLockStamp2).isGreaterThan(0L);

// Can't acquire write lock now, as read lock has been held
long writeLockStamp1 = authorizerLock.tryWriteLock(100);
Expand All @@ -85,7 +86,7 @@ public void testStampedLockBehavior() throws InterruptedException {
// Release the other read lock. And again. Should work
authorizerLock.unlockRead(readLockStamp1);
writeLockStamp1 = authorizerLock.tryWriteLock(100);
assertTrue(writeLockStamp1 > 0L);
assertThat(writeLockStamp1).isGreaterThan(0L);

// But a second write lock won't work
long writeLockStamp2 = authorizerLock.tryWriteLock(100);
Expand Down Expand Up @@ -162,7 +163,7 @@ public void testOptimisticRead() throws Exception {
// With only competing reads, an optimistic read should be valid.
long optStamp = authorizerLock.tryOptimisticReadThrowOnTimeout();
long readStamp = authorizerLock.tryReadLock(100);
assertTrue(readStamp > 0L);
assertThat(readStamp).isGreaterThan(0L);
assertTrue(authorizerLock.validateOptimisticRead(optStamp));
authorizerLock.unlockRead(readStamp);
assertTrue(authorizerLock.validateOptimisticRead(optStamp));
Expand All @@ -178,7 +179,7 @@ public void testOptimisticRead() throws Exception {
// stamp should be invalidated.
optStamp = authorizerLock.tryOptimisticReadThrowOnTimeout();
writeStamp = authorizerLock.tryWriteLockThrowOnTimeout();
assertTrue(writeStamp > 0L);
assertThat(writeStamp).isGreaterThan(0L);
assertFalse(authorizerLock.validateOptimisticRead(optStamp));
authorizerLock.unlockWrite(writeStamp);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.mockito.junit.jupiter.MockitoExtension;

import static java.util.Collections.singletonMap;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
Expand Down Expand Up @@ -192,7 +193,7 @@ public void testGetBucketInfoForInvalidBucket() throws Exception {
BucketManager bucketManager = omTestManagers.getBucketManager();
bucketManager.getBucketInfo("sample-vol", "bucket-one");
});
assertTrue(exception.getMessage().contains("Bucket not found"));
assertThat(exception.getMessage()).contains("Bucket not found");
assertEquals(ResultCodes.BUCKET_NOT_FOUND,
exception.getResult());
}
Expand Down Expand Up @@ -337,7 +338,7 @@ public void testDeleteBucket() throws Exception {
});
assertEquals(ResultCodes.BUCKET_NOT_FOUND,
omEx.getResult());
assertTrue(omEx.getMessage().contains("Bucket not found"));
assertThat(omEx.getMessage()).contains("Bucket not found");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@
import static java.util.Collections.singletonList;
import static java.util.Comparator.comparing;
import static java.util.stream.Collectors.toList;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.anySet;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
Expand Down Expand Up @@ -515,9 +515,8 @@ public void testLookupFileWithDnFailure() throws IOException {
.getBlockID().getLocalID());
assertEquals(pipelineTwo.getId(),
newBlockLocation.getPipeline().getId());
assertTrue(newBlockLocation.getPipeline().getNodes().contains(dnFour));
assertTrue(newBlockLocation.getPipeline().getNodes().contains(dnFive));
assertTrue(newBlockLocation.getPipeline().getNodes().contains(dnSix));
assertThat(newBlockLocation.getPipeline().getNodes())
.contains(dnFour, dnFive, dnSix);
}

private void insertKey(Pipeline pipeline, String volumeName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_RANGER_HTTPS_ADDRESS_KEY;
import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_RANGER_SERVICE;
import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.FEATURE_NOT_ENABLED;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -119,7 +120,7 @@ private void expectConfigCheckToFail(OzoneManager ozoneManager,
OMMultiTenantManager.checkAndEnableMultiTenancy(ozoneManager, conf);
fail("Should have thrown RuntimeException");
} catch (RuntimeException e) {
assertTrue(e.getMessage().contains("Failed to meet"));
assertThat(e.getMessage()).contains("Failed to meet");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_MULTITENANCY_RANGER_SYNC_INTERVAL;
import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_MULTITENANCY_RANGER_SYNC_INTERVAL_DEFAULT;
import static org.apache.hadoop.ozone.om.OMMultiTenantManagerImpl.OZONE_OM_TENANT_DEV_SKIP_RANGER;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -141,8 +142,8 @@ public void testListUsersInTenant() throws Exception {
() -> tenantManager.listUsersInTenant("tenant2", null));
assertEquals("Tenant 'tenant2' not found!", ioException.getMessage());

assertTrue(tenantManager.listUsersInTenant(TENANT_ID, "abc")
.getUserAccessIds().isEmpty());
assertThat(tenantManager.listUsersInTenant(TENANT_ID, "abc")
.getUserAccessIds()).isEmpty();
}

@Test
Expand All @@ -155,10 +156,10 @@ public void testRevokeUserAccessId() throws Exception {

tenantManager.getCacheOp()
.revokeUserAccessId("seed-accessId1", TENANT_ID);
assertTrue(tenantManager.getTenantCache().get(TENANT_ID)
.getAccessIdInfoMap().isEmpty());
assertTrue(tenantManager.listUsersInTenant(TENANT_ID, null)
.getUserAccessIds().isEmpty());
assertThat(tenantManager.getTenantCache().get(TENANT_ID)
.getAccessIdInfoMap()).isEmpty();
assertThat(tenantManager.listUsersInTenant(TENANT_ID, null)
.getUserAccessIds()).isEmpty();
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_DB_DIRS;
import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.BUCKET_NOT_FOUND;
import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_NOT_FOUND;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
Expand Down Expand Up @@ -645,23 +646,23 @@ private void testGetExpiredOpenKeys(BucketLayout bucketLayout)
numExpiredOpenKeys - 1, bucketLayout).getOpenKeyBuckets();
List<String> names = getOpenKeyNames(someExpiredKeys);
assertEquals(numExpiredOpenKeys - 1, names.size());
assertTrue(expiredKeys.containsAll(names));
assertThat(expiredKeys).containsAll(names);

// Test attempting to retrieving more expired keys than actually exist.
Collection<OpenKeyBucket.Builder> allExpiredKeys =
omMetadataManager.getExpiredOpenKeys(expireThreshold,
numExpiredOpenKeys + 1, bucketLayout).getOpenKeyBuckets();
names = getOpenKeyNames(allExpiredKeys);
assertEquals(numExpiredOpenKeys, names.size());
assertTrue(expiredKeys.containsAll(names));
assertThat(expiredKeys).containsAll(names);

// Test retrieving exact amount of expired keys that exist.
allExpiredKeys =
omMetadataManager.getExpiredOpenKeys(expireThreshold,
numExpiredOpenKeys, bucketLayout).getOpenKeyBuckets();
names = getOpenKeyNames(allExpiredKeys);
assertEquals(numExpiredOpenKeys, names.size());
assertTrue(expiredKeys.containsAll(names));
assertThat(expiredKeys).containsAll(names);
}

private void testGetExpiredOpenKeysExcludeMPUKeys(
Expand Down Expand Up @@ -810,31 +811,31 @@ private void testGetExpiredMPUs() throws Exception {
(numExpiredMPUs * numPartsPerMPU) - (numPartsPerMPU));
List<String> names = getMultipartKeyNames(someExpiredMPUs);
assertEquals(numExpiredMPUs - 1, names.size());
assertTrue(expiredMPUs.containsAll(names));
assertThat(expiredMPUs).containsAll(names);

// Test retrieving fewer expire MPU parts than actually exist (round up).
someExpiredMPUs = omMetadataManager.getExpiredMultipartUploads(
expireThreshold,
(numExpiredMPUs * numPartsPerMPU) - (numPartsPerMPU + 1));
names = getMultipartKeyNames(someExpiredMPUs);
assertEquals(numExpiredMPUs - 1, names.size());
assertTrue(expiredMPUs.containsAll(names));
assertThat(expiredMPUs).containsAll(names);

// Test attempting to retrieving more expire MPU parts than actually exist.
List<ExpiredMultipartUploadsBucket> allExpiredMPUs =
omMetadataManager.getExpiredMultipartUploads(expireThreshold,
(numExpiredMPUs * numPartsPerMPU) + numPartsPerMPU);
names = getMultipartKeyNames(allExpiredMPUs);
assertEquals(numExpiredMPUs, names.size());
assertTrue(expiredMPUs.containsAll(names));
assertThat(expiredMPUs).containsAll(names);

// Test retrieving exact amount of MPU parts than actually exist.
allExpiredMPUs =
omMetadataManager.getExpiredMultipartUploads(expireThreshold,
(numExpiredMPUs * numPartsPerMPU));
names = getMultipartKeyNames(allExpiredMPUs);
assertEquals(numExpiredMPUs, names.size());
assertTrue(expiredMPUs.containsAll(names));
assertThat(expiredMPUs).containsAll(names);
}

private List<String> getOpenKeyNames(
Expand Down Expand Up @@ -926,7 +927,7 @@ public void testListSnapshot() throws Exception {
assertTrue(snapshotInfo.getName().startsWith(prefixA));
assertEquals(snapshotInfo, snapshotsASnapshotIDMap.get(
snapshotInfo.getName()));
assertTrue(snapshotInfo.getName().compareTo(startSnapshot) > 0);
assertThat(snapshotInfo.getName().compareTo(startSnapshot)).isGreaterThan(0);
}

startSnapshot = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import static org.apache.hadoop.ozone.om.snapshot.OmSnapshotUtils.getINode;
import static org.apache.hadoop.ozone.om.OmSnapshotManager.getSnapshotPrefix;
import static org.apache.hadoop.ozone.om.snapshot.OmSnapshotUtils.truncateFileName;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -633,16 +634,16 @@ public void testCreateSnapshotIdempotent() throws Exception {
// Create first checkpoint for the snapshot checkpoint
OmSnapshotManager.createOmSnapshotCheckpoint(om.getMetadataManager(),
first);
assertFalse(logCapturer.getOutput().contains(
"for snapshot " + first.getName() + " already exists."));
assertThat(logCapturer.getOutput()).doesNotContain(
"for snapshot " + first.getName() + " already exists.");
logCapturer.clearOutput();

// Create checkpoint again for the same snapshot.
OmSnapshotManager.createOmSnapshotCheckpoint(om.getMetadataManager(),
first);

assertTrue(logCapturer.getOutput().contains(
"for snapshot " + first.getName() + " already exists."));
assertThat(logCapturer.getOutput()).contains(
"for snapshot " + first.getName() + " already exists.");
}

private SnapshotInfo createSnapshotInfo(String volumeName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
import java.io.IOException;
import java.util.Arrays;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -125,17 +125,17 @@ public void testS3AdminExtraction() throws IOException {
OzoneConfiguration configuration = new OzoneConfiguration();
configuration.set(OzoneConfigKeys.OZONE_S3_ADMINISTRATORS, "alice,bob");

assertTrue(OzoneConfigUtil.getS3AdminsFromConfig(configuration)
.containsAll(Arrays.asList("alice", "bob")));
assertThat(OzoneConfigUtil.getS3AdminsFromConfig(configuration))
.containsAll(Arrays.asList("alice", "bob"));
}

@Test
public void testS3AdminExtractionWithFallback() throws IOException {
OzoneConfiguration configuration = new OzoneConfiguration();
configuration.set(OzoneConfigKeys.OZONE_ADMINISTRATORS, "alice,bob");

assertTrue(OzoneConfigUtil.getS3AdminsFromConfig(configuration)
.containsAll(Arrays.asList("alice", "bob")));
assertThat(OzoneConfigUtil.getS3AdminsFromConfig(configuration))
.containsAll(Arrays.asList("alice", "bob"));
}

@Test
Expand All @@ -144,8 +144,8 @@ public void testS3AdminGroupExtraction() {
configuration.set(OzoneConfigKeys.OZONE_S3_ADMINISTRATORS_GROUPS,
"test1, test2");

assertTrue(OzoneConfigUtil.getS3AdminsGroupsFromConfig(
configuration).containsAll(Arrays.asList("test1", "test2")));
assertThat(OzoneConfigUtil.getS3AdminsGroupsFromConfig(configuration))
.containsAll(Arrays.asList("test1", "test2"));
}

@Test
Expand All @@ -154,7 +154,7 @@ public void testS3AdminGroupExtractionWithFallback() {
configuration.set(OzoneConfigKeys.OZONE_ADMINISTRATORS_GROUPS,
"test1, test2");

assertTrue(OzoneConfigUtil.getS3AdminsGroupsFromConfig(
configuration).containsAll(Arrays.asList("test1", "test2")));
assertThat(OzoneConfigUtil.getS3AdminsGroupsFromConfig(configuration))
.containsAll(Arrays.asList("test1", "test2"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;
import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_SST_FILTERING_SERVICE_INTERVAL;
import static org.awaitility.Awaitility.with;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
Expand Down Expand Up @@ -178,7 +179,7 @@ public void testIrrelevantSstFileDeletion()
}
}

assertTrue(nonLevel0FilesCountAfterCompact > 0);
assertThat(nonLevel0FilesCountAfterCompact).isGreaterThan(0);

String bucketName2 = "buck2";
createVolumeAndBucket(volumeName, bucketName2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
import org.apache.hadoop.security.AccessControlException;
import org.apache.ozone.test.GenericTestUtils;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.slf4j.event.Level;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;

/**
* Tests OM failover protocols using a Mock Failover provider and a Mock OM
Expand Down Expand Up @@ -70,16 +70,16 @@ public void testAccessContorlExceptionFailovers() throws Exception {
failoverProxyProvider.getRetryPolicy(
OzoneConfigKeys.OZONE_CLIENT_FAILOVER_MAX_ATTEMPTS_DEFAULT));

ServiceException serviceException = Assertions.assertThrows(ServiceException.class,
ServiceException serviceException = assertThrows(ServiceException.class,
() -> proxy.submitRequest(null, null));

// Request should try all OMs one be one and fail when the last OM also
// throws AccessControlException.
assertThat(serviceException).hasCauseInstanceOf(AccessControlException.class)
.hasMessage("ServiceException of type class org.apache.hadoop.security.AccessControlException for om3");
Assertions.assertTrue(logCapturer.getOutput().contains(getRetryProxyDebugMsg("om1")));
Assertions.assertTrue(logCapturer.getOutput().contains(getRetryProxyDebugMsg("om2")));
Assertions.assertTrue(logCapturer.getOutput().contains(getRetryProxyDebugMsg("om3")));
assertThat(logCapturer.getOutput()).contains(getRetryProxyDebugMsg("om1"));
assertThat(logCapturer.getOutput()).contains(getRetryProxyDebugMsg("om2"));
assertThat(logCapturer.getOutput()).contains(getRetryProxyDebugMsg("om3"));
}

private String getRetryProxyDebugMsg(String omNodeId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import java.util.Arrays;
import java.util.Collection;

import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -83,13 +83,13 @@ public void testOzoneLockProviderUtil(BucketLayout bucketLayout) {

if (keyPathLockEnabled) {
if (bucketLayout == BucketLayout.OBJECT_STORE) {
assertTrue(ozoneLockStrategy instanceof OBSKeyPathLockStrategy);
assertInstanceOf(OBSKeyPathLockStrategy.class, ozoneLockStrategy);
} else if (!enableFileSystemPaths &&
bucketLayout == BucketLayout.LEGACY) {
assertTrue(ozoneLockStrategy instanceof OBSKeyPathLockStrategy);
assertInstanceOf(OBSKeyPathLockStrategy.class, ozoneLockStrategy);
}
} else {
assertTrue(ozoneLockStrategy instanceof RegularBucketLockStrategy);
assertInstanceOf(RegularBucketLockStrategy.class, ozoneLockStrategy);
}
}
}
Loading

0 comments on commit b9e74f6

Please sign in to comment.