1996fanrui commented on code in PR #23200:
URL: https://github.com/apache/flink/pull/23200#discussion_r1295793233
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/disk/FileChannelManagerImplTest.java:
##########
@@ -89,28 +90,29 @@ public void testFairness() throws Exception {
threads[i].join();
}
- assertEquals(counter1.get(), counter2.get());
+ assertThat(counter2.get()).isEqualTo(counter1.get());
Review Comment:
```suggestion
assertThat(counter2).isEqualTo(counter1);
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/disk/iomanager/AsynchronousFileIOChannelTest.java:
##########
@@ -308,8 +308,8 @@ public void requestFailed(MemorySegment buffer, IOException
e) {
writer.close();
- assertEquals(NUM_BLOCKS, callbackCounter.get());
- assertFalse(exceptionOccurred.get());
+ assertThat(callbackCounter.get()).isEqualTo(NUM_BLOCKS);
+ assertThat(exceptionOccurred.get()).isFalse();
Review Comment:
```suggestion
assertThat(callbackCounter).hasValue(NUM_BLOCKS);
assertThat(exceptionOccurred).isFalse();
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/disk/iomanager/IOManagerITCase.java:
##########
@@ -189,15 +189,13 @@ public void parallelChannelsTest() throws Exception {
try {
intValue = Integer.parseInt(val.value);
} catch (NumberFormatException nfex) {
- Assert.fail(
- "Invalid value read from reader. Valid
decimal number expected.");
+ fail("Invalid value read from reader. Valid
decimal number expected.");
Review Comment:
fail with catch can be updated to assertThatThrownBy
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/disk/iomanager/IOManagerITCase.java:
##########
@@ -222,7 +220,7 @@ public void parallelChannelsTest() throws Exception {
// check that files are deleted
for (int i = 0; i < NUM_CHANNELS; i++) {
File f = new File(ids[i].getPath());
- Assert.assertFalse("Channel file has not been deleted.",
f.exists());
+ assertThat(f.exists()).withFailMessage("Channel file has not been
deleted.").isFalse();
Review Comment:
```suggestion
assertThat(f)).withFailMessage("Channel file has not been
deleted.").doesNotExists(();
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/disk/iomanager/IOManagerTest.java:
##########
@@ -61,12 +54,16 @@ public void channelEnumerator() throws Exception {
File path = id.getPathFile();
- assertTrue("Channel IDs must name an absolute path.",
path.isAbsolute());
- assertFalse("Channel IDs must name a file, not a directory.",
path.isDirectory());
+ assertThat(path.isAbsolute())
+ .withFailMessage("Channel IDs must name an absolute
path.")
+ .isTrue();
Review Comment:
```suggestion
assertThat(path)
.withFailMessage("Channel IDs must name an absolute
path.")
.isAbsolute();
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/api/writer/RecordWriterTest.java:
##########
@@ -309,19 +297,19 @@ public void testIsAvailableOrNot() throws Exception {
try {
// record writer is available because of initial available global
pool
- assertTrue(recordWriter.getAvailableFuture().isDone());
+ assertThat(recordWriter.getAvailableFuture().isDone()).isTrue();
// request one buffer from the local pool to make it unavailable
afterwards
try (BufferBuilder bufferBuilder =
localPool.requestBufferBuilder(0)) {
- assertNotNull(bufferBuilder);
- assertFalse(recordWriter.getAvailableFuture().isDone());
+ assertThat(bufferBuilder).isNotNull();
+
assertThat(recordWriter.getAvailableFuture().isDone()).isFalse();
// recycle the buffer to make the local pool available again
final Buffer buffer =
BufferBuilderTestUtils.buildSingleBuffer(bufferBuilder);
buffer.recycleBuffer();
}
- assertTrue(recordWriter.getAvailableFuture().isDone());
- assertEquals(recordWriter.AVAILABLE,
recordWriter.getAvailableFuture());
+ assertThat(recordWriter.getAvailableFuture().isDone()).isTrue();
Review Comment:
```suggestion
assertThat(recordWriter.getAvailableFuture()).isDone();
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/api/writer/RecordWriterTest.java:
##########
@@ -309,19 +297,19 @@ public void testIsAvailableOrNot() throws Exception {
try {
// record writer is available because of initial available global
pool
- assertTrue(recordWriter.getAvailableFuture().isDone());
+ assertThat(recordWriter.getAvailableFuture().isDone()).isTrue();
// request one buffer from the local pool to make it unavailable
afterwards
try (BufferBuilder bufferBuilder =
localPool.requestBufferBuilder(0)) {
- assertNotNull(bufferBuilder);
- assertFalse(recordWriter.getAvailableFuture().isDone());
+ assertThat(bufferBuilder).isNotNull();
+
assertThat(recordWriter.getAvailableFuture().isDone()).isFalse();
Review Comment:
```suggestion
assertThat(recordWriter.getAvailableFuture()).isNotDone();
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/api/writer/RecordWriterTest.java:
##########
@@ -309,19 +297,19 @@ public void testIsAvailableOrNot() throws Exception {
try {
// record writer is available because of initial available global
pool
- assertTrue(recordWriter.getAvailableFuture().isDone());
+ assertThat(recordWriter.getAvailableFuture().isDone()).isTrue();
Review Comment:
```suggestion
assertThat(recordWriter.getAvailableFuture()).isDone();
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/disk/iomanager/IOManagerAsyncTest.java:
##########
@@ -199,15 +199,15 @@ public void read() throws IOException {
}
}
- assertTrue(exceptionForwarded.get());
+ assertThat(exceptionForwarded.get()).isTrue();
Review Comment:
```suggestion
assertThat(exceptionForwarded).isTrue();
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/api/writer/RecordWriterDelegateTest.java:
##########
@@ -157,9 +154,9 @@ private void verifyAvailability(RecordWriterDelegate
writerDelegate) throws Exce
Buffer buffer = readView.getNextBuffer().buffer();
buffer.recycleBuffer();
- assertTrue(future.isDone());
- assertTrue(writerDelegate.isAvailable());
- assertTrue(writerDelegate.getAvailableFuture().isDone());
+ assertThat(future.isDone()).isTrue();
+ assertThat(writerDelegate.isAvailable()).isTrue();
+ assertThat(writerDelegate.getAvailableFuture().isDone()).isTrue();
Review Comment:
```suggestion
assertThat(future).isDone();
assertThat(writerDelegate.isAvailable()).isTrue();
assertThat(writerDelegate.getAvailableFuture()).isDone();
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/disk/iomanager/IOManagerAsyncTest.java:
##########
@@ -246,15 +246,15 @@ public void write() throws IOException {
}
}
- assertTrue(exceptionForwarded.get());
+ assertThat(exceptionForwarded.get()).isTrue();
Review Comment:
```suggestion
assertThat(exceptionForwarded).isTrue();
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/api/writer/RecordWriterDelegateTest.java:
##########
@@ -137,17 +134,17 @@ private RecordWriter createRecordWriter(NetworkBufferPool
globalPool) throws Exc
private void verifyAvailability(RecordWriterDelegate writerDelegate)
throws Exception {
// writer is available at the beginning
- assertTrue(writerDelegate.isAvailable());
- assertTrue(writerDelegate.getAvailableFuture().isDone());
+ assertThat(writerDelegate.isAvailable()).isTrue();
+ assertThat(writerDelegate.getAvailableFuture().isDone()).isTrue();
Review Comment:
```suggestion
assertThat(writerDelegate.getAvailableFuture()).isDone();
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/disk/iomanager/IOManagerTest.java:
##########
@@ -61,12 +54,16 @@ public void channelEnumerator() throws Exception {
File path = id.getPathFile();
- assertTrue("Channel IDs must name an absolute path.",
path.isAbsolute());
- assertFalse("Channel IDs must name a file, not a directory.",
path.isDirectory());
+ assertThat(path.isAbsolute())
+ .withFailMessage("Channel IDs must name an absolute
path.")
+ .isTrue();
+ assertThat(path.isDirectory())
+ .withFailMessage("Channel IDs must name a file, not a
directory.")
+ .isFalse();
Review Comment:
```suggestion
assertThat(path)
.withFailMessage("Channel IDs must name a file, not
a directory.")
.isFile(();
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/api/serialization/EventSerializerTest.java:
##########
@@ -119,50 +116,51 @@ public class EventSerializerTest {
};
@Test
- public void testSerializeDeserializeEvent() throws Exception {
+ void testSerializeDeserializeEvent() throws Exception {
for (AbstractEvent evt : events) {
ByteBuffer serializedEvent =
EventSerializer.toSerializedEvent(evt);
- assertTrue(serializedEvent.hasRemaining());
+ assertThat(serializedEvent.hasRemaining()).isTrue();
AbstractEvent deserialized =
EventSerializer.fromSerializedEvent(
serializedEvent, getClass().getClassLoader());
- assertNotNull(deserialized);
- assertEquals(evt, deserialized);
+ assertThat(deserialized).isNotNull();
+ assertThat(deserialized).isEqualTo(evt);
Review Comment:
```suggestion
assertThat(deserialized).isNotNull().isEqualTo(evt);
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/api/writer/RecordWriterDelegateTest.java:
##########
@@ -137,17 +134,17 @@ private RecordWriter createRecordWriter(NetworkBufferPool
globalPool) throws Exc
private void verifyAvailability(RecordWriterDelegate writerDelegate)
throws Exception {
// writer is available at the beginning
- assertTrue(writerDelegate.isAvailable());
- assertTrue(writerDelegate.getAvailableFuture().isDone());
+ assertThat(writerDelegate.isAvailable()).isTrue();
+ assertThat(writerDelegate.getAvailableFuture().isDone()).isTrue();
// request one buffer from the local pool to make it unavailable
RecordWriter recordWriter = writerDelegate.getRecordWriter(0);
for (int i = 0; i < memorySegmentSize / recordSize; ++i) {
recordWriter.emit(new IntValue(i));
}
- assertFalse(writerDelegate.isAvailable());
+ assertThat(writerDelegate.isAvailable()).isFalse();
CompletableFuture future = writerDelegate.getAvailableFuture();
- assertFalse(future.isDone());
+ assertThat(future.isDone()).isFalse();
Review Comment:
```suggestion
assertThat(future).isNotDone();
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/api/writer/RecordWriterTest.java:
##########
@@ -51,43 +51,31 @@
import org.apache.flink.types.IntValue;
import org.apache.flink.util.XORShiftRandom;
-import org.junit.Assert;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import java.io.File;
import java.io.IOException;
import java.util.ArrayDeque;
import java.util.Random;
import static
org.apache.flink.runtime.io.network.partition.PartitionTestUtils.createPartition;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
/** Tests for the {@link RecordWriter}. */
-public class RecordWriterTest {
+class RecordWriterTest {
- private final boolean isBroadcastWriter;
+ boolean isBroadcastWriter = false;
Review Comment:
It's always false, right?
--
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
