Remove JUnit4 dependency from Flink

[nastra]

Feature Request / Improvement

I started removing JUnit4 from Flink via the below diff

diff --git a/flink/v2.0/build.gradle b/flink/v2.0/build.gradle
index c0de408e98..ef4635e133 100644
--- a/flink/v2.0/build.gradle
+++ b/flink/v2.0/build.gradle
@@ -68,7 +68,9 @@ project(":iceberg-flink:iceberg-flink-${flinkMajorVersion}") {

     implementation libs.datasketches

-    testImplementation libs.flink20.connector.test.utils
+    testImplementation(libs.flink20.connector.test.utils) {
+      exclude group: 'junit'
+    }
     testImplementation libs.flink20.core
     testImplementation libs.flink20.runtime
     testImplementation(libs.flink20.test.utilsjunit) {
diff --git a/flink/v2.0/flink/src/test/java/org/apache/iceberg/flink/sink/TestIcebergSink.java b/flink/v2.0/flink/src/test/java/org/apache/iceberg/flink/sink/TestIcebergSink.java
index b02b3337dc..362d004cfe 100644
--- a/flink/v2.0/flink/src/test/java/org/apache/iceberg/flink/sink/TestIcebergSink.java
+++ b/flink/v2.0/flink/src/test/java/org/apache/iceberg/flink/sink/TestIcebergSink.java
@@ -21,6 +21,7 @@ package org.apache.iceberg.flink.sink;
 import static org.apache.iceberg.flink.TestFixtures.DATABASE;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.Assumptions.assumeThat;

 import java.io.IOException;
 import java.util.Collections;
@@ -55,7 +56,6 @@ import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
 import org.apache.iceberg.relocated.com.google.common.collect.Lists;
 import org.apache.iceberg.relocated.com.google.common.collect.Maps;
 import org.apache.iceberg.types.Types;
-import org.junit.Assume;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.TestTemplate;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -421,8 +421,9 @@ public class TestIcebergSink extends TestFlinkIcebergSinkBase {

   @TestTemplate
   void testErrorOnNullForRequiredField() throws Exception {
-    Assume.assumeFalse(
-        "ORC file format supports null values even for required fields.", format == FileFormat.ORC);
+    assumeThat(format)
+        .as("ORC file format supports null values even for required fields.")
+        .isEqualTo(FileFormat.ORC);

However, there is still TestIcebergSourceFailover that uses MiniClusterWithClientResource which requires JUnit4. This would be good to convert so that we can eventually remove all JUnit4 dependencies across the codebase.
We would also need to backport the fix to Flink 1.20 & 1.19.

Query engine

None

Willingness to contribute

• I can contribute this improvement/feature independently
• I would be willing to contribute this improvement/feature with guidance from the Iceberg community
• I cannot contribute this improvement/feature at this time

[tomtongue]

@nastra I can work for this to remove JUnit 4 along with the Spark migration. But if anyone is already working on this or will work, I could collaborate the migration for this.

[JeonDaehong]

@nastra I can work for this to remove JUnit 4 along with the Spark migration. But if anyone is already working on this or will work, I could collaborate the migration for this.

Hello,
I'm a developer who is eager to properly learn Apache Iceberg and contribute to the project.
I would love to work on this issue and submit a PR—would that be okay?

[nastra]

@JeonDaehong welcome to the community. We always welcome new contributions, so feel free to work on this. You might also want to sync up with @tomtongue, since he expressed interest as well.

[JeonDaehong]

@JeonDaehong welcome to the community. We always welcome new contributions, so feel free to work on this. You might also want to sync up with @tomtongue, since he expressed interest as well.

Thank you!
It would be great to work together on this.
I'll start by forking the project and begin making and testing changes from today.

[tomtongue]

@JeonDaehong thanks for helping this, as @nastra is mentioning, I will be able to work for Flink migration, but maybe I'll take a little bit time for starting the Flink migration, so it would be great if you start working on this first.

[JeonDaehong]

@JeonDaehong thanks for helping this, as @nastra is mentioning, I will be able to work for Flink migration, but maybe I'll take a little bit time for starting the Flink migration, so it would be greate if you start working on this first.

Alright !

Once I complete the migration work and confirm the tests pass successfully, I’ll open a PR and reference this issue link.
It might take a bit of time, but I’ll do my best to achieve a good result.

Thank you!

[JeonDaehong]

@nastra @tomtongue

Would it be okay to migrate only the TestIcebergSourceFailover class, which uses MiniClusterWithClientResource, to JUnit 5 and run the build/test to verify everything works?
(Planning to apply the same change to both the 1.20 and 1.19 packages as well.)

Since this is my first PR contribution, I wanted to ask and make sure! :D

[nastra]

@JeonDaehong ideally what we want is to be able to remove the JUnit4 dependency from Flink via the diff outlined in the issue description. So I would use the diff + the changes required for TestIcebergSourceFailover.
I would suggest that you first do the changes for Flink 2.0. Once everything looks good you can backport those changes to earlier Flink versions.

[JeonDaehong]

@JeonDaehong ideally what we want is to be able to remove the JUnit4 dependency from Flink via the diff outlined in the issue description. So I would use the diff + the changes required for TestIcebergSourceFailover.
I would suggest that you first do the changes for Flink 2.0. Once everything looks good you can backport those changes to earlier Flink versions.

Thank you.
I'll modify the three classes and backport the changes from 2.0 to 1.9 in that order

[JeonDaehong]

#13016

@nastra Hello,

I have removed the MiniClusterWithClientResource class, which was still dependent on JUnit4, and have also incorporated changes to the gradle and TestIcebergSink classes.

Since this is my first time doing this, could you please confirm if the approach is correct? If so, I kindly request you to approve the merge.

If this direction is correct, I will proceed with backporting to versions 1.20 and 1.19.

I will continue working hard. Thank you.

[JeonDaehong]

#13016

@nastra Hello,

I have removed the MiniClusterWithClientResource class, which was still dependent on JUnit4, and have also incorporated changes to the gradle and TestIcebergSink classes.

Since this is my first time doing this, could you please confirm if the approach is correct? If so, I kindly request you to approve the merge.

If this direction is correct, I will proceed with backporting to versions 1.20 and 1.19.

I will continue working hard. Thank you.

If this is not the correct approach, I will close this PR and make the necessary changes based on your feedback