Skip to content

[AMORO-4203][format-iceberg] Migrate tests from JUnit 4 to JUnit 5#4205

Open
lintingbin wants to merge 2 commits intoapache:masterfrom
lintingbin:feat/junit5-amoro-format-iceberg
Open

[AMORO-4203][format-iceberg] Migrate tests from JUnit 4 to JUnit 5#4205
lintingbin wants to merge 2 commits intoapache:masterfrom
lintingbin:feat/junit5-amoro-format-iceberg

Conversation

@lintingbin
Copy link
Copy Markdown
Contributor

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 @MethodSource per @czy006's preference expressed on #4004.

Dual-mode test bases (the central design decision)

CatalogTestBase, TableTestBase, and TableDataTestBase are still extended by ~19 JUnit 4 @RunWith(Parameterized.class) subclasses in amoro-format-mixed-hive and amoro-ams that 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:

  • The original (CatalogTestHelper, TableTestHelper) constructor and @Before setupCatalog() / @Before setupTable() are preserved — JUnit 4 Parameterized callers continue to work unchanged.
  • A no-arg constructor is added.
  • New protected void setupCatalog(CatalogTestHelper) / protected void setupTable(CatalogTestHelper, TableTestHelper) entry points let JUnit 5 callers initialise the catalog/table state explicitly from their @ParameterizedTest method body.
  • @ClassRule TestAms and @Rule TemporaryFolder are kept for the JUnit 4 path. For the JUnit 5 path we add @BeforeAll startTestAms() / @AfterAll stopTestAms() calling the public before() / after() of TestAms directly, and the TemporaryFolder is initialised manually inside setupCatalog(helper) because JUnit 4 @Rule does not fire under the JUnit Platform engine.

Cross-module compile is verified: mvn -pl amoro-format-mixed-hive,amoro-ams -am test-compile builds 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 by TestMixedHiveCatalog/TestHiveTaskReader/TestHiveTaskWriter/TestHiveTableTrashManagers in mixed-hive.
  • TestIcebergAmoroCatalog, TestMixedIcebergFormatCatalog — extend TestAmoroCatalogBase in amoro-common, which the umbrella plan keeps on JUnit 4 until the closing PR.

Parameterized rewrite template

public class TestX extends TableTestBase {
  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 {
    setupTable(c, t);
    // existing body
  }
}

TestIcebergFindFiles is a small exception: its parent org.apache.iceberg.TestBase (Iceberg 1.7.2) is already JUnit 5 native, so it migrates to @TestTemplate + ParameterizedTestExtension to match upstream's pattern.

Other mechanical changes

  • org.junit.*org.junit.jupiter.api.*
  • @Before / @After@BeforeEach / @AfterEach
  • 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 in non-dual-mode files; temp.newFolder()Files.createTempDirectory(temp, "...").toFile()
  • @Test(expected = X.class)Assertions.assertThrows(X.class, () -> ...)
  • @Ignore@Disabled
  • org.junit.AssumeAssumptions

Type of change

  • Improvement (test infrastructure)

How was this patch tested?

  • mvn -pl amoro-format-iceberg -am test501 run, 0 failures, 0 errors, 5 skipped (skipped via JUnit 5 Assumptions.assumeTrue/False, not failures). Both Vintage and Jupiter engines run.
  • mvn -pl amoro-format-mixed-hive,amoro-ams -am test-compileBUILD SUCCESS (dual-mode contract preserved for downstream Steps 4 / 8).
  • mvn -pl amoro-format-iceberg spotless:check clean.

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.
@lintingbin
Copy link
Copy Markdown
Contributor Author

@czy006 @xxubai CI is green. This is the iceberg piece of #4203 — 49 files, 15 @RunWith(Parameterized.class) classes rewritten as @ParameterizedTest @MethodSource per your guidance on #4004. The dual-mode design on CatalogTestBase/TableTestBase/TableDataTestBase is the central design decision and is flagged in the PR body; cross-module test-compile of amoro-format-mixed-hive and amoro-ams is verified so Steps 4 and 8 stay unblocked. Review appreciated.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants