X-czh commented on code in PR #23200:
URL: https://github.com/apache/flink/pull/23200#discussion_r1299296506
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/buffer/LocalBufferPoolDestroyTest.java:
##########
@@ -20,30 +20,24 @@
import org.apache.flink.runtime.execution.CancelTaskException;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
-import java.io.IOException;
import java.util.concurrent.atomic.AtomicReference;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
/** Tests for the destruction of a {@link LocalBufferPool}. */
public class LocalBufferPoolDestroyTest {
Review Comment:
```suggestion
class LocalBufferPoolDestroyTest {
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/disk/BatchShuffleReadBufferPoolTest.java:
##########
@@ -220,52 +219,52 @@ public void testMultipleThreadRequestAndRecycle() throws
Exception {
requestThread.join();
}
- assertNull(exception.get());
- assertEquals(bufferPool.getNumTotalBuffers(),
bufferPool.getAvailableBuffers());
+ assertThat(exception.get()).isNull();
Review Comment:
```suggestion
assertThat(exception).isNull();
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/buffer/LocalBufferPoolTest.java:
##########
@@ -65,15 +65,15 @@ class LocalBufferPoolTest {
private BufferPool localBufferPool;
@RegisterExtension
- public static final TestExecutorExtension<ExecutorService>
EXECUTOR_RESOURCE =
+ static final TestExecutorExtension<ExecutorService> EXECUTOR_EXTENSION =
Review Comment:
Could you clarify a bit on why we change the variable name here?
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/partition/SortMergeResultPartitionTest.java:
##########
@@ -340,7 +340,7 @@ void testReleaseWhileWriting() throws Exception {
assertThatThrownBy(
() ->
partition.emitRecord(ByteBuffer.allocate(bufferSize * numBuffers), 2))
.isInstanceOf(IllegalStateException.class);
-
assertThat(fileChannelManager.getPaths()[0].list().length).isEqualTo(0);
+ assertThat(fileChannelManager.getPaths()[0].list().length).isZero();
Review Comment:
```suggestion
assertThat(fileChannelManager.getPaths()[0].list()).isEmpty();
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/partition/SortMergeResultPartitionTest.java:
##########
@@ -373,7 +373,7 @@ void testRelease() throws Exception {
while (partition.getResultFile() != null) {
Thread.sleep(100);
}
-
assertThat(checkNotNull(fileChannelManager.getPaths()[0].list()).length).isEqualTo(0);
+
assertThat(checkNotNull(fileChannelManager.getPaths()[0].list()).length).isZero();
Review Comment:
same here
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/partition/ProducerFailedExceptionTest.java:
##########
@@ -21,26 +21,26 @@
import org.apache.flink.runtime.execution.CancelTaskException;
import org.apache.flink.util.SerializedThrowable;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
/** Tests for {@link ProducerFailedException}. */
-public class ProducerFailedExceptionTest {
+class ProducerFailedExceptionTest {
@Test
- public void testInstanceOfCancelTaskException() throws Exception {
-
assertTrue(CancelTaskException.class.isAssignableFrom(ProducerFailedException.class));
+ void testInstanceOfCancelTaskException() throws Exception {
+
assertThat(CancelTaskException.class.isAssignableFrom(ProducerFailedException.class))
+ .isTrue();
}
@Test
- public void testCauseIsSerialized() throws Exception {
+ void testCauseIsSerialized() throws Exception {
// Tests that the cause is stringified, because it might be an instance
// of a user level Exception, which can not be deserialized by the
// remote receiver's system class loader.
ProducerFailedException e = new ProducerFailedException(new
Exception());
- assertNotNull(e.getCause());
- assertTrue(e.getCause() instanceof SerializedThrowable);
+ assertThat(e.getCause()).isNotNull();
+ assertThat(e.getCause()).isInstanceOf(SerializedThrowable.class);
Review Comment:
```suggestion
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/partition/ProducerFailedExceptionTest.java:
##########
@@ -21,26 +21,26 @@
import org.apache.flink.runtime.execution.CancelTaskException;
import org.apache.flink.util.SerializedThrowable;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
/** Tests for {@link ProducerFailedException}. */
-public class ProducerFailedExceptionTest {
+class ProducerFailedExceptionTest {
@Test
- public void testInstanceOfCancelTaskException() throws Exception {
-
assertTrue(CancelTaskException.class.isAssignableFrom(ProducerFailedException.class));
+ void testInstanceOfCancelTaskException() throws Exception {
+
assertThat(CancelTaskException.class.isAssignableFrom(ProducerFailedException.class))
+ .isTrue();
}
@Test
- public void testCauseIsSerialized() throws Exception {
+ void testCauseIsSerialized() throws Exception {
// Tests that the cause is stringified, because it might be an instance
// of a user level Exception, which can not be deserialized by the
// remote receiver's system class loader.
ProducerFailedException e = new ProducerFailedException(new
Exception());
- assertNotNull(e.getCause());
- assertTrue(e.getCause() instanceof SerializedThrowable);
+ assertThat(e.getCause()).isNotNull();
Review Comment:
```suggestion
assertThat(e.getCause()).isNotNull().isInstanceOf(SerializedThrowable.class);
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]