Skip to content

[AMORO-4203][mixed-flink-common] Migrate tests from JUnit 4 to JUnit 5#4206

Open
lintingbin wants to merge 2 commits intoapache:masterfrom
lintingbin:feat/junit5-amoro-mixed-flink-common
Open

[AMORO-4203][mixed-flink-common] Migrate tests from JUnit 4 to JUnit 5#4206
lintingbin wants to merge 2 commits intoapache:masterfrom
lintingbin:feat/junit5-amoro-mixed-flink-common

Conversation

@lintingbin
Copy link
Copy Markdown
Contributor

What is this PR for?

Closes part of #4203 (Step 6 — amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common). 53 test files migrated to JUnit Jupiter, including 14 @RunWith(Parameterized.class) classes rewritten as @ParameterizedTest @MethodSource, plus the AmoroRunListener test execution listener migration.

AmoroRunListener: JUnit 4 RunListener → JUnit 5 TestExecutionListener

AmoroRunListener was a JUnit 4 RunListener registered through this module's surefire <listener> property. JUnit Platform's surefire provider does not honour that property, so the class is rewritten to implement org.junit.platform.launcher.TestExecutionListener and registered via SPI.

Concretely:

  • The class moves from amoro-common/src/test/java/org/apache/amoro/listener/AmoroRunListener.java to amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common/src/test/java/.... It had no other consumers in the repository — verified via grep — so the move keeps it co-located with its only user and lets the closing PR of [Improvement]: Remove JUnit 4 from the repository #4203 leave amoro-common test sources untouched.
  • The lifecycle hooks map cleanly: testRunStarted/testRunFinishedtestPlanExecutionStarted/Finished, testStarted/testFinishedexecutionStarted/Finished filtered to TestIdentifier#isTest(). Method names come from TestIdentifier#getDisplayName() instead of Description#getMethodName().
  • A new SPI service file src/test/resources/META-INF/services/org.junit.platform.launcher.TestExecutionListener registers the listener for this module only (it stays opt-in elsewhere).
  • junit-platform-launcher is added as an explicit <scope>test</scope> dependency in this module's pom — surefire pulls it in transitively at runtime, but the listener references the launcher API at compile time. The old <properties><property><name>listener</name>…</property></properties> block is removed; <argLine>-verbose:class</argLine> stays.

Test base classes: clean migration, no dual-mode

Unlike #4205 (Step 3 in iceberg), the test bases in this module (FlinkTestBase, AmoroCatalogITCaseBase, CatalogITCaseBase, TestMixedCatalog) have no external consumers — verified via grep across the repository. So they migrate cleanly with no JUnit 4 surface.

One non-mechanical structural decision worth your attention (please push back if you'd prefer the alternative):

FlinkTestBase, AmoroCatalogITCaseBase, CatalogITCaseBase, and TestMixedCatalog no longer extend org.apache.amoro.catalog.TableTestBase / CatalogTestBase / org.apache.amoro.formats.AmoroCatalogTestBase. The catalog/table lifecycle is reimplemented inline against the same CatalogTestHelper / TableTestHelper / AmoroCatalogTestHelper contracts.

The reasons:

  1. The parent classes pass helpers via constructor injection (super(catalogTestHelper, tableTestHelper)), which is incompatible with @ParameterizedTest — Jupiter doesn't inject test instances from method parameters into the constructor. Even with the dual-mode no-arg constructor that [AMORO-4203][format-iceberg] Migrate tests from JUnit 4 to JUnit 5 #4205 adds, every flink ITCase would still need to call setupTable(c, t) from each method — at which point inheriting the parent gives little leverage and adds a JUnit 4 dependency that [Improvement]: Remove JUnit 4 from the repository #4203's closing PR would still have to clean up.
  2. Once LookupITCase / TestLookupSecondary re-extended FlinkTestBase via the new CatalogITCaseBase, the inherited static helper createKeyedTaskWriter(KeyedTable, RowType, boolean, long) clashed with a same-signature default method on FlinkTableTestBase. The 4-arg static was renamed to createKeyedTaskWriterWithMask; its only external caller (TestRowDataReaderFunction) was updated. The 2-arg / 3-arg overloads keep their original names for source compatibility.

The cost of inlining is some duplicated setup boilerplate. The benefit is that flink-common is now JUnit 5-pure: the closing PR of #4203 will not need to touch this module at all.

Parameterized rewrite (14 classes)

Standard template per czy006's preference on #4004:

public class TestX extends FlinkTestBase {
  public static Stream<Arguments> parameters() {
    return Stream.of(Arguments.of(c1, t1), Arguments.of(c2, t2), ...);
  }

  @ParameterizedTest(name = "{0}, {1}")
  @MethodSource("parameters")
  public void testFoo(CatalogTestHelper c, TableTestHelper t) throws Exception {
    setup(c, t);
    // existing body
  }
}

Classes converted: FlinkAmoroCatalogITCase, FlinkUnifiedCatalogITCase, TestMixedCatalog, TestKVTable, MixedIncrementalLoaderTest, TestRoundRobinShuffleRulePolicy, TestKeyed, TestTableRefresh, TestUnkeyed, TestUnkeyedOverwrite, TestAdaptHiveWriter, TestAutomaticLogWriter, TestFlinkSink, TestMixedFormatFileWriter.

Other mechanical changes

  • org.junit.*org.junit.jupiter.api.*
  • @Before / @After@BeforeEach / @AfterEach; @BeforeClass / @AfterClass@BeforeAll / @AfterAll
  • Assert.assertX(...)Assertions.assertX(...) with corrected message-arg order (JUnit 4's leading-message position flips to trailing in JUnit 5)
  • @Rule TemporaryFolder@TempDir Path; temp.newFolder()Files.createTempDirectory(temp, "...").toFile()
  • @Test(expected = X.class)Assertions.assertThrows(X.class, () -> ...)
  • org.junit.rules.TestNameTestInfo parameter injection; TestUtil.getUtMethodName(TestInfo) updated (only flink-common consumers)
  • Mockito: @RunWith(MockitoJUnitRunner.class)@ExtendWith(MockitoExtension.class)
  • @Ignore@Disabled; org.junit.AssumeAssumptions

Type of change

  • Improvement (test infrastructure)

How was this patch tested?

  • mvn -pl amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common -am test423 run, 0 failures, 0 errors, 102 skipped in ~9:24. The 102 skipped are Hive-only parameter sets gated by Assumptions.assumeTrue/False(...) on macOS — same skip count as on master.
  • mvn -pl amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common spotless:check clean.
  • Sanity grep for leftover JUnit 4 imports / annotations in this module's test sources is empty.

Migrate all 53 test files in amoro-mixed-flink-common to JUnit Jupiter and
convert AmoroRunListener from a JUnit 4 RunListener to a JUnit 5
TestExecutionListener.

AmoroRunListener was only consumed by this module's surefire <listener>
property, so it moves from amoro-common/src/test to amoro-mixed-flink-common
and is registered via META-INF/services/org.junit.platform.launcher.TestExecutionListener.
The surefire <listener> property is removed; -verbose:class argLine remains.
junit-platform-launcher is added as a test dependency for the launcher API.

Parameterized rewrites: 14 @RunWith(Parameterized.class) classes converted
to @ParameterizedTest @MethodSource (or @valuesource where the parameter is
a single boolean/string). Their base classes drop the JUnit 4 constructor +
@before lifecycle in favor of an explicit setUpForParam(...) / initX(...)
helper that each @ParameterizedTest method calls.

The flink-common base classes FlinkTestBase, AmoroCatalogITCaseBase and
CatalogITCaseBase were rewritten to no longer extend the still-JUnit-4
TableTestBase / CatalogTestBase / AmoroCatalogTestBase from amoro-common
and amoro-format-iceberg. The catalog/table lifecycle (TestAms,
MiniClusterWithClientResource, TemporaryFolder, setupCatalog, setupTable,
dropCatalog, dropTable) is reimplemented inline against the same
CatalogTestHelper / TableTestHelper / AmoroCatalogTestHelper contracts.
This avoids a Java compile clash between the static
createKeyedTaskWriter(K, T, b, long) inherited from FlinkTestBase and the
default method of the same signature on the FlinkTaskWriterBaseTest
interface; the 4-arg static helper is renamed to createKeyedTaskWriterWithMask.

TestUtil.getUtMethodName(TestName) is migrated to take a JUnit 5 TestInfo
since the only callers are tests in this module. The two test classes that
formerly relied on @rule TestName now receive TestInfo as an
@beforeeach / @ParameterizedTest parameter.

Other mechanical changes: org.junit.* -> org.junit.jupiter.api.*;
@Before/@after -> @BeforeEach/@AfterEach; Assert.assertX(...) ->
Assertions.assertX(...) with corrected message-arg order;
@rule TemporaryFolder -> @tempdir Path on TestKVTable.

mvn -pl amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common -am test
passes (423 run, 0 failures, 0 errors, 102 skipped on macOS - the skipped
tests are the Hive-specific parameter sets gated by Assumptions).
@github-actions github-actions Bot added module:mixed-flink Flink moduel for Mixed Format type:build labels May 7, 2026
@lintingbin
Copy link
Copy Markdown
Contributor Author

@czy006 @xxubai CI is green. This is the flink-common piece of #4203 — 53 files + the AmoroRunListener migration to a JUnit 5 TestExecutionListener (registered via SPI), 14 @RunWith(Parameterized.class) classes rewritten. One non-mechanical decision worth your eye is flagged in the PR body: FlinkTestBase / AmoroCatalogITCaseBase / CatalogITCaseBase no longer extend the still-JUnit-4 iceberg/amoro-common bases; the catalog/table lifecycle is reimplemented inline. Happy to refactor to keep the inheritance if you'd prefer — please push back either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:mixed-flink Flink moduel for Mixed Format type:build

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants