lintingbin opened a new pull request, #4206:
URL: https://github.com/apache/amoro/pull/4206
## 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 #4203 leave
`amoro-common` test sources untouched.
- The lifecycle hooks map cleanly: `testRunStarted/testRunFinished` →
`testPlanExecutionStarted/Finished`, `testStarted/testFinished` →
`executionStarted/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 #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 #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:
```java
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.TestName` → `TestInfo` parameter injection;
`TestUtil.getUtMethodName(TestInfo)` updated (only flink-common consumers)
- Mockito: `@RunWith(MockitoJUnitRunner.class)` →
`@ExtendWith(MockitoExtension.class)`
- `@Ignore` → `@Disabled`; `org.junit.Assume` → `Assumptions`
## Type of change
- [x] Improvement (test infrastructure)
## 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 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
