[AMORO-4203][mixed-flink-common] Migrate tests from JUnit 4 to JUnit 5#4206
Open
lintingbin wants to merge 2 commits intoapache:masterfrom
Open
[AMORO-4203][mixed-flink-common] Migrate tests from JUnit 4 to JUnit 5#4206lintingbin wants to merge 2 commits intoapache:masterfrom
lintingbin wants to merge 2 commits intoapache:masterfrom
Conversation
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).
Contributor
Author
|
@czy006 @xxubai CI is green. This is the flink-common piece of #4203 — 53 files + the |
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 theAmoroRunListenertest execution listener migration.AmoroRunListener: JUnit 4 RunListener → JUnit 5 TestExecutionListener
AmoroRunListenerwas a JUnit 4RunListenerregistered through this module's surefire<listener>property. JUnit Platform's surefire provider does not honour that property, so the class is rewritten to implementorg.junit.platform.launcher.TestExecutionListenerand registered via SPI.Concretely:
amoro-common/src/test/java/org/apache/amoro/listener/AmoroRunListener.javatoamoro-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 leaveamoro-commontest sources untouched.testRunStarted/testRunFinished→testPlanExecutionStarted/Finished,testStarted/testFinished→executionStarted/Finishedfiltered toTestIdentifier#isTest(). Method names come fromTestIdentifier#getDisplayName()instead ofDescription#getMethodName().src/test/resources/META-INF/services/org.junit.platform.launcher.TestExecutionListenerregisters the listener for this module only (it stays opt-in elsewhere).junit-platform-launcheris 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, andTestMixedCatalogno longer extendorg.apache.amoro.catalog.TableTestBase/CatalogTestBase/org.apache.amoro.formats.AmoroCatalogTestBase. The catalog/table lifecycle is reimplemented inline against the sameCatalogTestHelper/TableTestHelper/AmoroCatalogTestHelpercontracts.The reasons:
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 callsetupTable(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.LookupITCase/TestLookupSecondaryre-extendedFlinkTestBasevia the newCatalogITCaseBase, the inherited static helpercreateKeyedTaskWriter(KeyedTable, RowType, boolean, long)clashed with a same-signature default method onFlinkTableTestBase. The 4-arg static was renamed tocreateKeyedTaskWriterWithMask; 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:
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/@AfterAllAssert.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.TestName→TestInfoparameter injection;TestUtil.getUtMethodName(TestInfo)updated (only flink-common consumers)@RunWith(MockitoJUnitRunner.class)→@ExtendWith(MockitoExtension.class)@Ignore→@Disabled;org.junit.Assume→AssumptionsType of change
How was this patch tested?
mvn -pl amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common -am test→ 423 run, 0 failures, 0 errors, 102 skipped in ~9:24. The 102 skipped are Hive-only parameter sets gated byAssumptions.assumeTrue/False(...)on macOS — same skip count as on master.mvn -pl amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common spotless:checkclean.