This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new d7fd453356 HDDS-10030. Improve assertTrue assertions in ozone-common
(#5899)
d7fd453356 is described below
commit d7fd453356d182ca97f17b943df21f3dfb5dc777
Author: Doroszlai, Attila <[email protected]>
AuthorDate: Thu Jan 4 11:20:12 2024 +0100
HDDS-10030. Improve assertTrue assertions in ozone-common (#5899)
---
.../java/org/apache/hadoop/ozone/TestOmUtils.java | 126 +++++-----
.../org/apache/hadoop/ozone/TestOzoneAcls.java | 11 +-
.../ozone/client/io/TestSelectorOutputStream.java | 22 +-
.../hadoop/ozone/om/lock/TestKeyPathLock.java | 40 ++--
.../hadoop/ozone/om/lock/TestOzoneManagerLock.java | 265 ++++++++-------------
5 files changed, 207 insertions(+), 257 deletions(-)
diff --git
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java
index 0601b54782..cb27038c29 100644
--- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java
+++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java
@@ -26,26 +26,26 @@ import org.junit.jupiter.api.io.TempDir;
import java.io.File;
import java.io.IOException;
+import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.nio.file.Path;
-import java.util.List;
-import java.util.Map;
import java.util.Set;
+import java.util.TreeSet;
+import static java.util.Arrays.asList;
+import static java.util.stream.Collectors.toSet;
import static org.apache.hadoop.ozone.OmUtils.getOmHostsFromConfig;
import static org.apache.hadoop.ozone.OmUtils.getOzoneManagerServiceId;
import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ADDRESS_KEY;
import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_INTERNAL_SERVICE_ID;
import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_NODES_KEY;
import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SERVICE_IDS_KEY;
-import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.MatcherAssert.assertThat;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.fail;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assumptions.assumeTrue;
/**
@@ -58,7 +58,7 @@ public class TestOmUtils {
private Path folder;
@Test
- public void createOMDirCreatesDirectoryIfNecessary() throws IOException {
+ void createOMDirCreatesDirectoryIfNecessary() {
File parent = folder.toFile();
File omDir = new File(new File(parent, "sub"), "dir");
assertFalse(omDir.exists());
@@ -69,7 +69,7 @@ public class TestOmUtils {
}
@Test
- public void createOMDirDoesNotThrowIfAlreadyExists() throws IOException {
+ void createOMDirDoesNotThrowIfAlreadyExists() {
File omDir = folder.toFile();
assertTrue(omDir.exists());
@@ -79,7 +79,7 @@ public class TestOmUtils {
}
@Test
- public void createOMDirThrowsIfCannotCreate() {
+ void createOMDirThrowsIfCannotCreate() {
assertThrows(IllegalArgumentException.class, () -> {
File parent = folder.toFile();
File omDir = new File(new File(parent, "sub"), "dir");
@@ -91,76 +91,82 @@ public class TestOmUtils {
}
@Test
- public void testGetOmHAAddressesById() {
+ void testGetOmHAAddressesById() {
OzoneConfiguration conf = new OzoneConfiguration();
conf.set(OZONE_OM_SERVICE_IDS_KEY, "ozone1");
conf.set("ozone.om.nodes.ozone1", "node1,node2,node3");
conf.set("ozone.om.address.ozone1.node1", "1.1.1.1");
conf.set("ozone.om.address.ozone1.node2", "1.1.1.2");
conf.set("ozone.om.address.ozone1.node3", "1.1.1.3");
- Map<String, List<InetSocketAddress>> addresses =
- OmUtils.getOmHAAddressesById(conf);
- assertFalse(addresses.isEmpty());
- List<InetSocketAddress> rpcAddrs = addresses.get("ozone1");
- assertFalse(rpcAddrs.isEmpty());
- assertTrue(rpcAddrs.stream().anyMatch(
- a -> a.getAddress().getHostAddress().equals("1.1.1.1")));
- assertTrue(rpcAddrs.stream().anyMatch(
- a -> a.getAddress().getHostAddress().equals("1.1.1.2")));
- assertTrue(rpcAddrs.stream().anyMatch(
- a -> a.getAddress().getHostAddress().equals("1.1.1.3")));
+ Set<String> ozone1hosts = OmUtils.getOmHAAddressesById(conf)
+ .get("ozone1")
+ .stream()
+ .map(InetSocketAddress::getAddress)
+ .map(InetAddress::getHostAddress)
+ .collect(toSet());
+ assertEquals(
+ new TreeSet<>(asList("1.1.1.1", "1.1.1.2", "1.1.1.3")),
+ ozone1hosts);
}
@Test
- public void testGetOzoneManagerServiceId() throws IOException {
-
+ void getOzoneManagerServiceIdWithoutAnyServices() throws IOException {
// If the above is not configured, look at 'ozone.om.service.ids'.
// If no config is set, return null. (Non HA)
OzoneConfiguration configuration = new OzoneConfiguration();
assertNull(getOzoneManagerServiceId(configuration));
+ }
+ @Test
+ void getOzoneManagerServiceIdPrefersInternalService() throws IOException {
// Verify 'ozone.om.internal.service.id' takes precedence
+ OzoneConfiguration configuration = new OzoneConfiguration();
configuration.set(OZONE_OM_INTERNAL_SERVICE_ID, "om1");
configuration.set(OZONE_OM_SERVICE_IDS_KEY, "om2,om1");
String id = getOzoneManagerServiceId(configuration);
assertEquals("om1", id);
+ }
+ @Test
+ void getOzoneManagerServiceIdWithInternalServiceMismatch() {
+ OzoneConfiguration configuration = new OzoneConfiguration();
+ configuration.set(OZONE_OM_INTERNAL_SERVICE_ID, "om1");
configuration.set(OZONE_OM_SERVICE_IDS_KEY, "om2,om3");
- try {
- getOzoneManagerServiceId(configuration);
- fail();
- } catch (IOException ioEx) {
- assertTrue(ioEx.getMessage()
- .contains("Cannot find the internal service id om1 in [om2, om3]"));
- }
+ Exception e = assertThrows(IOException.class,
+ () -> getOzoneManagerServiceId(configuration));
+ assertThat(e)
+ .hasMessageContaining("Cannot find the internal service id om1 in
[om2, om3]");
+ }
+ @Test
+ void getOzoneManagerServiceIdWithSingleServiceConfigured() throws
IOException {
// When internal service ID is not defined.
// Verify if count(ozone.om.service.ids) == 1, return that id.
- configuration = new OzoneConfiguration();
+ OzoneConfiguration configuration = new OzoneConfiguration();
configuration.set(OZONE_OM_SERVICE_IDS_KEY, "om2");
- id = getOzoneManagerServiceId(configuration);
+ String id = getOzoneManagerServiceId(configuration);
assertEquals("om2", id);
+ }
+ @Test
+ void getOzoneManagerServiceIdWithMultipleServices() {
// Verify if more than count(ozone.om.service.ids) > 1 and internal
// service id is not defined, throw exception
+ OzoneConfiguration configuration = new OzoneConfiguration();
configuration.set(OZONE_OM_SERVICE_IDS_KEY, "om2,om1");
- try {
- getOzoneManagerServiceId(configuration);
- fail();
- } catch (IOException ioEx) {
- assertTrue(ioEx.getMessage()
- .contains("More than 1 OzoneManager ServiceID (ozone.om.service" +
- ".ids) configured"));
- }
+ Exception e = assertThrows(IOException.class,
+ () -> getOzoneManagerServiceId(configuration));
+ assertThat(e)
+ .hasMessageContaining("More than 1 OzoneManager ServiceID
(ozone.om.service.ids) configured");
}
@Test
- public void checkMaxTransactionID() {
+ void checkMaxTransactionID() {
assertEquals((long) (Math.pow(2, 54) - 2), OmUtils.MAX_TRXN_ID);
}
@Test
- public void testGetOmHostsFromConfig() {
+ void testGetOmHostsFromConfig() {
OzoneConfiguration conf = new OzoneConfiguration();
String serviceId = "myOmId";
@@ -175,40 +181,46 @@ public class TestOmUtils {
Set<String> hosts = getOmHostsFromConfig(conf, serviceId);
assertEquals(3, hosts.size());
- assertTrue(hosts.contains("omA-host"));
- assertTrue(hosts.contains("omB-host"));
- assertTrue(hosts.contains("omC-host"));
+ assertThat(hosts).contains("omA-host", "omB-host", "omC-host");
hosts = getOmHostsFromConfig(conf, serviceId2);
assertEquals(1, hosts.size());
- assertTrue(hosts.contains("om1-host"));
+ assertThat(hosts).contains("om1-host");
- assertTrue(getOmHostsFromConfig(conf, "newId").isEmpty());
+ assertEquals(0, getOmHostsFromConfig(conf, "newId").size());
}
@Test
- public void testgetOmSocketAddress() {
+ void getOmSocketAddressHost() {
final OzoneConfiguration conf = new OzoneConfiguration();
// First try a client address with just a host name. Verify it falls
// back to the default port.
conf.set(OMConfigKeys.OZONE_OM_ADDRESS_KEY, "1.2.3.4");
InetSocketAddress addr = OmUtils.getOmAddress(conf);
- assertThat(addr.getHostString(), is("1.2.3.4"));
- assertThat(addr.getPort(), is(OMConfigKeys.OZONE_OM_PORT_DEFAULT));
+ assertEquals("1.2.3.4", addr.getHostString());
+ assertEquals(OMConfigKeys.OZONE_OM_PORT_DEFAULT, addr.getPort());
+ }
+ @Test
+ void getOmSocketAddressHostPort() {
+ final OzoneConfiguration conf = new OzoneConfiguration();
// Next try a client address with just a host name and port. Verify the
port
// is ignored and the default OM port is used.
conf.set(OMConfigKeys.OZONE_OM_ADDRESS_KEY, "1.2.3.4:100");
- addr = OmUtils.getOmAddress(conf);
- assertThat(addr.getHostString(), is("1.2.3.4"));
- assertThat(addr.getPort(), is(100));
+ InetSocketAddress addr = OmUtils.getOmAddress(conf);
+ assertEquals("1.2.3.4", addr.getHostString());
+ assertEquals(100, addr.getPort());
+ }
- // Assert the we are able to use default configs if no value is specified.
+ @Test
+ void getOmSocketAddressEmpty() {
+ final OzoneConfiguration conf = new OzoneConfiguration();
+ // Assert that we are able to use default configs if no value is specified.
conf.set(OMConfigKeys.OZONE_OM_ADDRESS_KEY, "");
- addr = OmUtils.getOmAddress(conf);
- assertThat(addr.getHostString(), is("0.0.0.0"));
- assertThat(addr.getPort(), is(OMConfigKeys.OZONE_OM_PORT_DEFAULT));
+ InetSocketAddress addr = OmUtils.getOmAddress(conf);
+ assertEquals("0.0.0.0", addr.getHostString());
+ assertEquals(OMConfigKeys.OZONE_OM_PORT_DEFAULT, addr.getPort());
}
}
diff --git
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOzoneAcls.java
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOzoneAcls.java
index d20c1e4aa4..a815b72dec 100644
---
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOzoneAcls.java
+++
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOzoneAcls.java
@@ -35,6 +35,7 @@ import static
org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.REA
import static
org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.READ_ACL;
import static
org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.WRITE;
import static
org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.WRITE_ACL;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.fail;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -44,10 +45,10 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
/**
* This class is to test acl storage and retrieval in ozone store.
*/
-public class TestOzoneAcls {
+class TestOzoneAcls {
@Test
- public void testAclParse() {
+ void testAclParse() {
HashMap<String, Boolean> testMatrix;
testMatrix = new HashMap<>();
@@ -134,7 +135,7 @@ public class TestOzoneAcls {
}
@Test
- public void testAclValues() throws Exception {
+ void testAclValues() {
OzoneAcl acl = OzoneAcl.parseAcl("user:bilbo:rw");
assertEquals(acl.getName(), "bilbo");
assertTrue(acl.getAclBitSet().get(READ.ordinal()));
@@ -269,11 +270,11 @@ public class TestOzoneAcls {
IllegalArgumentException exception = assertThrows(
IllegalArgumentException.class,
() -> OzoneAcl.parseAcl("world::rwdlncxncxdfsfgbny"));
- assertTrue(exception.getMessage().contains("ACL right is not"));
+ assertThat(exception).hasMessageContaining("ACL right is not");
}
@Test
- public void testBitSetToListConversion() throws Exception {
+ void testBitSetToListConversion() {
OzoneAcl acl = OzoneAcl.parseAcl("user:bilbo:rw");
List<ACLType> rights = acl.getAclList();
diff --git
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/client/io/TestSelectorOutputStream.java
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/client/io/TestSelectorOutputStream.java
index 7e831645f9..d1c8213cd0 100644
---
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/client/io/TestSelectorOutputStream.java
+++
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/client/io/TestSelectorOutputStream.java
@@ -31,7 +31,7 @@ import java.io.IOException;
import java.io.OutputStream;
import java.util.function.Supplier;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -40,8 +40,8 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
* Test {@link SelectorOutputStream}.
*/
@Timeout(30)
-public class TestSelectorOutputStream {
- static final Logger LOG = LoggerFactory.getLogger(
+class TestSelectorOutputStream {
+ private static final Logger LOG = LoggerFactory.getLogger(
TestSelectorOutputStream.class);
enum Op {
@@ -116,48 +116,48 @@ public class TestSelectorOutputStream {
}
@Test
- public void testFlush() throws Exception {
+ void testFlush() throws Exception {
runTestSelector(10, 2, Op.FLUSH);
runTestSelector(10, 10, Op.FLUSH);
runTestSelector(10, 20, Op.FLUSH);
}
@Test
- public void testClose() throws Exception {
+ void testClose() throws Exception {
runTestSelector(10, 2, Op.CLOSE);
runTestSelector(10, 10, Op.CLOSE);
runTestSelector(10, 20, Op.CLOSE);
}
@Test
- public void testHflushSyncable() throws Exception {
+ void testHflushSyncable() throws Exception {
runTestSelector(10, 2, Op.HFLUSH, true);
runTestSelector(10, 10, Op.HFLUSH, true);
runTestSelector(10, 20, Op.HFLUSH, true);
}
@Test
- public void testHflushNonSyncable() {
+ void testHflushNonSyncable() {
final IllegalStateException thrown = assertThrows(
IllegalStateException.class,
() -> runTestSelector(10, 2, Op.HFLUSH, false));
LOG.info("thrown", thrown);
- assertTrue(thrown.getMessage().contains("not Syncable"));
+ assertThat(thrown).hasMessageContaining("not Syncable");
}
@Test
- public void testHSyncSyncable() throws Exception {
+ void testHSyncSyncable() throws Exception {
runTestSelector(10, 2, Op.HSYNC, true);
runTestSelector(10, 10, Op.HSYNC, true);
runTestSelector(10, 20, Op.HSYNC, true);
}
@Test
- public void testHSyncNonSyncable() {
+ void testHSyncNonSyncable() {
final IllegalStateException thrown = assertThrows(
IllegalStateException.class,
() -> runTestSelector(10, 2, Op.HSYNC, false));
LOG.info("thrown", thrown);
- assertTrue(thrown.getMessage().contains("not Syncable"));
+ assertThat(thrown).hasMessageContaining("not Syncable");
}
}
diff --git
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestKeyPathLock.java
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestKeyPathLock.java
index 0c3978ccd2..75adb7e6a1 100644
---
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestKeyPathLock.java
+++
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestKeyPathLock.java
@@ -29,23 +29,23 @@ import java.util.List;
import java.util.UUID;
import java.util.concurrent.CountDownLatch;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.fail;
-import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.assertEquals;
/**
* Tests OzoneManagerLock.Resource.KEY_PATH_LOCK.
*/
-public class TestKeyPathLock extends TestOzoneManagerLock {
+class TestKeyPathLock extends TestOzoneManagerLock {
private static final Logger LOG =
LoggerFactory.getLogger(TestKeyPathLock.class);
- private OzoneManagerLock.Resource resource =
+ private final OzoneManagerLock.Resource resource =
OzoneManagerLock.Resource.KEY_PATH_LOCK;
@Test
- public void testKeyPathLockMultiThreading() throws Exception {
+ void testKeyPathLockMultiThreading() throws Exception {
testSameKeyPathWriteLockMultiThreading(10, 100);
testDiffKeyPathWriteLockMultiThreading(10, 100);
}
@@ -70,8 +70,8 @@ public class TestKeyPathLock extends TestOzoneManagerLock {
// "/a/b/c/d/key1 - WLock - 5th iteration" -- blocked
// (iterations are sequential)
- public void testSameKeyPathWriteLockMultiThreading(int threadCount,
- int iterations)
+ void testSameKeyPathWriteLockMultiThreading(int threadCount,
+ int iterations)
throws InterruptedException {
String volumeName = UUID.randomUUID().toString();
@@ -140,9 +140,7 @@ public class TestKeyPathLock extends TestOzoneManagerLock {
lock.acquireWriteLock(resource, sampleResourceName);
LOG.info("Write Lock Acquired by " + Thread.currentThread().getName());
- /**
- * Critical Section. count = count + 1;
- */
+ // Critical Section. count = count + 1;
for (int idx = 0; idx < iterations; idx++) {
counter.incrementCount();
}
@@ -164,8 +162,8 @@ public class TestKeyPathLock extends TestOzoneManagerLock {
// "/a/b/c/d/key5 - WLock - 5th iteration" -- allowed
// (iterations are parallel)
- public void testDiffKeyPathWriteLockMultiThreading(int threadCount,
- int iterations)
+ void testDiffKeyPathWriteLockMultiThreading(int threadCount,
+ int iterations)
throws Exception {
String volumeName = UUID.randomUUID().toString();
@@ -190,9 +188,7 @@ public class TestKeyPathLock extends TestOzoneManagerLock {
threads[i].start();
}
- /**
- * Waiting for all the threads to count down
- */
+ // Waiting for all the threads to count down
GenericTestUtils.waitFor(() -> {
if (countDown.getCount() > 0) {
LOG.info("Waiting for the threads to count down {} ",
@@ -229,7 +225,7 @@ public class TestKeyPathLock extends TestOzoneManagerLock {
}
@Test
- public void testAcquireWriteBucketLockWhileAcquiredWriteKeyPathLock() {
+ void testAcquireWriteBucketLockWhileAcquiredWriteKeyPathLock() {
OzoneManagerLock.Resource higherResource =
OzoneManagerLock.Resource.BUCKET_LOCK;
@@ -249,12 +245,12 @@ public class TestKeyPathLock extends TestOzoneManagerLock
{
} catch (RuntimeException ex) {
String message = "cannot acquire " + higherResource.getName() + " lock "
+
"while holding [" + resource.getName() + "] lock(s).";
- assertTrue(ex.getMessage().contains(message), ex.getMessage());
+ assertThat(ex).hasMessageContaining(message);
}
}
@Test
- public void testAcquireWriteBucketLockWhileAcquiredReadKeyPathLock() {
+ void testAcquireWriteBucketLockWhileAcquiredReadKeyPathLock() {
OzoneManagerLock.Resource higherResource =
OzoneManagerLock.Resource.BUCKET_LOCK;
@@ -274,12 +270,12 @@ public class TestKeyPathLock extends TestOzoneManagerLock
{
} catch (RuntimeException ex) {
String message = "cannot acquire " + higherResource.getName() + " lock "
+
"while holding [" + resource.getName() + "] lock(s).";
- assertTrue(ex.getMessage().contains(message), ex.getMessage());
+ assertThat(ex).hasMessageContaining(message);
}
}
@Test
- public void testAcquireReadBucketLockWhileAcquiredReadKeyPathLock() {
+ void testAcquireReadBucketLockWhileAcquiredReadKeyPathLock() {
OzoneManagerLock.Resource higherResource =
OzoneManagerLock.Resource.BUCKET_LOCK;
@@ -299,12 +295,12 @@ public class TestKeyPathLock extends TestOzoneManagerLock
{
} catch (RuntimeException ex) {
String message = "cannot acquire " + higherResource.getName() + " lock "
+
"while holding [" + resource.getName() + "] lock(s).";
- assertTrue(ex.getMessage().contains(message), ex.getMessage());
+ assertThat(ex).hasMessageContaining(message);
}
}
@Test
- public void testAcquireReadBucketLockWhileAcquiredWriteKeyPathLock() {
+ void testAcquireReadBucketLockWhileAcquiredWriteKeyPathLock() {
OzoneManagerLock.Resource higherResource =
OzoneManagerLock.Resource.BUCKET_LOCK;
@@ -324,7 +320,7 @@ public class TestKeyPathLock extends TestOzoneManagerLock {
} catch (RuntimeException ex) {
String message = "cannot acquire " + higherResource.getName() + " lock "
+
"while holding [" + resource.getName() + "] lock(s).";
- assertTrue(ex.getMessage().contains(message), ex.getMessage());
+ assertThat(ex).hasMessageContaining(message);
}
}
}
diff --git
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java
index 5c4206fa19..856f2b238c 100644
---
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java
+++
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java
@@ -24,13 +24,18 @@ import java.util.Stack;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.hadoop.metrics2.MetricsRecord;
import org.apache.hadoop.metrics2.impl.MetricsCollectorImpl;
import org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -41,31 +46,26 @@ import static org.junit.jupiter.api.Assertions.fail;
* Class tests OzoneManagerLock.
*/
@Timeout(300)
-public class TestOzoneManagerLock {
+class TestOzoneManagerLock {
- @Test
- public void acquireResourceLock() {
- String[] resourceName;
- for (Resource resource : Resource.values()) {
- resourceName = generateResourceName(resource);
- testResourceLock(resourceName, resource);
- }
+ @ParameterizedTest
+ @EnumSource
+ void acquireResourceLock(Resource resource) {
+ String[] resourceName = generateResourceName(resource);
+ testResourceLock(resourceName, resource);
}
private void testResourceLock(String[] resourceName, Resource resource) {
OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
lock.acquireWriteLock(resource, resourceName);
- lock.releaseWriteLock(resource, resourceName);
- assertTrue(true);
+ assertDoesNotThrow(() -> lock.releaseWriteLock(resource, resourceName));
}
- @Test
- public void reacquireResourceLock() {
- String[] resourceName;
- for (Resource resource : Resource.values()) {
- resourceName = generateResourceName(resource);
- testResourceReacquireLock(resourceName, resource);
- }
+ @ParameterizedTest
+ @EnumSource
+ void reacquireResourceLock(Resource resource) {
+ String[] resourceName = generateResourceName(resource);
+ testResourceReacquireLock(resourceName, resource);
}
private void testResourceReacquireLock(String[] resourceName,
@@ -83,22 +83,19 @@ public class TestOzoneManagerLock {
} catch (RuntimeException ex) {
String message = "cannot acquire " + resource.getName() + " lock " +
"while holding [" + resource.getName() + "] lock(s).";
- assertTrue(ex.getMessage().contains(message),
- ex.getMessage());
+ assertThat(ex).hasMessageContaining(message);
}
- lock.releaseWriteLock(resource, resourceName);
- assertTrue(true);
+ assertDoesNotThrow(() -> lock.releaseWriteLock(resource, resourceName));
} else {
lock.acquireWriteLock(resource, resourceName);
lock.acquireWriteLock(resource, resourceName);
- lock.releaseWriteLock(resource, resourceName);
- lock.releaseWriteLock(resource, resourceName);
- assertTrue(true);
+ assertDoesNotThrow(() -> lock.releaseWriteLock(resource, resourceName));
+ assertDoesNotThrow(() -> lock.releaseWriteLock(resource, resourceName));
}
}
@Test
- public void testLockingOrder() {
+ void testLockingOrder() {
OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
String[] resourceName;
@@ -120,30 +117,27 @@ public class TestOzoneManagerLock {
// Now release locks
while (!stack.empty()) {
ResourceInfo resourceInfo = stack.pop();
- lock.releaseWriteLock(resourceInfo.getResource(),
- resourceInfo.getLockName());
+ assertDoesNotThrow(() ->
+ lock.releaseWriteLock(resourceInfo.getResource(),
resourceInfo.getLockName()));
}
}
- assertTrue(true);
}
- @Test
- public void testLockViolationsWithOneHigherLevelLock() {
+ @ParameterizedTest
+ @EnumSource
+ void testLockViolationsWithOneHigherLevelLock(Resource resource) {
OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
- for (Resource resource : Resource.values()) {
- for (Resource higherResource : Resource.values()) {
- if (higherResource.getMask() > resource.getMask()) {
- String[] resourceName = generateResourceName(higherResource);
- lock.acquireWriteLock(higherResource, resourceName);
- try {
- lock.acquireWriteLock(resource, generateResourceName(resource));
- fail("testLockViolationsWithOneHigherLevelLock failed");
- } catch (RuntimeException ex) {
- String message = "cannot acquire " + resource.getName() + " lock "
+
- "while holding [" + higherResource.getName() + "] lock(s).";
- assertTrue(ex.getMessage().contains(message),
- ex.getMessage());
- }
+ for (Resource higherResource : Resource.values()) {
+ if (higherResource.getMask() > resource.getMask()) {
+ String[] resourceName = generateResourceName(higherResource);
+ lock.acquireWriteLock(higherResource, resourceName);
+ try {
+ Exception e = assertThrows(RuntimeException.class,
+ () -> lock.acquireWriteLock(resource,
generateResourceName(resource)));
+ String message = "cannot acquire " + resource.getName() + " lock " +
+ "while holding [" + higherResource.getName() + "] lock(s).";
+ assertThat(e).hasMessageContaining(message);
+ } finally {
lock.releaseWriteLock(higherResource, resourceName);
}
}
@@ -151,7 +145,7 @@ public class TestOzoneManagerLock {
}
@Test
- public void testLockViolations() {
+ void testLockViolations() {
OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
String[] resourceName;
@@ -175,8 +169,7 @@ public class TestOzoneManagerLock {
} catch (RuntimeException ex) {
String message = "cannot acquire " + resource.getName() + " lock "
+
"while holding " + currentLocks + " lock(s).";
- assertTrue(ex.getMessage().contains(message),
- ex.getMessage());
+ assertThat(ex).hasMessageContaining(message);
}
}
}
@@ -191,7 +184,7 @@ public class TestOzoneManagerLock {
}
@Test
- public void releaseLockWithOutAcquiringLock() {
+ void releaseLockWithOutAcquiringLock() {
OzoneManagerLock lock =
new OzoneManagerLock(new OzoneConfiguration());
assertThrows(IllegalMonitorStateException.class,
@@ -215,9 +208,9 @@ public class TestOzoneManagerLock {
/**
* Class used to store locked resource info.
*/
- public static class ResourceInfo {
- private String[] lockName;
- private Resource resource;
+ private static class ResourceInfo {
+ private final String[] lockName;
+ private final Resource resource;
ResourceInfo(String[] resourceName, Resource resource) {
this.lockName = resourceName;
@@ -234,60 +227,47 @@ public class TestOzoneManagerLock {
}
@Test
- public void acquireMultiUserLock() {
+ void acquireMultiUserLock() {
OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
lock.acquireMultiUserLock("user1", "user2");
- lock.releaseMultiUserLock("user1", "user2");
- assertTrue(true);
+ assertDoesNotThrow(() -> lock.releaseMultiUserLock("user1", "user2"));
}
@Test
- public void reAcquireMultiUserLock() {
+ void reAcquireMultiUserLock() {
OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
lock.acquireMultiUserLock("user1", "user2");
- try {
- lock.acquireMultiUserLock("user1", "user2");
- fail("reAcquireMultiUserLock failed");
- } catch (RuntimeException ex) {
- String message = "cannot acquire USER_LOCK lock while holding " +
- "[USER_LOCK] lock(s).";
- assertTrue(ex.getMessage().contains(message), ex.getMessage());
- }
+ Exception e = assertThrows(RuntimeException.class,
+ () -> lock.acquireMultiUserLock("user1", "user2"));
+ assertThat(e)
+ .hasMessageContaining("cannot acquire USER_LOCK lock while holding
[USER_LOCK] lock(s).");
lock.releaseMultiUserLock("user1", "user2");
}
@Test
- public void acquireMultiUserLockAfterUserLock() {
+ void acquireMultiUserLockAfterUserLock() {
OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
lock.acquireWriteLock(Resource.USER_LOCK, "user3");
- try {
- lock.acquireMultiUserLock("user1", "user2");
- fail("acquireMultiUserLockAfterUserLock failed");
- } catch (RuntimeException ex) {
- String message = "cannot acquire USER_LOCK lock while holding " +
- "[USER_LOCK] lock(s).";
- assertTrue(ex.getMessage().contains(message), ex.getMessage());
- }
+ Exception e = assertThrows(RuntimeException.class,
+ () -> lock.acquireMultiUserLock("user1", "user2"));
+ assertThat(e)
+ .hasMessageContaining("cannot acquire USER_LOCK lock while holding
[USER_LOCK] lock(s).");
lock.releaseWriteLock(Resource.USER_LOCK, "user3");
}
@Test
- public void acquireUserLockAfterMultiUserLock() {
+ void acquireUserLockAfterMultiUserLock() {
OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
lock.acquireMultiUserLock("user1", "user2");
- try {
- lock.acquireWriteLock(Resource.USER_LOCK, "user3");
- fail("acquireUserLockAfterMultiUserLock failed");
- } catch (RuntimeException ex) {
- String message = "cannot acquire USER_LOCK lock while holding " +
- "[USER_LOCK] lock(s).";
- assertTrue(ex.getMessage().contains(message), ex.getMessage());
- }
+ Exception e = assertThrows(RuntimeException.class,
+ () -> lock.acquireWriteLock(Resource.USER_LOCK, "user3"));
+ assertThat(e)
+ .hasMessageContaining("cannot acquire USER_LOCK lock while holding
[USER_LOCK] lock(s).");
lock.releaseMultiUserLock("user1", "user2");
}
@Test
- public void testLockResourceParallel() throws Exception {
+ void testLockResourceParallel() throws Exception {
OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
for (Resource resource :
@@ -317,7 +297,7 @@ public class TestOzoneManagerLock {
}
@Test
- public void testMultiLockResourceParallel() throws Exception {
+ void testMultiLockResourceParallel() throws Exception {
OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
lock.acquireMultiUserLock("user2", "user1");
@@ -340,19 +320,14 @@ public class TestOzoneManagerLock {
assertTrue(gotLock.get());
}
- @Test
- public void testLockHoldCount() {
- String[] resourceName;
- for (Resource resource : Resource.values()) {
+ @ParameterizedTest
+ @EnumSource(mode = EnumSource.Mode.EXCLUDE,
// USER_LOCK, S3_SECRET_LOCK and PREFIX_LOCK disallow lock re-acquire by
// the same thread.
- if (resource != Resource.USER_LOCK &&
- resource != Resource.S3_SECRET_LOCK &&
- resource != Resource.PREFIX_LOCK) {
- resourceName = generateResourceName(resource);
- testLockHoldCountUtil(resource, resourceName);
- }
- }
+ names = { "PREFIX_LOCK", "S3_SECRET_LOCK", "USER_LOCK" })
+ void testLockHoldCount(Resource resource) {
+ String[] resourceName = generateResourceName(resource);
+ testLockHoldCountUtil(resource, resourceName);
}
private void testLockHoldCountUtil(Resource resource,
@@ -372,44 +347,36 @@ public class TestOzoneManagerLock {
lock.releaseReadLock(resource, resourceName);
assertEquals(0, lock.getReadHoldCount(resource, resourceName));
- assertFalse(
- lock.isWriteLockedByCurrentThread(resource, resourceName));
+ assertFalse(lock.isWriteLockedByCurrentThread(resource, resourceName));
assertEquals(0, lock.getWriteHoldCount(resource, resourceName));
lock.acquireWriteLock(resource, resourceName);
- assertTrue(
- lock.isWriteLockedByCurrentThread(resource, resourceName));
+ assertTrue(lock.isWriteLockedByCurrentThread(resource, resourceName));
assertEquals(1, lock.getWriteHoldCount(resource, resourceName));
lock.acquireWriteLock(resource, resourceName);
- assertTrue(
- lock.isWriteLockedByCurrentThread(resource, resourceName));
+ assertTrue(lock.isWriteLockedByCurrentThread(resource, resourceName));
assertEquals(2, lock.getWriteHoldCount(resource, resourceName));
lock.releaseWriteLock(resource, resourceName);
- assertTrue(
- lock.isWriteLockedByCurrentThread(resource, resourceName));
+ assertTrue(lock.isWriteLockedByCurrentThread(resource, resourceName));
assertEquals(1, lock.getWriteHoldCount(resource, resourceName));
lock.releaseWriteLock(resource, resourceName);
- assertFalse(
- lock.isWriteLockedByCurrentThread(resource, resourceName));
+ assertFalse(lock.isWriteLockedByCurrentThread(resource, resourceName));
assertEquals(0, lock.getWriteHoldCount(resource, resourceName));
}
- @Test
- public void testLockConcurrentStats() throws InterruptedException {
- String[] resourceName;
- for (Resource resource :
- Resource.values()) {
- resourceName = generateResourceName(resource);
- testReadLockConcurrentStats(resource, resourceName, 10);
- testWriteLockConcurrentStats(resource, resourceName, 5);
- testSyntheticReadWriteLockConcurrentStats(resource, resourceName, 10, 3);
- }
+ @ParameterizedTest
+ @EnumSource
+ void testLockConcurrentStats(Resource resource) throws InterruptedException {
+ String[] resourceName = generateResourceName(resource);
+ testReadLockConcurrentStats(resource, resourceName, 10);
+ testWriteLockConcurrentStats(resource, resourceName, 5);
+ testSyntheticReadWriteLockConcurrentStats(resource, resourceName, 10, 3);
}
- public void testReadLockConcurrentStats(Resource resource,
+ private void testReadLockConcurrentStats(Resource resource,
String[] resourceName,
int threadCount)
throws InterruptedException {
@@ -434,18 +401,14 @@ public class TestOzoneManagerLock {
}
String readHeldStat = lock.getOMLockMetrics().getReadLockHeldTimeMsStat();
- assertTrue(readHeldStat.contains("Samples = " + threadCount),
- "Expected " + threadCount +
- " samples in readLockHeldTimeMsStat: " + readHeldStat);
+ assertThat(readHeldStat).contains("Samples = " + threadCount);
String readWaitingStat =
lock.getOMLockMetrics().getReadLockWaitingTimeMsStat();
- assertTrue(readWaitingStat.contains("Samples = " + threadCount),
- "Expected " + threadCount +
- " samples in readLockWaitingTimeMsStat: " + readWaitingStat);
+ assertThat(readWaitingStat).contains("Samples = " + threadCount);
}
- public void testWriteLockConcurrentStats(Resource resource,
+ private void testWriteLockConcurrentStats(Resource resource,
String[] resourceName,
int threadCount)
throws InterruptedException {
@@ -470,18 +433,14 @@ public class TestOzoneManagerLock {
}
String writeHeldStat =
lock.getOMLockMetrics().getWriteLockHeldTimeMsStat();
- assertTrue(writeHeldStat.contains("Samples = " + threadCount),
- "Expected " + threadCount +
- " samples in writeLockHeldTimeMsStat: " + writeHeldStat);
+ assertThat(writeHeldStat).contains("Samples = " + threadCount);
String writeWaitingStat =
lock.getOMLockMetrics().getWriteLockWaitingTimeMsStat();
- assertTrue(writeWaitingStat.contains("Samples = " + threadCount),
- "Expected " + threadCount +
- " samples in writeLockWaitingTimeMsStat" + writeWaitingStat);
+ assertThat(writeWaitingStat).contains("Samples = " + threadCount);
}
- public void testSyntheticReadWriteLockConcurrentStats(
+ private void testSyntheticReadWriteLockConcurrentStats(
Resource resource, String[] resourceName,
int readThreadCount, int writeThreadCount)
throws InterruptedException {
@@ -525,49 +484,31 @@ public class TestOzoneManagerLock {
w.join();
}
- String readHeldStat = lock.getOMLockMetrics().getReadLockHeldTimeMsStat();
- assertTrue(readHeldStat.contains("Samples = " + readThreadCount),
- "Expected " + readThreadCount +
- " samples in readLockHeldTimeMsStat: " + readHeldStat);
+ final String readSamples = "Samples = " + readThreadCount;
+ assertThat(lock.getOMLockMetrics().getReadLockHeldTimeMsStat())
+ .contains(readSamples);
- String readWaitingStat =
- lock.getOMLockMetrics().getReadLockWaitingTimeMsStat();
- assertTrue(readWaitingStat.contains(
- "Samples = " + readThreadCount),
- "Expected " + readThreadCount +
- " samples in readLockWaitingTimeMsStat: " + readWaitingStat);
+ assertThat(lock.getOMLockMetrics().getReadLockWaitingTimeMsStat())
+ .contains(readSamples);
- String writeHeldStat =
lock.getOMLockMetrics().getWriteLockHeldTimeMsStat();
- assertTrue(writeHeldStat.contains(
- "Samples = " + writeThreadCount),
- "Expected " + writeThreadCount +
- " samples in writeLockHeldTimeMsStat: " + writeHeldStat);
+ final String writeSamples = "Samples = " + writeThreadCount;
+ assertThat(lock.getOMLockMetrics().getWriteLockHeldTimeMsStat())
+ .contains(writeSamples);
- String writeWaitingStat =
- lock.getOMLockMetrics().getWriteLockWaitingTimeMsStat();
- assertTrue(writeWaitingStat.contains(
- "Samples = " + writeThreadCount),
- "Expected " + writeThreadCount +
- " samples in writeLockWaitingTimeMsStat" + writeWaitingStat);
+ assertThat(lock.getOMLockMetrics().getWriteLockWaitingTimeMsStat())
+ .contains(writeSamples);
}
@Test
- public void testOMLockMetricsRecords() {
+ void testOMLockMetricsRecords() {
OMLockMetrics omLockMetrics = OMLockMetrics.create();
try {
MetricsCollectorImpl metricsCollector = new MetricsCollectorImpl();
omLockMetrics.getMetrics(metricsCollector, true);
- assertEquals(1, metricsCollector.getRecords().size());
-
- String omLockMetricsRecords = metricsCollector.getRecords().toString();
- assertTrue(omLockMetricsRecords.contains(
- "ReadLockWaitingTime"), omLockMetricsRecords);
- assertTrue(omLockMetricsRecords.contains("ReadLockHeldTime"),
- omLockMetricsRecords);
- assertTrue(omLockMetricsRecords.contains(
- "WriteLockWaitingTime"), omLockMetricsRecords);
- assertTrue(omLockMetricsRecords.contains("WriteLockHeldTime"),
- omLockMetricsRecords);
+ List<? extends MetricsRecord> metricsRecords =
metricsCollector.getRecords();
+ assertEquals(1, metricsRecords.size());
+ assertThat(metricsRecords.toString())
+ .contains("ReadLockWaitingTime", "ReadLockHeldTime",
"WriteLockWaitingTime", "WriteLockHeldTime");
} finally {
omLockMetrics.unRegister();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
