Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: migrate client module api tests to junit 5 #4377

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

test: migrate client module api tests to junit 5 #4377

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
859af11
migrate client module bookie tests to junit 5
sherlock-lin May 17, 2024
03df884
Merge branch 'master' into sherlock-migrate-to-junit5-0516
sherlock-lin May 23, 2024
84335b9
migrate client module bookie tests to junit 5
sherlock-lin May 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MockBookKeeperTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@
import org.apache.bookkeeper.versioning.Versioned;
import org.junit.After;
import org.junit.Before;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.mockito.stubbing.Stubber;
Expand Down Expand Up @@ -141,6 +143,7 @@ public MockEntry(byte[] payload, long lastAddConfirmed) {

}

@BeforeEach
@Before
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about add some todo here: // TODO: delete @Before when only run with junit5

public void setup() throws Exception {
maxNumberOfAvailableBookies = Integer.MAX_VALUE;
Expand Down Expand Up @@ -264,6 +267,7 @@ private DigestManager getDigestType(long ledgerId) throws GeneralSecurityExcepti
UnpooledByteBufAllocator.DEFAULT, false);
}

@AfterEach
@After
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about add some todo here: // TODO: delete @Before when only run with junit5

public void tearDown() {
scheduler.shutdown();
Expand Down
8 changes: 4 additions & 4 deletions bookkeeper-server/src/test/java/org/apache/bookkeeper/client/api/BKExceptionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@
package org.apache.bookkeeper.client.api;

import static org.apache.bookkeeper.client.api.BKException.Code.UnexpectedConditionException;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.lang.reflect.Field;
import org.junit.Test;
import org.junit.jupiter.api.Test;

/**
* Tests for BKException methods.
Expand Down
190 changes: 100 additions & 90 deletions bookkeeper-server/src/test/java/org/apache/bookkeeper/client/api/BookKeeperApiTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,13 @@
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.hasProperty;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
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.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;

import io.netty.buffer.Unpooled;
import java.nio.ByteBuffer;
Expand All @@ -47,7 +48,7 @@
import org.apache.bookkeeper.conf.ClientConfiguration;
import org.apache.bookkeeper.util.LoggerOutput;
import org.junit.Rule;
import org.junit.Test;
import org.junit.jupiter.api.Test;
import org.slf4j.event.LoggingEvent;

/**
Expand Down Expand Up @@ -128,41 +129,45 @@ public void testWriteAdvHandleWithFixedLedgerId() throws Exception {
}
}

@Test(expected = BKDuplicateEntryIdException.class)
@Test
public void testWriteAdvHandleBKDuplicateEntryId() throws Exception {
try (WriteAdvHandle writer = result(newCreateLedgerOp()
.withAckQuorumSize(1)
.withWriteQuorumSize(2)
.withEnsembleSize(3)
.withPassword(password)
.makeAdv()
.withLedgerId(1234)
.execute())) {
assertEquals(1234, writer.getId());
long entryId = 0;
writer.write(entryId++, ByteBuffer.wrap(data));
assertEquals(data.length, writer.getLength());
writer.write(entryId - 1, ByteBuffer.wrap(data));
}
assertThrows(BKDuplicateEntryIdException.class, () -> {
try (WriteAdvHandle writer = result(newCreateLedgerOp()
.withAckQuorumSize(1)
.withWriteQuorumSize(2)
.withEnsembleSize(3)
.withPassword(password)
.makeAdv()
.withLedgerId(1234)
.execute())) {
assertEquals(1234, writer.getId());
long entryId = 0;
writer.write(entryId++, ByteBuffer.wrap(data));
assertEquals(data.length, writer.getLength());
writer.write(entryId - 1, ByteBuffer.wrap(data));
}
});
}

@Test(expected = BKUnauthorizedAccessException.class)
@Test
public void testOpenLedgerUnauthorized() throws Exception {
long lId;
try (WriteHandle writer = result(newCreateLedgerOp()
.withAckQuorumSize(1)
.withWriteQuorumSize(2)
.withEnsembleSize(3)
.withPassword(password)
.execute())) {
lId = writer.getId();
assertEquals(-1L, writer.getLastAddPushed());
}
try (ReadHandle ignored = result(newOpenLedgerOp()
.withPassword("bad-password".getBytes(UTF_8))
.withLedgerId(lId)
.execute())) {
}
assertThrows(BKUnauthorizedAccessException.class, () -> {
long lId;
try (WriteHandle writer = result(newCreateLedgerOp()
.withAckQuorumSize(1)
.withWriteQuorumSize(2)
.withEnsembleSize(3)
.withPassword(password)
.execute())) {
lId = writer.getId();
assertEquals(-1L, writer.getLastAddPushed());
}
try (ReadHandle ignored = result(newOpenLedgerOp()
.withPassword("bad-password".getBytes(UTF_8))
.withLedgerId(lId)
.execute())) {
}
});
}

/**
Expand Down Expand Up @@ -301,75 +306,80 @@ public void testOpenLedgerRead() throws Exception {
}
}

@Test(expected = BKLedgerFencedException.class)
@Test
public void testOpenLedgerWithRecovery() throws Exception {
assertThrows(BKLedgerFencedException.class, () -> {
loggerOutput.expect((List<LoggingEvent> logEvents) -> {
assertThat(logEvents, hasItem(hasProperty("message",
containsString("due to LedgerFencedException: "
+ "Ledger has been fenced off. Some other client must have opened it to read")
)));
});

loggerOutput.expect((List<LoggingEvent> logEvents) -> {
assertThat(logEvents, hasItem(hasProperty("message",
containsString("due to LedgerFencedException: "
+ "Ledger has been fenced off. Some other client must have opened it to read")
)));
});

long lId;
try (WriteHandle writer = result(newCreateLedgerOp()
.withAckQuorumSize(1)
.withWriteQuorumSize(2)
.withEnsembleSize(3)
.withPassword(password)
.execute())) {
lId = writer.getId();

writer.append(ByteBuffer.wrap(data));
writer.append(ByteBuffer.wrap(data));
assertEquals(1L, writer.getLastAddPushed());

// open with fencing
try (ReadHandle reader = result(newOpenLedgerOp()
long lId;
try (WriteHandle writer = result(newCreateLedgerOp()
.withAckQuorumSize(1)
.withWriteQuorumSize(2)
.withEnsembleSize(3)
.withPassword(password)
.withRecovery(true)
.withLedgerId(lId)
.execute())) {
assertTrue(reader.isClosed());
assertEquals(1L, reader.getLastAddConfirmed());
}
lId = writer.getId();

writer.append(ByteBuffer.wrap(data));
writer.append(ByteBuffer.wrap(data));
writer.append(ByteBuffer.wrap(data));
assertEquals(1L, writer.getLastAddPushed());

// open with fencing
try (ReadHandle reader = result(newOpenLedgerOp()
.withPassword(password)
.withRecovery(true)
.withLedgerId(lId)
.execute())) {
assertTrue(reader.isClosed());
assertEquals(1L, reader.getLastAddConfirmed());
}

}
writer.append(ByteBuffer.wrap(data));

}
});
}

@Test(expected = BKNoSuchLedgerExistsOnMetadataServerException.class)
@Test
public void testDeleteLedger() throws Exception {
long lId;
assertThrows(BKNoSuchLedgerExistsOnMetadataServerException.class, () -> {
long lId;

try (WriteHandle writer = result(newCreateLedgerOp()
.withPassword(password)
.execute())) {
lId = writer.getId();
assertEquals(-1L, writer.getLastAddPushed());
}
try (WriteHandle writer = result(newCreateLedgerOp()
.withPassword(password)
.execute())) {
lId = writer.getId();
assertEquals(-1L, writer.getLastAddPushed());
}

result(newDeleteLedgerOp().withLedgerId(lId).execute());
result(newDeleteLedgerOp().withLedgerId(lId).execute());

result(newOpenLedgerOp()
.withPassword(password)
.withLedgerId(lId)
.execute());
result(newOpenLedgerOp()
.withPassword(password)
.withLedgerId(lId)
.execute());
});
}

@Test(expected = BKNoSuchLedgerExistsOnMetadataServerException.class)
@Test
public void testCannotDeleteLedgerTwice() throws Exception {
long lId;
assertThrows(BKNoSuchLedgerExistsOnMetadataServerException.class, () -> {
long lId;

try (WriteHandle writer = result(newCreateLedgerOp()
.withPassword(password)
.execute())) {
lId = writer.getId();
assertEquals(-1L, writer.getLastAddPushed());
}
result(newDeleteLedgerOp().withLedgerId(lId).execute());
result(newDeleteLedgerOp().withLedgerId(lId).execute());
try (WriteHandle writer = result(newCreateLedgerOp()
.withPassword(password)
.execute())) {
lId = writer.getId();
assertEquals(-1L, writer.getLastAddPushed());
}
result(newDeleteLedgerOp().withLedgerId(lId).execute());
result(newDeleteLedgerOp().withLedgerId(lId).execute());
});
}

@Test
Expand Down
19 changes: 9 additions & 10 deletions ...rver/src/test/java/org/apache/bookkeeper/client/api/BookKeeperBuildersOpenLedgerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,12 @@
import org.apache.bookkeeper.net.BookieId;
import org.apache.bookkeeper.proto.BookieProtocol;
import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

/**
* Tests for BookKeeper open ledger operations.
*/
@RunWith(Parameterized.class)
public class BookKeeperBuildersOpenLedgerTest extends MockBookKeeperTestCase {

private static final int ensembleSize = 3;
Expand All @@ -58,20 +56,20 @@ public class BookKeeperBuildersOpenLedgerTest extends MockBookKeeperTestCase {

private boolean withRecovery;

public BookKeeperBuildersOpenLedgerTest(boolean withRecovery) {
public void initBookKeeperBuildersOpenLedgerTest(boolean withRecovery) {
this.withRecovery = withRecovery;
}

@Parameterized.Parameters(name = "withRecovery:({0})")
public static Collection<Object[]> data() {
return Arrays.asList(new Object[][]{
{true},
{false}
});
}

@Test
public void testOpenLedger() throws Exception {
@MethodSource("data")
Copy link
Member

@lhotari lhotari May 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In junit5, it's better to use @ValueSource(booleans = {true, false}) for cases where there's a single boolean parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lhotari Thanks，It's a better choice.

@ParameterizedTest(name = "withRecovery:({0})")
public void testOpenLedger(boolean withRecovery) throws Exception {
LedgerMetadata ledgerMetadata = generateLedgerMetadata(ensembleSize,
writeQuorumSize, ackQuorumSize, password, customMetadata);
registerMockLedgerMetadata(ledgerId, ledgerMetadata);
Expand All @@ -91,8 +89,9 @@ public void testOpenLedger() throws Exception {
.execute());
}

@Test
public void testOpenLedgerWithTimeoutEx() throws Exception {
@MethodSource("data")
@ParameterizedTest(name = "withRecovery:({0})")
public void testOpenLedgerWithTimeoutEx(boolean withRecovery) throws Exception {
mockReadEntryTimeout();
LedgerMetadata ledgerMetadata = generateLedgerMetadata(ensembleSize,
writeQuorumSize, ackQuorumSize, password, customMetadata);
Expand Down
Loading