This is an automated email from the ASF dual-hosted git repository.
shoothzj pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/master by this push:
new f66e962314 Disable two tests in BookKeeperTest on JDK 17+ due to
UnsupportedOperationException (#4347)
f66e962314 is described below
commit f66e9623145fd54ddca5f6f6a6e31a06f7944fda
Author: ZhangJian He <[email protected]>
AuthorDate: Wed May 8 14:28:37 2024 +0800
Disable two tests in BookKeeperTest on JDK 17+ due to
UnsupportedOperationException (#4347)
### Motivation
JDK 17 and later throw `UnsupportedOperationException` for the `suspend`
and `resume` methods, causing the `testConstructionZkDelay` and
`testConstructionNotConnectedExplicitZk` tests to fail.
See jdk21 daily build error:
https://github.com/apache/bookkeeper/actions/runs/8962118072
### Changes
- Disabled the two affected test methods for JDK 17 and above using the
`@EnabledForJreRange` annotation.
- Transitioned to JUnit 5 annotations and assertions.
### Q&A
Q: why there are so many line changes? like format change
A: In junit5, the msg should be put in the latest param, related to some
changes.
Signed-off-by: ZhangJian He <[email protected]>
---
.../apache/bookkeeper/client/BookKeeperTest.java | 150 ++++++++++-----------
1 file changed, 71 insertions(+), 79 deletions(-)
diff --git
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTest.java
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTest.java
index fd0a2a9b76..78a762f8e1 100644
---
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTest.java
+++
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTest.java
@@ -23,13 +23,13 @@ package org.apache.bookkeeper.client;
import static
org.apache.bookkeeper.client.BookKeeperClientStats.WRITE_DELAYED_DUE_TO_NOT_ENOUGH_FAULT_DOMAINS;
import static
org.apache.bookkeeper.client.BookKeeperClientStats.WRITE_TIMED_OUT_DUE_TO_NOT_ENOUGH_FAULT_DOMAINS;
import static org.apache.bookkeeper.common.concurrent.FutureUtils.result;
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
import io.netty.util.IllegalReferenceCountException;
import java.io.IOException;
@@ -46,7 +46,6 @@ import java.util.concurrent.atomic.AtomicLong;
import org.apache.bookkeeper.client.AsyncCallback.AddCallback;
import org.apache.bookkeeper.client.AsyncCallback.ReadCallback;
import
org.apache.bookkeeper.client.BKException.BKBookieHandleNotAvailableException;
-import org.apache.bookkeeper.client.BKException.BKIllegalOpException;
import org.apache.bookkeeper.client.BookKeeper.DigestType;
import org.apache.bookkeeper.client.api.WriteFlag;
import org.apache.bookkeeper.client.api.WriteHandle;
@@ -71,7 +70,10 @@ import org.apache.zookeeper.Watcher.Event.KeeperState;
import org.apache.zookeeper.ZooKeeper;
import org.apache.zookeeper.ZooKeeper.States;
import org.apache.zookeeper.data.ACL;
-import org.junit.Test;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.EnabledForJreRange;
+import org.junit.jupiter.api.condition.JRE;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -89,6 +91,7 @@ public class BookKeeperTest extends BookKeeperClusterTestCase
{
}
@Test
+ @EnabledForJreRange(max = JRE.JAVA_17)
public void testConstructionZkDelay() throws Exception {
ClientConfiguration conf = new ClientConfiguration();
conf.setMetadataServiceUri(zkUtil.getMetadataServiceUri())
@@ -104,6 +107,7 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
}
@Test
+ @EnabledForJreRange(max = JRE.JAVA_17)
public void testConstructionNotConnectedExplicitZk() throws Exception {
ClientConfiguration conf = new ClientConfiguration();
conf.setMetadataServiceUri(zkUtil.getMetadataServiceUri())
@@ -117,7 +121,7 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
zkUtil.getZooKeeperConnectString(),
50,
event -> {});
- assertFalse("ZK shouldn't have connected yet",
zk.getState().isConnected());
+ assertFalse(zk.getState().isConnected(), "ZK shouldn't have connected
yet");
try {
BookKeeper bkc = new BookKeeper(conf, zk);
fail("Shouldn't be able to construct with unconnected zk");
@@ -262,8 +266,8 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
}
};
t.start();
- assertTrue("Close never completed", l.await(10, TimeUnit.SECONDS));
- assertTrue("Close was not successful", success.get());
+ assertTrue(l.await(10, TimeUnit.SECONDS), "Close never completed");
+ assertTrue(success.get(), "Close was not successful");
}
}
@@ -274,15 +278,15 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
BookKeeper bkc = new BookKeeper(conf);
LedgerHandle lh = bkc.createLedger(digestType,
"testPasswd".getBytes());
- Long lId = lh.getId();
+ long lId = lh.getId();
lh.addEntry("000".getBytes());
boolean result = bkc.isClosed(lId);
- assertTrue("Ledger shouldn't be flagged as closed!", !result);
+ assertFalse(result, "Ledger shouldn't be flagged as closed!");
lh.close();
result = bkc.isClosed(lId);
- assertTrue("Ledger should be flagged as closed!", result);
+ assertTrue(result, "Ledger should be flagged as closed!");
bkc.close();
}
@@ -346,9 +350,9 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
lh.addEntry("foobar".getBytes());
}
}
- assertTrue("Ledger should be closed!", bkc.isClosed(ledgerId));
+ assertTrue(bkc.isClosed(ledgerId), "Ledger should be closed!");
}
- assertTrue("BookKeeper should be closed!", bkc2.closed);
+ assertTrue(bkc2.closed, "BookKeeper should be closed!");
}
@Test
@@ -367,9 +371,8 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
try (BookKeeper bkReader = new BookKeeper(clientConfiguration);
LedgerHandle rlh = bkReader.openLedgerNoRecovery(ledgerId,
digestType, "testPasswd".getBytes())) {
- assertTrue(
- "Expected LAC of rlh: " + (numOfEntries - 2) + " actual
LAC of rlh: " + rlh.getLastAddConfirmed(),
- (rlh.getLastAddConfirmed() == (numOfEntries - 2)));
+ assertTrue((rlh.getLastAddConfirmed() == (numOfEntries - 2)),
"Expected LAC of rlh: "
+ + (numOfEntries - 2) + " actual LAC of rlh: " +
rlh.getLastAddConfirmed());
assertFalse(writeLh.isClosed());
@@ -379,18 +382,16 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
while (entries.hasMoreElements()) {
LedgerEntry entry = entries.nextElement();
String entryString = new String(entry.getEntry());
- assertTrue("Expected entry String: " + ("foobar" + entryId)
- + " actual entry String: " + entryString,
- entryString.equals("foobar" + entryId));
+ assertEquals(entryString, "foobar" + entryId, "Expected
entry String: " + ("foobar" + entryId)
+ + " actual entry String: " + entryString);
entryId++;
}
}
try (BookKeeper bkReader = new BookKeeper(clientConfiguration);
LedgerHandle rlh = bkReader.openLedgerNoRecovery(ledgerId,
digestType, "testPasswd".getBytes())) {
- assertTrue(
- "Expected LAC of rlh: " + (numOfEntries - 2) + " actual
LAC of rlh: " + rlh.getLastAddConfirmed(),
- (rlh.getLastAddConfirmed() == (numOfEntries - 2)));
+ assertTrue((rlh.getLastAddConfirmed() == (numOfEntries - 2)),
"Expected LAC of rlh: "
+ + (numOfEntries - 2) + " actual LAC of rlh: " +
rlh.getLastAddConfirmed());
assertFalse(writeLh.isClosed());
@@ -406,27 +407,24 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
Collections.list(rlh.readEntries(0,
rlh.getLastAddConfirmed())).size());
// assert local LAC does not change after reads
- assertTrue(
- "Expected LAC of rlh: " + (numOfEntries - 2) + " actual
LAC of rlh: " + rlh.getLastAddConfirmed(),
- (rlh.getLastAddConfirmed() == (numOfEntries - 2)));
+ assertTrue((rlh.getLastAddConfirmed() == (numOfEntries - 2)),
"Expected LAC of rlh: "
+ + (numOfEntries - 2) + " actual LAC of rlh: " +
rlh.getLastAddConfirmed());
// read all entries within the 0..LastAddConfirmed range with
readUnconfirmedEntries
assertEquals(rlh.getLastAddConfirmed() + 1,
Collections.list(rlh.readUnconfirmedEntries(0,
rlh.getLastAddConfirmed())).size());
// assert local LAC does not change after reads
- assertTrue(
- "Expected LAC of rlh: " + (numOfEntries - 2) + " actual
LAC of rlh: " + rlh.getLastAddConfirmed(),
- (rlh.getLastAddConfirmed() == (numOfEntries - 2)));
+ assertTrue((rlh.getLastAddConfirmed() == (numOfEntries - 2)),
"Expected LAC of rlh: "
+ + (numOfEntries - 2) + " actual LAC of rlh: " +
rlh.getLastAddConfirmed());
// read all entries within the LastAddConfirmed..numOfEntries
- 1 range with readUnconfirmedEntries
assertEquals(numOfEntries - rlh.getLastAddConfirmed(),
Collections.list(rlh.readUnconfirmedEntries(rlh.getLastAddConfirmed(),
numOfEntries - 1)).size());
// assert local LAC does not change after reads
- assertTrue(
- "Expected LAC of rlh: " + (numOfEntries - 2) + " actual
LAC of rlh: " + rlh.getLastAddConfirmed(),
- (rlh.getLastAddConfirmed() == (numOfEntries - 2)));
+ assertTrue((rlh.getLastAddConfirmed() == (numOfEntries - 2)),
"Expected LAC of rlh: "
+ + (numOfEntries - 2) + " actual LAC of rlh: " +
rlh.getLastAddConfirmed());
try {
// read all entries within the
LastAddConfirmed..numOfEntries range with readUnconfirmedEntries
@@ -455,9 +453,8 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
try (BookKeeper bkReader = new BookKeeper(clientConfiguration);
LedgerHandle rlh = bkReader.openLedgerNoRecovery(ledgerId,
digestType, "testPasswd".getBytes())) {
- assertTrue(
- "Expected LAC of rlh: " + (numOfEntries - 2) + " actual
LAC of rlh: " + rlh.getLastAddConfirmed(),
- (rlh.getLastAddConfirmed() == (numOfEntries - 2)));
+ assertTrue((rlh.getLastAddConfirmed() == (numOfEntries - 2)),
"Expected LAC of rlh: "
+ + (numOfEntries - 2) + " actual LAC of rlh: " +
rlh.getLastAddConfirmed());
assertFalse(writeLh.isClosed());
@@ -467,18 +464,16 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
while (entries.hasMoreElements()) {
LedgerEntry entry = entries.nextElement();
String entryString = new String(entry.getEntry());
- assertTrue("Expected entry String: " + ("foobar" + entryId)
- + " actual entry String: " + entryString,
- entryString.equals("foobar" + entryId));
+ assertEquals(entryString, "foobar" + entryId, "Expected
entry String: " + ("foobar" + entryId)
+ + " actual entry String: " + entryString);
entryId++;
}
}
try (BookKeeper bkReader = new BookKeeper(clientConfiguration);
LedgerHandle rlh = bkReader.openLedgerNoRecovery(ledgerId,
digestType, "testPasswd".getBytes())) {
- assertTrue(
- "Expected LAC of rlh: " + (numOfEntries - 2) + " actual
LAC of rlh: " + rlh.getLastAddConfirmed(),
- (rlh.getLastAddConfirmed() == (numOfEntries - 2)));
+ assertTrue((rlh.getLastAddConfirmed() == (numOfEntries - 2)),
"Expected LAC of rlh: "
+ + (numOfEntries - 2) + " actual LAC of rlh: " +
rlh.getLastAddConfirmed());
assertFalse(writeLh.isClosed());
@@ -494,27 +489,24 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
Collections.list(rlh.readEntries(0,
rlh.getLastAddConfirmed())).size());
// assert local LAC does not change after reads
- assertTrue(
- "Expected LAC of rlh: " + (numOfEntries - 2) + " actual
LAC of rlh: " + rlh.getLastAddConfirmed(),
- (rlh.getLastAddConfirmed() == (numOfEntries - 2)));
+ assertTrue((rlh.getLastAddConfirmed() == (numOfEntries - 2)),
"Expected LAC of rlh: "
+ + (numOfEntries - 2) + " actual LAC of rlh: " +
rlh.getLastAddConfirmed());
// read all entries within the 0..LastAddConfirmed range with
readUnconfirmedEntries
assertEquals(rlh.getLastAddConfirmed() + 1,
Collections.list(rlh.readUnconfirmedEntries(0,
rlh.getLastAddConfirmed())).size());
// assert local LAC does not change after reads
- assertTrue(
- "Expected LAC of rlh: " + (numOfEntries - 2) + " actual
LAC of rlh: " + rlh.getLastAddConfirmed(),
- (rlh.getLastAddConfirmed() == (numOfEntries - 2)));
+ assertTrue((rlh.getLastAddConfirmed() == (numOfEntries - 2)),
"Expected LAC of rlh: "
+ + (numOfEntries - 2) + " actual LAC of rlh: " +
rlh.getLastAddConfirmed());
// read all entries within the LastAddConfirmed..numOfEntries
- 1 range with readUnconfirmedEntries
assertEquals(numOfEntries - rlh.getLastAddConfirmed(),
Collections.list(rlh.readUnconfirmedEntries(rlh.getLastAddConfirmed(),
numOfEntries - 1)).size());
// assert local LAC does not change after reads
- assertTrue(
- "Expected LAC of rlh: " + (numOfEntries - 2) + " actual
LAC of rlh: " + rlh.getLastAddConfirmed(),
- (rlh.getLastAddConfirmed() == (numOfEntries - 2)));
+ assertTrue((rlh.getLastAddConfirmed() == (numOfEntries - 2)),
"Expected LAC of rlh: "
+ + (numOfEntries - 2) + " actual LAC of rlh: " +
rlh.getLastAddConfirmed());
try {
// read all entries within the
LastAddConfirmed..numOfEntries range with readUnconfirmedEntries
@@ -540,9 +532,8 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
// open ledger with fencing, this will repair the ledger and make
the last entry readable
try (BookKeeper bkReader = new BookKeeper(clientConfiguration);
LedgerHandle rlh = bkReader.openLedger(ledgerId, digestType,
"testPasswd".getBytes())) {
- assertTrue(
- "Expected LAC of rlh: " + (numOfEntries - 1) + " actual
LAC of rlh: " + rlh.getLastAddConfirmed(),
- (rlh.getLastAddConfirmed() == (numOfEntries - 1)));
+ assertTrue((rlh.getLastAddConfirmed() == (numOfEntries - 1)),
"Expected LAC of rlh: "
+ + (numOfEntries - 1) + " actual LAC of rlh: " +
rlh.getLastAddConfirmed());
assertFalse(writeLh.isClosed());
@@ -552,9 +543,8 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
while (entries.hasMoreElements()) {
LedgerEntry entry = entries.nextElement();
String entryString = new String(entry.getEntry());
- assertTrue("Expected entry String: " + ("foobar" + entryId)
- + " actual entry String: " + entryString,
- entryString.equals("foobar" + entryId));
+ assertEquals(entryString, "foobar" + entryId, "Expected
entry String: " + ("foobar" + entryId)
+ + " actual entry String: " + entryString);
entryId++;
}
}
@@ -859,8 +849,8 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
for (Enumeration<LedgerEntry> readEntries = lh.readEntries(0,
numEntries - 1);
readEntries.hasMoreElements();) {
LedgerEntry entry = readEntries.nextElement();
- assertTrue("Can't release entry " + entry.getEntryId() +
": ref = " + entry.data.refCnt(),
- entry.data.release());
+ assertTrue(entry.data.release(),
+ "Can't release entry " + entry.getEntryId() + ":
ref = " + entry.data.refCnt());
try {
assertFalse(entry.data.release());
fail("ByteBuf already released");
@@ -884,8 +874,8 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
readEntries.hasMoreElements();) {
LedgerEntry entry = readEntries.nextElement();
// ButeBufs not reference counter
- assertTrue("Can't release entry " + entry.getEntryId() +
": ref = " + entry.data.refCnt(),
- entry.data.release());
+ assertTrue(entry.data.release(),
+ "Can't release entry " + entry.getEntryId() + ":
ref = " + entry.data.refCnt());
try {
assertFalse(entry.data.release());
fail("ByteBuf already released");
@@ -984,7 +974,7 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
bkc.close();
}
- @Test(expected = BKIllegalOpException.class)
+ @Test
public void testCannotUseWriteFlagsOnV2Protocol() throws Exception {
ClientConfiguration conf = new ClientConfiguration(baseClientConf);
conf.setUseV2WireProtocol(true);
@@ -996,12 +986,13 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
.withPassword("".getBytes())
.withWriteFlags(WriteFlag.DEFERRED_SYNC)
.execute())) {
- result(wh.appendAsync("test".getBytes()));
+ Assertions.assertThrows(BKException.BKIllegalOpException.class,
+ () -> result(wh.appendAsync("test".getBytes())));
}
}
}
- @Test(expected = BKIllegalOpException.class)
+ @Test
public void testCannotUseForceOnV2Protocol() throws Exception {
ClientConfiguration conf = new ClientConfiguration(baseClientConf);
conf.setUseV2WireProtocol(true);
@@ -1013,8 +1004,9 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
.withPassword("".getBytes())
.withWriteFlags(WriteFlag.NONE)
.execute())) {
- result(wh.appendAsync("".getBytes()));
- result(wh.force());
+ result(wh.appendAsync("".getBytes()));
+ Assertions.assertThrows(BKException.BKIllegalOpException.class,
+ () -> result(wh.force()));
}
}
}
@@ -1085,8 +1077,8 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
MockZooKeeperClient zkFaultInjectionWrapper = new
MockZooKeeperClient(zkUtil.getZooKeeperConnectString(),
zkSessionTimeOut, zooKeeperWatcherBase,
ledgerIdToInjectFailure);
zkFaultInjectionWrapper.waitForConnection();
- assertEquals("zkFaultInjectionWrapper should be in connected state",
States.CONNECTED,
- zkFaultInjectionWrapper.getState());
+ assertEquals(States.CONNECTED, zkFaultInjectionWrapper.getState(),
+ "zkFaultInjectionWrapper should be in connected state");
BookKeeper bk = new BookKeeper(baseClientConf,
zkFaultInjectionWrapper);
long oldZkInstanceSessionId = zkFaultInjectionWrapper.getSessionId();
long ledgerId = 567L;
@@ -1106,10 +1098,10 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
}
Thread.sleep(200);
}
- assertEquals("zkFaultInjectionWrapper should be in connected state",
States.CONNECTED,
- zkFaultInjectionWrapper.getState());
- assertNotEquals("Session Id of old and new ZK instance should be
different", oldZkInstanceSessionId,
- zkFaultInjectionWrapper.getSessionId());
+ assertEquals(States.CONNECTED, zkFaultInjectionWrapper.getState(),
+ "zkFaultInjectionWrapper should be in connected state");
+ assertNotEquals(oldZkInstanceSessionId,
zkFaultInjectionWrapper.getSessionId(),
+ "Session Id of old and new ZK instance should be different");
ledgerId++;
ledgerIdToInjectFailure.set(ledgerId);
/**
@@ -1122,8 +1114,8 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
*/
lh = bk.createLedgerAdv(ledgerId, 1, 1, 1, DigestType.CRC32,
"".getBytes(), null);
lh.close();
- assertEquals("injectZnodeCreationNoNodeFailure should have been reset
it to INVALID_LEDGERID", INVALID_LEDGERID,
- ledgerIdToInjectFailure.get());
+ assertEquals(INVALID_LEDGERID, ledgerIdToInjectFailure.get(),
+ "injectZnodeCreationNoNodeFailure should have been reset it to
INVALID_LEDGERID");
lh = bk.openLedger(ledgerId, DigestType.CRC32, "".getBytes());
lh.close();
ledgerId++;
@@ -1245,14 +1237,14 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
// Waiting and checking to make sure that write has not succeeded
countDownLatch.await(conf.getAddEntryTimeout(), TimeUnit.SECONDS);
- assertEquals("Write succeeded but should not have", -1,
lh.lastAddConfirmed);
+ assertEquals(-1, lh.lastAddConfirmed, "Write succeeded but should
not have");
// Wake the bookie
sleepLatchCase1.countDown();
// Waiting and checking to make sure that write has succeeded
writeToLedger.join(conf.getAddEntryTimeout() * 1000);
- assertEquals("Write did not succeed but should have", 0,
lh.lastAddConfirmed);
+ assertEquals(0, lh.lastAddConfirmed, "Write did not succeed but
should have");
assertEquals(statsLogger
.getCounter(WRITE_DELAYED_DUE_TO_NOT_ENOUGH_FAULT_DOMAINS)
@@ -1279,7 +1271,7 @@ public class BookKeeperTest extends
BookKeeperClusterTestCase {
// Waiting and checking to make sure that write has failed
writeToLedger2.join((conf.getAddEntryQuorumTimeout() + 2) * 1000);
- assertEquals("Write succeeded but should not have", 0,
lh.lastAddConfirmed);
+ assertEquals(0, lh.lastAddConfirmed, "Write succeeded but should
not have");
sleepLatchCase2.countDown();
