[FLINK-27939][hive] Migrate unit tests to JUnit 5 by jlalwani-amazon · Pull Request #39 · apache/flink-connector-hive

jlalwani-amazon · 2026-04-27T21:20:51Z

What is the purpose of the change

Complete JUnit 5 migration for all 32 test files in flink-connector-hive (FLINK-27939).

Brief change log

Batch 1 (7 files): Simple import swaps for HiveModuleFactoryTest, HiveTableUtilTest, HiveGenericUDAFTest, HiveGenericUDTFTest, HiveSimpleUDFTest, HiveGenericUDFTest, HiveASTParserTest

Batch 2 (7 files): Simple import swaps + parameterized test migration for PartitionMonitorTest, HiveConfUtilsTest, HiveOutputFormatFactoryTest, HiveDeserializeExceptionTest (@RunWith(Parameterized.class) → @ParameterizedTest @MethodSource), ContinuousHiveSplitEnumeratorTest, HiveInputFormatPartitionReaderITCase, HivePartitionFetcherTest

Batch 3 (6 files): Lifecycle annotations for HiveDynamicTableFactoryTest, HiveLookupJoinITCase, HiveSourceITCase, HiveDialectQueryPlanTest, HiveCatalogTest, HiveTableSourceITCase. @BeforeClass → @BeforeAll, @AfterClass → @AfterAll, @Before → @BeforeEach, @After → @AfterEach, @Test(timeout=N) → @Test @Timeout.

Batch 4 (7 files): TemporaryFolder migration for HiveModuleTest, HiveSourceFileEnumeratorTest, HivePartitionUtilsTest, HiveCatalogUdfITCase, HiveCatalogITCase, HiveDialectITCase, TableEnvHiveConnectorITCase. @Rule TemporaryFolder → @TempDir Path, @ClassRule static TemporaryFolder → @TempDir static Path.

Batch 5 (4 files): Complex migrations for HiveCatalogDataTypeTest (ExpectedException → assertThatThrownBy), HiveCatalogFactoryTest (TestExecutorResource → TestExecutorExtension, ExpectedException → assertThatThrownBy), HiveDialectQueryITCase (ComparisonFailure → AssertionFailedError), HiveDialectAggITCase (removed unused TemporaryFolder).

Batch 6: Migrate FlinkEmbeddedHiveRunner (JUnit 4 BlockJUnit4ClassRunner) to FlinkEmbeddedHiveRunnerExtension (JUnit 5 BeforeAllCallback/AfterAllCallback). Replicates annotation processing for @HiveSQL, @HiveResource, @HiveProperties, @HiveSetupScript, @HiveRunnerSetup. Delete old FlinkEmbeddedHiveRunner.java and FlinkStandaloneHiveRunner.java (dead code). Add @Isolated to HiveRunnerITCase to avoid Derby metastore contention with other IT tests.

Review fixes: Address reviewer feedback on assertion style and test organization.

Verifying this change

CI passed on fork: https://github.com/jlalwani-amazon/flink-connector-hive/actions

Does this pull request potentially affect one of the following parts?

Dependencies: no (junit-jupiter already a transitive dependency)
The public API: no
The serializers: no
The runtime per-record code paths: no
Anything that affects deployment or recovery: no

snuyanzin · 2026-05-09T20:16:41Z

+            <dependency>
+                <groupId>xml-apis</groupId>
+                <artifactId>xml-apis</artifactId>
+                <version>1.4.01</version>
+            </dependency>
+            <dependency>
+                <groupId>com.google.code.gson</groupId>
+                <artifactId>gson</artifactId>
+                <version>2.2.4</version>
+            </dependency>
+            <dependency>
+                <groupId>io.netty</groupId>
+                <artifactId>netty</artifactId>
+                <version>3.10.6.Final</version>
+            </dependency>
+            <dependency>
+                <groupId>com.lmax</groupId>
+                <artifactId>disruptor</artifactId>
+                <version>3.3.7</version>
+            </dependency>
+            <dependency>
+                <groupId>org.apache.zookeeper</groupId>
+                <artifactId>zookeeper</artifactId>
+                <version>3.4.6</version>
+            </dependency>
+            <dependency>
+                <groupId>org.codehaus.jettison</groupId>
+                <artifactId>jettison</artifactId>
+                <version>1.1</version>
+            </dependency>
+            <dependency>
+                <groupId>junit</groupId>
+                <artifactId>junit</artifactId>
+                <version>4.13.2</version>


why do we need such old dependencies?

These are dependency convergence pins that were based on dependencies pulled from 1.20.3. SInce, main is on 2.0-preview now, we will need to update these dependencies. We will push tha change shortly

snuyanzin · 2026-05-11T16:47:13Z

in Apache Flink we should follow linear history
any merge commit is a blocker

jlalwani-amazon · 2026-05-11T17:10:04Z

Thanks for flagging — I'll rebase to get a linear history. Will push the fix shortly.

Migrate 7 test files with no Rules/ClassRule/lifecycle methods: - HiveModuleFactoryTest - HiveTableUtilTest - HiveGenericUDAFTest - HiveGenericUDTFTest - HiveSimpleUDFTest - HiveGenericUDFTest - HiveASTParserTest Changes: org.junit.Test -> org.junit.jupiter.api.Test, org.junit.Assume -> org.junit.jupiter.api.Assumptions

Migrate 7 more test files with no Rules/ClassRule/lifecycle methods: - PartitionMonitorTest - HiveConfUtilsTest - HiveOutputFormatFactoryTest - HiveDeserializeExceptionTest (includes Parameterized -> ParameterizedTest) - ContinuousHiveSplitEnumeratorTest - HiveInputFormatPartitionReaderITCase - HivePartitionFetcherTest

snuyanzin · 2026-05-11T19:23:29Z


-    private void loadAnnotatedSetupScripts(Class testClass, HiveShellBuilder hiveShellBuilder) {
+    private void loadAnnotatedSetupScripts(
+            final Class<?> testClass, HiveShellBuilder hiveShellBuilder) {


can we align on style: either all args are final or none of them, however not mixed
here and in other places

Removed final on all parameters ro be consistent

snuyanzin · 2026-05-11T19:24:50Z

 		<hiverunner.version>4.0.0</hiverunner.version>
 		<reflections.version>0.9.8</reflections.version>
+		<!-- Allow tests to run on Java 17+ where Hive uses deep reflection on java.base internals -->
+		<flink.surefire.baseArgLine>-XX:+UseG1GC -Xms256m --add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.net=ALL-UNNAMED --add-opens java.base/java.nio=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens java.base/java.time=ALL-UNNAMED --add-exports java.security.jgss/sun.security.krb5=ALL-UNNAMED</flink.surefire.baseArgLine>


instead of putting everything in one heap can we also put a comment for each flag: what test requires it?
similar things we have in flink main repo

This was a mistake. We don't need Java 17 for Junit5. This change got carried over from some Hive4 work I was doing. Rolling it back

snuyanzin · 2026-05-11T19:25:38Z

 * setup
 */
-public class HiveCatalogUdfITCase extends AbstractTestBaseJUnit4 {
+public class HiveCatalogUdfITCase extends AbstractTestBase {


do we still need to have junit5 tests public?

removed public from all tests except HiveGenericUDTFTest because HiveDialectITCase uses it

snuyanzin · 2026-05-11T19:26:27Z


-    @AfterClass
+    @AfterAll
    public static void closeCatalog() {


do we still need method marked with junit5's BeforeAll, AfterAll, Test public?

removed public from all JUnit annotated methods

@after

Migrate 6 test files using @BeforeClass/@AfterClass/@Before/@after: - HiveDynamicTableFactoryTest - HiveLookupJoinITCase - HiveSourceITCase - HiveDialectQueryPlanTest - HiveCatalogTest - HiveTableSourceITCase @BeforeClass -> @BeforeAll, @afterclass -> @afterall, @before -> @beforeeach, @after -> @AfterEach, @test(timeout=N) -> @test @timeout

…atch 4) Migrate 7 test files using @Rule/@ClassRule TemporaryFolder: - HiveModuleTest - HiveSourceFileEnumeratorTest - HivePartitionUtilsTest - HiveCatalogUdfITCase - HiveCatalogITCase - HiveDialectITCase - TableEnvHiveConnectorITCase @rule TemporaryFolder -> @tempdir Path, @ClassRule static TemporaryFolder -> @tempdir static Path, LegacyRowResource -> manual before()/after() in lifecycle methods

Migrate 4 test files with complex JUnit 4 features: - HiveCatalogDataTypeTest (ExpectedException -> assertThatThrownBy) - HiveCatalogFactoryTest (TestExecutorResource -> TestExecutorExtension, ExpectedException -> assertThatThrownBy, TemporaryFolder -> @tempdir) - HiveDialectQueryITCase (ClassRule TemporaryFolder -> @tempdir, ComparisonFailure -> AssertionFailedError) - HiveDialectAggITCase (removed unused TemporaryFolder) HiveRunnerITCase kept on JUnit 4 vintage — FlinkEmbeddedHiveRunner extends BlockJUnit4ClassRunner with no JUnit 5 equivalent.

…ion (batch 6) - Convert FlinkEmbeddedHiveRunner to FlinkEmbeddedHiveRunnerExtension - Remove FlinkStandaloneHiveRunner (unused) - Remove public from test classes and JUnit 5 annotated methods - Remove mixed final/non-final parameter style - Add per-flag comments to flink.surefire.baseArgLine

jlalwani-amazon · 2026-05-12T00:13:42Z

Ready for re-review. Verified that the PR build passes on my github account

snuyanzin · 2026-05-12T06:05:43Z


 /** Test for {@link HiveCatalog} created by {@link HiveCatalogFactory}. */
-public class HiveCatalogFactoryTest extends TestLogger {
+class HiveCatalogFactoryTest {


this drops junit4's logger however doesn't add anything from junit5

I guess we should use @ExtendWith(TestLoggerExtension.class)
like for instance in ImmutableByteArrayWrapperTest of main repo

Note that because TestLoggerExtension is in the META-INF/services file, the extension will auto-register. However, to be consistent with main repo and other tests, I am adding the declarative registration in next commit

snuyanzin · 2026-05-12T06:10:14Z

            // test partition location
            tableEnv.executeSql("create table tbl2 (x int) partitioned by (p string)");
-            location = tempFolder.newFolder(",");
+            location = Files.createDirectories(tempFolder.resolve(",")).toFile();


we have such line in multiple places.
Should we extract it in a util method and reuse in a shorter manner?

Good catch! Fixing in next commit

@ExtendWith

- Add @ExtendWith(TestLoggerExtension.class) to HiveCatalogFactoryTest to replace dropped TestLogger base class - Extract createSubDir helper in TableEnvHiveConnectorITCase to reduce repetition of Files.createDirectories(tempFolder.resolve(...)).toFile()

jlalwani-amazon · 2026-05-12T17:16:24Z

Both comments addressed. Ready for re-review

snuyanzin · 2026-05-14T09:05:02Z

+                Files.createDirectories(temporaryFolder.resolve("testHiveTablePartitionSerDe"))
+                        .toFile();


didn't we mention it in prev review iteration?

snuyanzin · 2026-05-14T09:06:39Z

+            LOGGER.info("Tearing down {}", context.getDisplayName());
            try {
                container.tearDown();
            } catch (Throwable e) {
                LOGGER.warn("Tear down failed: " + e.getMessage(), e);
            }
        }
+        if (temporaryFolder != null) {
+            temporaryFolder.delete();
+        }


does it mean in case of exception nobody will drop the folder?

why can't we use @tempdir?

@tempdir doesn't work on Extesitons. It tried is and I get this

java.lang.NullPointerException at FlinkEmbeddedHiveServerContext.newFolder(FlinkEmbeddedHiveServerContext.java:217) at FlinkEmbeddedHiveServerContext.createAndSetFolderProperty(FlinkEmbeddedHiveServerContext.java:243) at FlinkEmbeddedHiveServerContext.configureFileSystem(FlinkEmbeddedHiveServerContext.java:203) at FlinkEmbeddedHiveServerContext.init(FlinkEmbeddedHiveServerContext.java:86) at FlinkEmbeddedHiveRunnerExtension.createHiveServerContainer(FlinkEmbeddedHiveRunnerExtension.java:106) at FlinkEmbeddedHiveRunnerExtension.beforeAll(FlinkEmbeddedHiveRunnerExtension.java:81)

See https://stackoverflow.com/questions/54647577/how-to-use-tempdir-in-extensions

Also, afterAll is called even if there is an exception in the test. We are catching exception fro container.tearDown. So, the folder should get deleted even if there is exception in container shutdown

ok, but then we are mixing junit4 and junit5 in this extension , aren't we?

snuyanzin · 2026-05-14T09:08:12Z

-    public void testCreateTableWithConstraints() throws Exception {
-        Assume.assumeTrue(HiveVersionTestUtil.HIVE_310_OR_LATER);
+    void testCreateTableWithConstraints() throws Exception {
+        Assumptions.assumeTrue(HiveVersionTestUtil.HIVE_310_OR_LATER);


Assumptions and Assertions should be in static import

ok. I tried to keep the same convention as existing code. I'll fix in next commit.

Do we have any other coding conventions besides checkstyle that I should be aware about?

Should we use static imports everywhere? or only for Assumptions and Assertions. HiveTestUtils has static methods that are not being imported staticly. I'm not changing those static imports

snuyanzin · 2026-05-14T09:09:20Z

-        String sinkPath = new File(tempFolder.newFolder(), "csv-order-sink").toURI().toString();
+        String sinkPath =
+                new File(
+                                Files.createDirectories(tempFolder.resolve("csvSink")).toFile(),


seems to be not addressed here and in other places

Pulling this code into a util method won't reduce the code too much. Because we will need to do something like FilesUtil.createSubDir(tempFolder, "csvSink") instead of Files.createDirectories(tempFolder.resolve("csvSink")).toFile().

Alternatively, we can pull the tempdir into a extension, but it seems overkill for one line of code.

I'll pull the createSubDir into a common util class in next commit

snuyanzin · 2026-05-14T09:10:33Z

@jlalwani-amazon ai slop doesn't handle properly PR description

what is the reason to have this in description

Commit 3 - Java 17+ compatibility: Add --add-opens JVM flags to flink.surefire.baseArgLine (test-scoped only). Fixes 61 pre-existing test errors on Java 17+ caused by Hive's deep reflection on java.base internals.

if there is no change about it?

Please correct all other points as well

…atic imports - Extract HiveTestUtils.createTempSubDir() to deduplicate Files.createDirectories(tempFolder.resolve(...)).toFile() pattern - Use static import for Assumptions.assumeTrue in HiveDialectITCase - Remove private createSubDir helper from TableEnvHiveConnectorITCase

snuyanzin · 2026-05-17T11:56:41Z


-    private static final Logger LOGGER = LoggerFactory.getLogger(FlinkEmbeddedHiveRunner.class);
    private HiveShellContainer container;
+    private TemporaryFolder temporaryFolder;


probably missed in previous iteration
if it is junit5 extension why do we use junit4's class here?

boring-cyborg Bot added the component=Connectors/Hive label Apr 27, 2026

jlalwani-amazon marked this pull request as draft April 27, 2026 21:21

jlalwani-amazon marked this pull request as ready for review April 27, 2026 21:26

jlalwani-amazon changed the title ~~[FLINK-27939][hive] Migrate simple unit tests to JUnit 5~~ [FLINK-27939][hive] Migrate unit tests to JUnit 5 Apr 27, 2026

jlalwani-amazon force-pushed the flink-27939-junit5-migration branch from 44b0025 to 63ca36e Compare April 27, 2026 22:15

jlalwani-amazon marked this pull request as draft April 27, 2026 22:22

jlalwani-amazon force-pushed the flink-27939-junit5-migration branch from 63ca36e to 5c8cefb Compare April 27, 2026 22:47

jlalwani-amazon marked this pull request as ready for review April 27, 2026 22:55

boring-cyborg Bot added the component=BuildSystem label Apr 29, 2026

jlalwani-amazon force-pushed the flink-27939-junit5-migration branch 3 times, most recently from 46e7d55 to 4409a6e Compare May 8, 2026 23:43

snuyanzin reviewed May 9, 2026

View reviewed changes

jlalwani-amazon added 2 commits May 11, 2026 17:14

jlalwani-amazon force-pushed the flink-27939-junit5-migration branch from 44662c3 to 76ca022 Compare May 11, 2026 17:16

snuyanzin reviewed May 11, 2026

View reviewed changes

jlalwani-amazon force-pushed the flink-27939-junit5-migration branch 4 times, most recently from ed0aa00 to d2ba23a Compare May 11, 2026 23:22

jlalwani-amazon added 4 commits May 11, 2026 23:25

jlalwani-amazon force-pushed the flink-27939-junit5-migration branch from d2ba23a to 312468a Compare May 11, 2026 23:27

snuyanzin reviewed May 12, 2026

View reviewed changes

jlalwani-amazon force-pushed the flink-27939-junit5-migration branch from d7be91e to d91a2f2 Compare May 12, 2026 15:05

jlalwani-amazon force-pushed the flink-27939-junit5-migration branch from d91a2f2 to 081544f Compare May 12, 2026 15:32

snuyanzin reviewed May 14, 2026

View reviewed changes

jlalwani-amazon force-pushed the flink-27939-junit5-migration branch 5 times, most recently from 246bee5 to 3b1d2e9 Compare May 14, 2026 17:06

jlalwani-amazon force-pushed the flink-27939-junit5-migration branch from 3b1d2e9 to 05d0442 Compare May 14, 2026 17:10

snuyanzin reviewed May 17, 2026

View reviewed changes

		Files.createDirectories(temporaryFolder.resolve("testHiveTablePartitionSerDe"))
		.toFile();

Conversation

jlalwani-amazon commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snuyanzin commented May 11, 2026

Uh oh!

jlalwani-amazon commented May 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jlalwani-amazon commented May 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jlalwani-amazon commented May 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jlalwani-amazon May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snuyanzin commented May 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jlalwani-amazon commented Apr 27, 2026 •

edited

Loading

jlalwani-amazon May 14, 2026 •

edited

Loading