1996fanrui commented on code in PR #23199:
URL: https://github.com/apache/flink/pull/23199#discussion_r1292784420
##########
flink-runtime/src/test/java/org/apache/flink/runtime/util/BlockingShutdownTest.java:
##########
@@ -96,7 +95,7 @@ public void testProcessExitsDespiteBlockingShutdownHook()
throws Exception {
try {
blockingProcess.startProcess();
long pid = blockingProcess.getProcessId();
- assertTrue("Cannot determine process ID", pid != -1);
+ assertThat(pid != -1).withFailMessage("Cannot determine process
ID").isTrue();
Review Comment:
```suggestion
assertThat(pid).withFailMessage("Cannot determine process
ID").isNotEqualTo(-1);
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/util/BoundedFIFOQueueTest.java:
##########
@@ -18,90 +18,87 @@
package org.apache.flink.runtime.util;
-import org.apache.flink.util.TestLogger;
+import org.apache.flink.util.TestLoggerExtension;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
-import static org.hamcrest.collection.IsIterableContainingInOrder.contains;
-import static org.hamcrest.collection.IsIterableWithSize.iterableWithSize;
-import static org.hamcrest.core.Is.is;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
/** {@code BoundedFIFOQueueTest} tests {@link BoundedFIFOQueue}. */
-public class BoundedFIFOQueueTest extends TestLogger {
+@ExtendWith(TestLoggerExtension.class)
+class BoundedFIFOQueueTest {
- @Test(expected = IllegalArgumentException.class)
- public void testConstructorFailing() {
- new BoundedFIFOQueue<>(-1);
+ @Test
+ void testConstructorFailing() {
+ assertThatThrownBy(() -> new BoundedFIFOQueue<>(-1))
+ .isInstanceOf(IllegalArgumentException.class);
}
@Test
- public void testQueueWithMaxSize0() {
+ void testQueueWithMaxSize0() {
final BoundedFIFOQueue<Integer> testInstance = new
BoundedFIFOQueue<>(0);
- assertThat(testInstance, iterableWithSize(0));
+ assertThat(testInstance.size()).isEqualTo(0);
testInstance.add(1);
- assertThat(testInstance, iterableWithSize(0));
+ assertThat(testInstance.size()).isEqualTo(0);
}
@Test
- public void testQueueWithMaxSize2() {
+ void testQueueWithMaxSize2() {
final BoundedFIFOQueue<Integer> testInstance = new
BoundedFIFOQueue<>(2);
- assertThat(testInstance, iterableWithSize(0));
+ assertThat(testInstance.size()).isEqualTo(0);
testInstance.add(1);
- assertThat(testInstance, contains(1));
+ assertThat(testInstance).contains(1);
testInstance.add(2);
- assertThat(testInstance, contains(1, 2));
+ assertThat(testInstance).contains(1, 2);
testInstance.add(3);
- assertThat(testInstance, contains(2, 3));
+ assertThat(testInstance).contains(2, 3);
}
@Test
- public void testAddNullHandling() {
+ void testAddNullHandling() {
final BoundedFIFOQueue<Integer> testInstance = new
BoundedFIFOQueue<>(1);
- try {
- testInstance.add(null);
- fail("A NullPointerException is expected to be thrown.");
- } catch (NullPointerException e) {
- // NullPointerException is expected
- }
-
- assertThat(testInstance, iterableWithSize(0));
+ assertThatThrownBy(() -> testInstance.add(null))
+ .withFailMessage("A NullPointerException is expected to be
thrown.")
+ .isInstanceOf(NullPointerException.class);
+
+ assertThat(testInstance.size()).isEqualTo(0);
}
/**
* Tests that {@link BoundedFIFOQueue#size()} returns the number of
elements currently stored in
* the queue with a {@code maxSize} of 0.
*/
@Test
- public void testSizeWithMaxSize0() {
+ void testSizeWithMaxSize0() {
final BoundedFIFOQueue<Integer> testInstance = new
BoundedFIFOQueue<>(0);
- assertThat(testInstance.size(), is(0));
+ assertThat(testInstance.size()).isEqualTo(0);
testInstance.add(1);
- assertThat(testInstance.size(), is(0));
+ assertThat(testInstance.size()).isEqualTo(0);
}
/**
* Tests that {@link BoundedFIFOQueue#size()} returns the number of
elements currently stored in
* the queue with a {@code maxSize} of 2.
*/
@Test
- public void testSizeWithMaxSize2() {
+ void testSizeWithMaxSize2() {
final BoundedFIFOQueue<Integer> testInstance = new
BoundedFIFOQueue<>(2);
- assertThat(testInstance.size(), is(0));
+ assertThat(testInstance.size()).isEqualTo(0);
testInstance.add(5);
- assertThat(testInstance.size(), is(1));
+ assertThat(testInstance.size()).isEqualTo(1);
Review Comment:
```suggestion
assertThat(testInstance).hasSize(1);
```
##########
flink-test-utils-parent/flink-test-utils-junit/src/main/java/org/apache/flink/util/TestLogger.java:
##########
@@ -31,7 +31,10 @@
/**
* Adds automatic test name logging. Every test which wants to log which test
is currently executed
* and why it failed, simply has to extend this class.
+ *
+ * @deprecated Continue using {@link TestLoggerExtension} with Junit5 tests.
*/
+@Deprecated
Review Comment:
Hi @JingGe , I'm not sure shoule these classes releted to junit4 be marked
to `Deprecated`. Would you mind helping check it? thanks a lot~
##########
flink-runtime/src/test/java/org/apache/flink/runtime/util/BlockingShutdownTest.java:
##########
@@ -54,7 +53,7 @@ public void testProcessShutdownBlocking() throws Exception {
try {
blockingProcess.startProcess();
long pid = blockingProcess.getProcessId();
- assertTrue("Cannot determine process ID", pid != -1);
+ assertThat(pid != -1).withFailMessage("Cannot determine process
ID").isTrue();
Review Comment:
```suggestion
assertThat(pid).withFailMessage("Cannot determine process
ID").isNotEqualTo(-1);
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/util/BoundedFIFOQueueTest.java:
##########
@@ -18,90 +18,87 @@
package org.apache.flink.runtime.util;
-import org.apache.flink.util.TestLogger;
+import org.apache.flink.util.TestLoggerExtension;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
-import static org.hamcrest.collection.IsIterableContainingInOrder.contains;
-import static org.hamcrest.collection.IsIterableWithSize.iterableWithSize;
-import static org.hamcrest.core.Is.is;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
/** {@code BoundedFIFOQueueTest} tests {@link BoundedFIFOQueue}. */
-public class BoundedFIFOQueueTest extends TestLogger {
+@ExtendWith(TestLoggerExtension.class)
+class BoundedFIFOQueueTest {
- @Test(expected = IllegalArgumentException.class)
- public void testConstructorFailing() {
- new BoundedFIFOQueue<>(-1);
+ @Test
+ void testConstructorFailing() {
+ assertThatThrownBy(() -> new BoundedFIFOQueue<>(-1))
+ .isInstanceOf(IllegalArgumentException.class);
}
@Test
- public void testQueueWithMaxSize0() {
+ void testQueueWithMaxSize0() {
final BoundedFIFOQueue<Integer> testInstance = new
BoundedFIFOQueue<>(0);
- assertThat(testInstance, iterableWithSize(0));
+ assertThat(testInstance.size()).isEqualTo(0);
Review Comment:
```suggestion
assertThat(testInstance).isEmpty();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
