[AMORO-3974] Migrate JUnit 4 to JUnit 5 in amoro-common module#4004
[AMORO-3974] Migrate JUnit 4 to JUnit 5 in amoro-common module#4004Jungkihong07 wants to merge 11 commits intoapache:masterfrom
Conversation
|
Thank you for your contribution to the new unit test support. You can run |
|
I’ve completed the code formatting. |
|
I noticed that some tests in the amoro-format-iceberg module are failing with the following error: No ParameterResolver registered for parameter [org.apache.amoro.formats.AmoroCatalogTestHelper arg0] in constructor [public org.apache.amoro.formats.TestIcebergAmoroCatalog(org.apache.amoro.formats.AmoroCatalogTestHelper)]. Similar errors occur for several test methods in TestMixedIcebergFormatCatalog, which causes the amoro-format-iceberg module tests to fail and the subsequent modules to be skipped. My understanding is that, in JUnit 4, AmoroCatalogTestHelper was likely injected via a custom runner, rule, or test utility, but after migrating to JUnit 5 there is no ParameterResolver (or equivalent mechanism) configured for this helper. Before making further changes, I would like to ask for guidance on the preferred approach in this project: Should I introduce a JUnit 5 ParameterResolver (and register it via @ExtendWith or the service loader) to provide AmoroCatalogTestHelper? Or would you prefer to refactor these tests to avoid constructor injection and use another pattern that is more consistent with the existing JUnit 5 tests in this repository? Once I know the preferred direction, I can update the tests accordingly. |
If it were me, I would prefer to refactor it. It seems that Junit5 has made some new incompatible changes to ParameterResolver, so it's also feasible for us to achieve equivalent effects in a new way cc @Jungkihong07 |
|
Thank you for the clear direction! I'll proceed with refactoring to align with the JUnit 5 approach used in this repository. cc @czy006 |
|
CI Error |
|
test is failing cc @Jungkihong07 |
Why are the changes needed?
JUnit 4 is no longer actively maintained, and JUnit 5 provides a more modern and powerful testing framework.
This PR migrates all JUnit 4 tests in the
amoro-commonmodule to JUnit 5, enabling the use of the latest testing framework and reducing technical debt.Close #3974.
Brief change log
JUnit 4 → JUnit 5 Migration for amoro-common Module
Migrated Test Files (9 files):
AmoroRunListener.java- ConvertedRunListener→TestExecutionListenerTestServerTableDescriptor.java- Converted@Rule,@ClassRule→@BeforeAll/@AfterAlland@TempDirMemorySizeTest.java- Converted@Test(expected=...)→assertThrows()JacksonUtilTest.javaTestSimpleFuture.javaTestAmoroCatalogBase.javaAmoroCatalogTestBase.javaTestPoolConfig.javaMockZookeeperServer.javaKey Changes:
org.junit.*→org.junit.jupiter.api.*@Before/After→@BeforeEach/AfterEach,@BeforeClass/AfterClass→@BeforeAll/AfterAll(static)Assert.*→Assertions.*@Test(expected=...)→Assertions.assertThrows()@Rule→@RegisterExtensionor@TempDirAmoroRunListenerconversionDetailed Implementation:
Assert.*calls withAssertions.*, updated imports fromorg.junit.*toorg.junit.jupiter.api.*@Before,@After) and related methods (setupCatalogJUnit4(),cleanCatalogJUnit4()), simplified to use only JUnit 5 lifecycle annotations (@BeforeEach,@AfterEach) with@TempDirfor temporary directory managementextends ExternalResource(JUnit 4 Rule), converted to plain Java class, replacedTemporaryFolderwithjava.nio.file.FilesandPathfor temporary directory handlingextends ExternalResource(JUnit 4 Rule), converted to plain Java classMigration Strategy:
createHelper()factory method pattern inAmoroCatalogTestBasefor JUnit 5 test classesHow was this patch tested?
amoro-commonmodule:./mvnw test -pl amoro-common./mvnw clean install -pl amoro-common -amDocumentation
Does this pull request introduce a new feature? (no)
If yes, how is the feature documented? (not applicable)