[AMORO-4203][format-iceberg] Migrate tests from JUnit 4 to JUnit 5#4205
[AMORO-4203][format-iceberg] Migrate tests from JUnit 4 to JUnit 5#4205lintingbin wants to merge 2 commits into
Conversation
Migrate 49 test files in amoro-format-iceberg to JUnit Jupiter, including 15 @RunWith(Parameterized.class) classes rewritten as @ParameterizedTest @MethodSource per czy006's preference on apache#4004. CatalogTestBase, TableTestBase, and TableDataTestBase remain dual-mode because they are still extended by JUnit 4 Parameterized subclasses in amoro-format-mixed-hive and amoro-ams (Steps 4 and 8 will migrate them). The original (CatalogTestHelper, TableTestHelper) constructor and @before lifecycle are preserved; new no-arg constructor + setupCatalog(c) / setupTable(c, t) entry points are added for JUnit 5 callers, which invoke them explicitly from @ParameterizedTest method bodies. The @ClassRule TestAms and @rule TemporaryFolder are kept for the JUnit 4 path; the JUnit 5 path initialises the temp folder manually inside setupCatalog because JUnit 4 @rule does not fire under the JUnit Platform engine. All of this dual plumbing is removed in the closing PR of apache#4203 once the cross-module consumers migrate. TestMixedCatalog, TestTaskReader, TestTaskWriter, and TestTableTrashManagers also remain on JUnit 4 for the same reason: they are subclassed by TestMixedHiveCatalog, TestHiveTaskReader, TestHiveTaskWriter, and TestHiveTableTrashManagers in amoro-mixed-hive. TestIcebergAmoroCatalog and TestMixedIcebergFormatCatalog stay on JUnit 4 because their parent TestAmoroCatalogBase lives in amoro-common, which the umbrella plan keeps on JUnit 4 until the closing PR. TestIcebergFindFiles extends Iceberg's TestBase (already JUnit 5 in 1.7.2), so it is rewritten with @testtemplate + ParameterizedTestExtension. Parameterized rewrite template: each old @test becomes a @ParameterizedTest @MethodSource("parameters") taking the helpers as method args and calling setupTable(c, t) at the top of the body. The Parameters source method returns Stream<Arguments>. 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 (in non-dual-mode files); @test(expected=X) -> Assertions.assertThrows(X, () -> ...); @ignore -> @disabled. JUnit 4 Assert.assertThrows(message, class, executable) flips to JUnit 5 Assertions.assertThrows(class, executable, message). mvn -pl amoro-format-iceberg -am test passes (501/506, 5 skipped via JUnit 5 Assumptions). mvn -pl amoro-format-mixed-hive,amoro-ams -am test-compile passes.
|
@czy006 @xxubai CI is green. This is the iceberg piece of #4203 — 49 files, 15 |
|
@xxubai Thanks for keeping the branch fresh with master 🙏. Is there anything you'd like me to change before this can move forward — for example the dual-mode contract on |
What is this PR for?
Closes part of #4203 (Step 3 —
amoro-format-iceberg). 49 test files migrated to JUnit Jupiter, including 15@RunWith(Parameterized.class)classes rewritten as@ParameterizedTest @MethodSourceper @czy006's preference expressed on #4004.Dual-mode test bases (the central design decision)
CatalogTestBase,TableTestBase, andTableDataTestBaseare still extended by ~19 JUnit 4@RunWith(Parameterized.class)subclasses inamoro-format-mixed-hiveandamoro-amsthat Steps 4 and 8 of #4203 will migrate later. Migrating them now would either force a 200+ file mega-PR or drop into the same trap as #4004. Instead, this PR keeps the JUnit 4 surface intact and adds a parallel JUnit 5 surface:(CatalogTestHelper, TableTestHelper)constructor and@Before setupCatalog()/@Before setupTable()are preserved — JUnit 4 Parameterized callers continue to work unchanged.protected void setupCatalog(CatalogTestHelper)/protected void setupTable(CatalogTestHelper, TableTestHelper)entry points let JUnit 5 callers initialise the catalog/table state explicitly from their@ParameterizedTestmethod body.@ClassRule TestAmsand@Rule TemporaryFolderare kept for the JUnit 4 path. For the JUnit 5 path we add@BeforeAll startTestAms()/@AfterAll stopTestAms()calling the publicbefore()/after()ofTestAmsdirectly, and theTemporaryFolderis initialised manually insidesetupCatalog(helper)because JUnit 4@Ruledoes not fire under the JUnit Platform engine.Cross-module compile is verified:
mvn -pl amoro-format-mixed-hive,amoro-ams -am test-compilebuilds cleanly. All this dual plumbing is removed in the closing PR of #4203.Files intentionally kept on JUnit 4 in this PR
TestMixedCatalog,TestTaskReader,TestTaskWriter,TestTableTrashManagers— same dual-mode reasoning, they are subclassed byTestMixedHiveCatalog/TestHiveTaskReader/TestHiveTaskWriter/TestHiveTableTrashManagersin mixed-hive.TestIcebergAmoroCatalog,TestMixedIcebergFormatCatalog— extendTestAmoroCatalogBaseinamoro-common, which the umbrella plan keeps on JUnit 4 until the closing PR.Parameterized rewrite template
TestIcebergFindFilesis a small exception: its parentorg.apache.iceberg.TestBase(Iceberg 1.7.2) is already JUnit 5 native, so it migrates to@TestTemplate+ParameterizedTestExtensionto match upstream's pattern.Other mechanical changes
org.junit.*→org.junit.jupiter.api.*@Before/@After→@BeforeEach/@AfterEachAssert.assertX(...)→Assertions.assertX(...)with corrected message-arg order (JUnit 4's leading-message position flips to trailing in JUnit 5)@Rule TemporaryFolder→@TempDir Pathin non-dual-mode files;temp.newFolder()→Files.createTempDirectory(temp, "...").toFile()@Test(expected = X.class)→Assertions.assertThrows(X.class, () -> ...)@Ignore→@Disabledorg.junit.Assume→AssumptionsType of change
How was this patch tested?
mvn -pl amoro-format-iceberg -am test→ 501 run, 0 failures, 0 errors, 5 skipped (skipped via JUnit 5Assumptions.assumeTrue/False, not failures). Both Vintage and Jupiter engines run.mvn -pl amoro-format-mixed-hive,amoro-ams -am test-compile→ BUILD SUCCESS (dual-mode contract preserved for downstream Steps 4 / 8).mvn -pl amoro-format-iceberg spotless:checkclean.