This is an automated email from the ASF dual-hosted git repository.
xxubai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/amoro.git
The following commit(s) were added to refs/heads/master by this push:
new 4d8615160 [AMORO-3974][common] Migrate self-contained tests from JUnit
4 to JUnit 5 (#4199)
4d8615160 is described below
commit 4d861516082a4d1cfa36c6a723a03f96157c84c3
Author: Darcy <[email protected]>
AuthorDate: Thu May 7 10:27:49 2026 +0800
[AMORO-3974][common] Migrate self-contained tests from JUnit 4 to JUnit 5
(#4199)
Migrate the JUnit 4 test classes in amoro-common that have no
external JUnit 4 consumers to JUnit 5 (org.junit.jupiter):
- TestLocalExecutionEngine
- TestSimpleFuture
- JacksonUtilTest
- MemorySizeTest
- TestPoolConfig
- MockZookeeperServer
Shared test base classes and rule helpers (TestAms, TestHMS,
AmoroCatalogTestBase, TestAmoroCatalogBase, TestServerTableDescriptor,
AmoroRunListener) are kept on JUnit 4 because they are still extended
or referenced as @Rule/@ClassRule by JUnit 4 tests in other modules
(amoro-ams, amoro-format-iceberg, amoro-format-paimon,
amoro-format-mixed-flink, etc.). They will be migrated together with
those consumers in a follow-up so the JUnit 4 dependency can be dropped.
Co-authored-by: Lin Tingbin <[email protected]>
---
.../java/org/apache/amoro/MockZookeeperServer.java | 2 +-
.../org/apache/amoro/client/TestPoolConfig.java | 24 +++---
.../amoro/process/TestLocalExecutionEngine.java | 28 +++----
.../org/apache/amoro/process/TestSimpleFuture.java | 88 ++++++++++++----------
.../org/apache/amoro/utils/JacksonUtilTest.java | 10 +--
.../org/apache/amoro/utils/MemorySizeTest.java | 33 ++++----
6 files changed, 96 insertions(+), 89 deletions(-)
diff --git
a/amoro-common/src/test/java/org/apache/amoro/MockZookeeperServer.java
b/amoro-common/src/test/java/org/apache/amoro/MockZookeeperServer.java
index 95189c018..43c27a788 100644
--- a/amoro-common/src/test/java/org/apache/amoro/MockZookeeperServer.java
+++ b/amoro-common/src/test/java/org/apache/amoro/MockZookeeperServer.java
@@ -22,7 +22,7 @@ import
org.apache.amoro.shade.zookeeper3.org.apache.curator.framework.CuratorFra
import
org.apache.amoro.shade.zookeeper3.org.apache.curator.framework.CuratorFrameworkFactory;
import
org.apache.amoro.shade.zookeeper3.org.apache.curator.retry.ExponentialBackoffRetry;
import org.apache.curator.test.TestingServer;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
import java.io.IOException;
import java.util.Random;
diff --git
a/amoro-common/src/test/java/org/apache/amoro/client/TestPoolConfig.java
b/amoro-common/src/test/java/org/apache/amoro/client/TestPoolConfig.java
index a36f9ea79..d690e2d87 100644
--- a/amoro-common/src/test/java/org/apache/amoro/client/TestPoolConfig.java
+++ b/amoro-common/src/test/java/org/apache/amoro/client/TestPoolConfig.java
@@ -18,8 +18,8 @@
package org.apache.amoro.client;
-import org.junit.Assert;
-import org.junit.Test;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
public class TestPoolConfig {
@@ -30,14 +30,14 @@ public class TestPoolConfig {
+ "&maxReconnects=3&minIdle=10&maxIdle=10&maxWaitMillis=500";
PoolConfig<?> poolConfig = PoolConfig.forUrl(url);
- Assert.assertEquals(5000, poolConfig.getConnectTimeout());
- Assert.assertEquals(6000, poolConfig.getSocketTimeout());
- Assert.assertEquals(100, poolConfig.getMaxMessageSize());
- Assert.assertFalse(poolConfig.isAutoReconnect());
- Assert.assertEquals(3, poolConfig.getMaxReconnects());
- Assert.assertEquals(10, poolConfig.getMinIdle());
- Assert.assertEquals(10, poolConfig.getMaxIdle());
- Assert.assertEquals(500, poolConfig.getMaxWaitMillis());
+ Assertions.assertEquals(5000, poolConfig.getConnectTimeout());
+ Assertions.assertEquals(6000, poolConfig.getSocketTimeout());
+ Assertions.assertEquals(100, poolConfig.getMaxMessageSize());
+ Assertions.assertFalse(poolConfig.isAutoReconnect());
+ Assertions.assertEquals(3, poolConfig.getMaxReconnects());
+ Assertions.assertEquals(10, poolConfig.getMinIdle());
+ Assertions.assertEquals(10, poolConfig.getMaxIdle());
+ Assertions.assertEquals(500, poolConfig.getMaxWaitMillis());
}
@Test
@@ -45,13 +45,13 @@ public class TestPoolConfig {
// We will ignore parameters with unknown name
String url = "thrift://127.0.0.1:1261?connectTimeouts=300";
PoolConfig<?> poolConfig = PoolConfig.forUrl(url);
- Assert.assertEquals(0, poolConfig.getConnectTimeout());
+ Assertions.assertEquals(0, poolConfig.getConnectTimeout());
}
@Test
public void testUrlFormatError() {
String url = "thrift://127.0.0.1:1261?connectTimeout=5000& ";
- Assert.assertThrows(
+ Assertions.assertThrows(
IllegalArgumentException.class,
() -> {
PoolConfig.forUrl(url);
diff --git
a/amoro-common/src/test/java/org/apache/amoro/process/TestLocalExecutionEngine.java
b/amoro-common/src/test/java/org/apache/amoro/process/TestLocalExecutionEngine.java
index d6930622d..6c67a2097 100644
---
a/amoro-common/src/test/java/org/apache/amoro/process/TestLocalExecutionEngine.java
+++
b/amoro-common/src/test/java/org/apache/amoro/process/TestLocalExecutionEngine.java
@@ -22,9 +22,9 @@ import static org.mockito.Mockito.mock;
import org.apache.amoro.Action;
import org.apache.amoro.TableRuntime;
-import org.junit.After;
-import org.junit.Assert;
-import org.junit.Test;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
import java.util.HashMap;
import java.util.Map;
@@ -36,7 +36,7 @@ public class TestLocalExecutionEngine {
private LocalExecutionEngine engine;
- @After
+ @AfterEach
public void tearDown() {
if (engine != null) {
engine.close();
@@ -62,10 +62,10 @@ public class TestLocalExecutionEngine {
String identifier = engine.submitTableProcess(process);
- Assert.assertTrue("process should start", started.await(5,
TimeUnit.SECONDS));
- Assert.assertTrue(
- "should run in custom pool",
- threadName.get() != null &&
threadName.get().startsWith("local-snapshots-expiring-"));
+ Assertions.assertTrue(started.await(5, TimeUnit.SECONDS), "process should
start");
+ Assertions.assertTrue(
+ threadName.get() != null &&
threadName.get().startsWith("local-snapshots-expiring-"),
+ "should run in custom pool");
waitForStatus(identifier, ProcessStatus.SUCCESS, 5000);
}
@@ -91,7 +91,7 @@ public class TestLocalExecutionEngine {
String identifier = engine.submitTableProcess(process);
// Process may not be scheduled yet, but holder is already created.
- Assert.assertEquals(ProcessStatus.RUNNING, engine.getStatus(identifier));
+ Assertions.assertEquals(ProcessStatus.RUNNING,
engine.getStatus(identifier));
engine.tryCancelTableProcess(process, identifier);
@@ -113,7 +113,7 @@ public class TestLocalExecutionEngine {
// Wait until process info should be expired.
Thread.sleep(200);
- Assert.assertEquals(ProcessStatus.UNKNOWN, engine.getStatus(identifier));
+ Assertions.assertEquals(ProcessStatus.UNKNOWN,
engine.getStatus(identifier));
}
@Test
@@ -138,9 +138,9 @@ public class TestLocalExecutionEngine {
public void testGetStatusForInvalidIdentifier() {
engine = createEngineWithTtl("1h");
- Assert.assertEquals(ProcessStatus.UNKNOWN, engine.getStatus(null));
- Assert.assertEquals(ProcessStatus.UNKNOWN, engine.getStatus(""));
- Assert.assertEquals(ProcessStatus.UNKNOWN, engine.getStatus("not-exist"));
+ Assertions.assertEquals(ProcessStatus.UNKNOWN, engine.getStatus(null));
+ Assertions.assertEquals(ProcessStatus.UNKNOWN, engine.getStatus(""));
+ Assertions.assertEquals(ProcessStatus.UNKNOWN,
engine.getStatus("not-exist"));
}
private LocalExecutionEngine createEngineWithTtl(String ttl) {
@@ -163,7 +163,7 @@ public class TestLocalExecutionEngine {
}
Thread.sleep(10);
}
- Assert.fail(
+ Assertions.fail(
"Timeout waiting for status " + expected + ", current=" +
engine.getStatus(identifier));
}
diff --git
a/amoro-common/src/test/java/org/apache/amoro/process/TestSimpleFuture.java
b/amoro-common/src/test/java/org/apache/amoro/process/TestSimpleFuture.java
index 7f5c354e3..373aa3cc8 100644
--- a/amoro-common/src/test/java/org/apache/amoro/process/TestSimpleFuture.java
+++ b/amoro-common/src/test/java/org/apache/amoro/process/TestSimpleFuture.java
@@ -23,9 +23,9 @@ import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import org.apache.amoro.exception.AmoroRuntimeException;
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import java.util.Arrays;
import java.util.List;
@@ -43,7 +43,7 @@ public class TestSimpleFuture {
calledFlag = new boolean[] {false, false, false, false, false};
}
- @Before
+ @BeforeEach
public void setUp() {
resetCallbackData();
simpleFuture = new SimpleFuture();
@@ -53,18 +53,18 @@ public class TestSimpleFuture {
simpleFuture.whenCompleted(
() -> {
for (int j = num; j < calledFlag.length; j++) {
- Assert.assertFalse(
- "callback " + j + " should not be called before " + num,
calledFlag[j]);
+ Assertions.assertFalse(
+ calledFlag[j], "callback " + j + " should not be called
before " + num);
}
// Trigger error if callbackNum[num] == 0
if (callbackNum[num] == 0) {
throw new RuntimeException("Callback error");
}
callbackNum[num] = num;
- Assert.assertEquals(
- "Callback should be run in the same thread",
+ Assertions.assertEquals(
threadId,
- Thread.currentThread().getId());
+ Thread.currentThread().getId(),
+ "Callback should be run in the same thread");
});
}
}
@@ -74,9 +74,10 @@ public class TestSimpleFuture {
simpleFuture.complete();
for (int i = 0; i < 5; i++) {
- Assert.assertEquals("Current callback num: " + i, i, callbackNum[i]);
+ Assertions.assertEquals(i, callbackNum[i], "Current callback num: " + i);
}
- Assert.assertTrue("SimpleFuture should complete if callback no error",
simpleFuture.isDone());
+ Assertions.assertTrue(
+ simpleFuture.isDone(), "SimpleFuture should complete if callback no
error");
}
@Test
@@ -88,9 +89,10 @@ public class TestSimpleFuture {
}
simpleFuture.complete();
for (int i = 0; i < 5; i++) {
- Assert.assertEquals("Current callback num: " + i, -1, callbackNum[i]);
+ Assertions.assertEquals(-1, callbackNum[i], "Current callback num: " +
i);
}
- Assert.assertTrue("SimpleFuture should not complete if callback error",
simpleFuture.isDone());
+ Assertions.assertTrue(
+ simpleFuture.isDone(), "SimpleFuture should not complete if callback
error");
}
// Additional tests for edge cases and error conditions
@@ -101,21 +103,23 @@ public class TestSimpleFuture {
try {
simpleFuture.complete();
} catch (Throwable throwable) {
- Assert.assertTrue("Should catch the error", throwable instanceof
AmoroRuntimeException);
- Assert.assertTrue("Should catch the error", throwable.getCause()
instanceof RuntimeException);
- Assert.assertEquals(
- "Should catch the error", "Callback error",
throwable.getCause().getMessage());
+ Assertions.assertTrue(throwable instanceof AmoroRuntimeException,
"Should catch the error");
+ Assertions.assertTrue(
+ throwable.getCause() instanceof RuntimeException, "Should catch the
error");
+ Assertions.assertEquals(
+ "Callback error", throwable.getCause().getMessage(), "Should catch
the error");
}
for (int i = 0; i < 5; i++) {
if (i < 2) {
- Assert.assertEquals("Current callback num: " + i, i, callbackNum[i]);
+ Assertions.assertEquals(i, callbackNum[i], "Current callback num: " +
i);
} else if (i == 2) {
- Assert.assertEquals("Current callback num: " + i, 0, callbackNum[i]);
+ Assertions.assertEquals(0, callbackNum[i], "Current callback num: " +
i);
} else {
- Assert.assertEquals("Current callback num: " + i, -1, callbackNum[i]);
+ Assertions.assertEquals(-1, callbackNum[i], "Current callback num: " +
i);
}
}
- Assert.assertFalse("SimpleFuture should not complete if callback error",
simpleFuture.isDone());
+ Assertions.assertFalse(
+ simpleFuture.isDone(), "SimpleFuture should not complete if callback
error");
}
@Test
@@ -124,40 +128,42 @@ public class TestSimpleFuture {
try {
simpleFuture.complete();
} catch (Throwable throwable) {
- Assert.assertTrue("Should catch the error", throwable instanceof
AmoroRuntimeException);
+ Assertions.assertTrue(throwable instanceof AmoroRuntimeException,
"Should catch the error");
}
- Assert.assertFalse("SimpleFuture should not complete if callback error",
simpleFuture.isDone());
+ Assertions.assertFalse(
+ simpleFuture.isDone(), "SimpleFuture should not complete if callback
error");
resetCallbackData(); // Trigger normal callback
simpleFuture.reset();
simpleFuture.complete();
for (int i = 0; i < 5; i++) {
- Assert.assertEquals("Current callback num: " + i, i, callbackNum[i]);
+ Assertions.assertEquals(i, callbackNum[i], "Current callback num: " + i);
}
- Assert.assertTrue("SimpleFuture should not complete if callback error",
simpleFuture.isDone());
+ Assertions.assertTrue(
+ simpleFuture.isDone(), "SimpleFuture should not complete if callback
error");
}
@Test
public void testIsDone() {
simpleFuture.complete();
- Assert.assertTrue("Future should be completed", simpleFuture.isDone());
+ Assertions.assertTrue(simpleFuture.isDone(), "Future should be completed");
}
- @Test(expected = AmoroRuntimeException.class)
+ @Test
public void testCompleteException() throws ExecutionException,
InterruptedException {
CompletableFuture<?> future = mock(CompletableFuture.class);
doReturn(true).when(future).complete(null);
doThrow(new RuntimeException()).when(future).get();
SimpleFuture simpleFuture = new SimpleFuture(future);
- simpleFuture.complete();
+ Assertions.assertThrows(AmoroRuntimeException.class,
simpleFuture::complete);
}
@Test
public void testJoin() {
simpleFuture.complete();
simpleFuture.join();
- Assert.assertTrue("Future should be completed", simpleFuture.isDone());
+ Assertions.assertTrue(simpleFuture.isDone(), "Future should be completed");
}
@Test
@@ -168,7 +174,7 @@ public class TestSimpleFuture {
simpleFuture.reset();
simpleFuture.complete();
simpleFuture.join();
- Assert.assertTrue("Future should be completed", simpleFuture.isDone());
+ Assertions.assertTrue(simpleFuture.isDone(), "Future should be completed");
}
@Test
@@ -177,9 +183,9 @@ public class TestSimpleFuture {
SimpleFuture combinedFuture = simpleFuture.or(anotherFuture);
simpleFuture.complete();
- Assert.assertTrue(
- "Combined future should be completed when either future completes",
- combinedFuture.isDone());
+ Assertions.assertTrue(
+ combinedFuture.isDone(),
+ "Combined future should be completed when either future completes");
}
@Test
@@ -189,8 +195,8 @@ public class TestSimpleFuture {
simpleFuture.complete();
anotherFuture.complete();
- Assert.assertTrue(
- "Combined future should be completed when both futures complete",
combinedFuture.isDone());
+ Assertions.assertTrue(
+ combinedFuture.isDone(), "Combined future should be completed when
both futures complete");
}
@Test
@@ -200,8 +206,8 @@ public class TestSimpleFuture {
SimpleFuture combinedFuture = SimpleFuture.allOf(futures);
futures.forEach(SimpleFuture::complete);
- Assert.assertTrue(
- "Combined future should be completed when all futures complete",
combinedFuture.isDone());
+ Assertions.assertTrue(
+ combinedFuture.isDone(), "Combined future should be completed when all
futures complete");
}
@Test
@@ -210,8 +216,8 @@ public class TestSimpleFuture {
SimpleFuture combinedFuture = SimpleFuture.anyOf(futures);
futures.get(0).complete();
- Assert.assertTrue(
- "Combined future should be completed when any future completes",
combinedFuture.isDone());
+ Assertions.assertTrue(
+ combinedFuture.isDone(), "Combined future should be completed when any
future completes");
}
// Test for when the future is already completed before calling complete()
@@ -221,7 +227,7 @@ public class TestSimpleFuture {
try {
simpleFuture.complete();
} catch (Throwable throwable) {
- Assert.fail(throwable.getMessage());
+ Assertions.fail(throwable.getMessage());
}
}
@@ -232,7 +238,7 @@ public class TestSimpleFuture {
try {
simpleFuture.join();
} catch (Throwable throwable) {
- Assert.fail(throwable.getMessage());
+ Assertions.fail(throwable.getMessage());
}
}
}
diff --git
a/amoro-common/src/test/java/org/apache/amoro/utils/JacksonUtilTest.java
b/amoro-common/src/test/java/org/apache/amoro/utils/JacksonUtilTest.java
index c4c1e30af..d9e559a7a 100644
--- a/amoro-common/src/test/java/org/apache/amoro/utils/JacksonUtilTest.java
+++ b/amoro-common/src/test/java/org/apache/amoro/utils/JacksonUtilTest.java
@@ -18,13 +18,13 @@
package org.apache.amoro.utils;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.amoro.shade.jackson2.com.fasterxml.jackson.databind.JsonNode;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
import java.util.ArrayList;
import java.util.Collections;
diff --git
a/amoro-common/src/test/java/org/apache/amoro/utils/MemorySizeTest.java
b/amoro-common/src/test/java/org/apache/amoro/utils/MemorySizeTest.java
index f65454e81..402b14171 100644
--- a/amoro-common/src/test/java/org/apache/amoro/utils/MemorySizeTest.java
+++ b/amoro-common/src/test/java/org/apache/amoro/utils/MemorySizeTest.java
@@ -19,13 +19,12 @@
package org.apache.amoro.utils;
import static org.apache.amoro.utils.MemorySize.MemoryUnit.MEGA_BYTES;
-import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.fail;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.CsvSource;
@@ -68,9 +67,9 @@ public class MemorySizeTest {
assertEquals(expectedTebiBytes, memorySize.getTebiBytes());
}
- @Test(expected = IllegalArgumentException.class)
+ @Test
public void testInvalid() {
- new MemorySize(-1);
+ assertThrows(IllegalArgumentException.class, () -> new MemorySize(-1));
}
@ParameterizedTest
@@ -204,14 +203,16 @@ public class MemorySizeTest {
}
}
- @Test(expected = IllegalArgumentException.class)
+ @Test
public void testParseNumberOverflow() {
- MemorySize.parseBytes("100000000000000000000000000000000 bytes");
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> MemorySize.parseBytes("100000000000000000000000000000000
bytes"));
}
- @Test(expected = IllegalArgumentException.class)
+ @Test
public void testParseNumberTimeUnitOverflow() {
- MemorySize.parseBytes("100000000000000 tb");
+ assertThrows(IllegalArgumentException.class, () ->
MemorySize.parseBytes("100000000000000 tb"));
}
@Test
@@ -231,13 +232,13 @@ public class MemorySizeTest {
@Test
public void testDivideByLong() {
final MemorySize memory = new MemorySize(100L);
- assertThat(memory.divide(23), is(new MemorySize(4L)));
+ assertEquals(new MemorySize(4L), memory.divide(23));
}
- @Test(expected = IllegalArgumentException.class)
+ @Test
public void testDivideByNegativeLong() {
final MemorySize memory = new MemorySize(100L);
- memory.divide(-23L);
+ assertThrows(IllegalArgumentException.class, () -> memory.divide(-23L));
}
static Stream<Arguments> testToHumanReadableStringProvider() {
@@ -256,6 +257,6 @@ public class MemorySizeTest {
@ParameterizedTest
@MethodSource("testToHumanReadableStringProvider")
public void testToHumanReadableString(long bytes, String expected) {
- assertThat(new MemorySize(bytes).toHumanReadableString(), is(expected));
+ assertEquals(expected, new MemorySize(bytes).toHumanReadableString());
}
}
