Re: [PR] [FLINK-32850][flink-runtime][JUnit5 Migration] The io.network.api package of flink-runtime module [flink]

[via GitHub]

dawidwys merged PR #23590:
URL: https://github.com/apache/flink/pull/23590


-- 
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

Re: [PR] [FLINK-32850][flink-runtime][JUnit5 Migration] The io.network.api package of flink-runtime module [flink]

[via GitHub]

dawidwys commented on code in PR #23590:
URL: https://github.com/apache/flink/pull/23590#discussion_r1430305765


##
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/api/CheckpointBarrierTest.java:
##
@@ -22,37 +22,29 @@
 import org.apache.flink.core.memory.DataOutputSerializer;
 import org.apache.flink.runtime.checkpoint.CheckpointOptions;
 
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
-import static org.junit.Assert.fail;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 /** Tests for the {@link CheckpointBarrier} type. */
-public class CheckpointBarrierTest {
+class CheckpointBarrierTest {
 
 /**
  * Test serialization of the checkpoint barrier. The checkpoint barrier 
does not support its own
  * serialization, in order to be immutable.
  */
 @Test
-public void testSerialization() throws Exception {
+void testSerialization() {
 long id = Integer.MAX_VALUE + 123123L;
 long timestamp = Integer.MAX_VALUE + 1228L;
 
 CheckpointOptions options = 
CheckpointOptions.forCheckpointWithDefaultLocation();
 CheckpointBarrier barrier = new CheckpointBarrier(id, timestamp, 
options);
 
-try {
-barrier.write(new DataOutputSerializer(1024));
-fail("should throw an exception");

Review Comment:
   I don't think it adds any value. The `fail("should throw an exception")` is 
there simply to make sure, the exception is thrown



-- 
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

Re: [PR] [FLINK-32850][flink-runtime][JUnit5 Migration] The io.network.api package of flink-runtime module [flink]

[via GitHub]

RocMarshal commented on code in PR #23590:
URL: https://github.com/apache/flink/pull/23590#discussion_r1430131390


##
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/api/CheckpointBarrierTest.java:
##
@@ -22,37 +22,29 @@
 import org.apache.flink.core.memory.DataOutputSerializer;
 import org.apache.flink.runtime.checkpoint.CheckpointOptions;
 
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
-import static org.junit.Assert.fail;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 /** Tests for the {@link CheckpointBarrier} type. */
-public class CheckpointBarrierTest {
+class CheckpointBarrierTest {
 
 /**
  * Test serialization of the checkpoint barrier. The checkpoint barrier 
does not support its own
  * serialization, in order to be immutable.
  */
 @Test
-public void testSerialization() throws Exception {
+void testSerialization() {
 long id = Integer.MAX_VALUE + 123123L;
 long timestamp = Integer.MAX_VALUE + 1228L;
 
 CheckpointOptions options = 
CheckpointOptions.forCheckpointWithDefaultLocation();
 CheckpointBarrier barrier = new CheckpointBarrier(id, timestamp, 
options);
 
-try {
-barrier.write(new DataOutputSerializer(1024));
-fail("should throw an exception");

Review Comment:
   Should we keep the hint message  by 
   ```
assertThatThrownBy(() -> barrier.write(new 
DataOutputSerializer(1024))).as("should throw an exception")
   .isInstanceOf(UnsupportedOperationException.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: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-32850][flink-runtime][JUnit5 Migration] The io.network.api package of flink-runtime module [flink]

[via GitHub]

leonardBang commented on PR #23590:
URL: https://github.com/apache/flink/pull/23590#issuecomment-1857368418

   @dawidwys Would you like to help review this PR?


-- 
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

Re: [PR] [FLINK-32850][flink-runtime][JUnit5 Migration] The io.network.api package of flink-runtime module [flink]

[via GitHub]

flinkbot commented on PR #23590:
URL: https://github.com/apache/flink/pull/23590#issuecomment-1778793748

   
   ## CI report:
   
   * 227d9ad14fae571cc8dc5167ca8c9b91bc2853fb UNKNOWN
   
   
   Bot commands
 The @flinkbot bot supports the following commands:
   
- `@flinkbot run azure` re-run the last Azure build
   


-- 
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

Re: [PR] [FLINK-32850][flink-runtime][JUnit5 Migration] The io.network.api package of flink-runtime module [flink]

[via GitHub]

Jiabao-Sun commented on PR #23590:
URL: https://github.com/apache/flink/pull/23590#issuecomment-1778784476

   Hi @1996fanrui, @RocMarshal, could you help review it when you have time?


-- 
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

[PR] [FLINK-32850][flink-runtime][JUnit5 Migration] The io.network.api package of flink-runtime module [flink]

[via GitHub]

Jiabao-Sun opened a new pull request, #23590:
URL: https://github.com/apache/flink/pull/23590

   
   
   ## What is the purpose of the change
   [FLINK-32850][flink-runtime][JUnit5 Migration] The io.network.api package of 
flink-runtime module
   
   ## Brief change log
   [FLINK-32850][flink-runtime][JUnit5 Migration] The io.network.api package of 
flink-runtime module
   
   ## Verifying this change
   This change is already covered by existing tests
   
   ## Does this pull request potentially affect one of the following parts:
   
 - Dependencies (does it add or upgrade a dependency): (no)
 - The public API, i.e., is any changed class annotated with 
`@Public(Evolving)`: (no)
 - The serializers: (no)
 - The runtime per-record code paths (performance sensitive): (no)
 - Anything that affects deployment or recovery: JobManager (and its 
components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
 - The S3 file system connector: (no)
   
   ## Documentation
   
 - Does this pull request introduce a new feature? (no)
 - If yes, how is the feature documented? (not documented)
   


-- 
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