apache / apache/amoro

[Improvement]: Remove JUnit 4 from the repository

Open
#4,203 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
1.2k
Forks
395
Avg merge
4d 10h
Merged PRs (30d)
33

Description

### Search before asking

- [x] I have searched in the [issues](https://github.com/apache/amoro/issues?q=is%3Aissue) and found no similar issues.

### What would you like to be improved?

Remove JUnit 4 (`org.junit.*`) from the entire repository so the parent POM can drop `junit:junit` and `junit-vintage-engine`. Currently ~246 test files still depend on JUnit 4, including ~89 `@RunWith(Parameterized.class)` classes.

This issue tracks the umbrella effort across all modules. The original #3974 was scoped only to `amoro-common`, but the shared test bases there (`TestAms`, `TestHMS`, `AmoroCatalogTestBase`, `TestAmoroCatalogBase`, `TestServerTableDescriptor`, `AmoroRunListener`) are extended or used as `@Rule`/`@ClassRule`/`RunListener` by tests in many other modules. Migrating those bases in isolation breaks downstream modules — that is what blocked #4004 and #4059. So the migration has to happen across all modules in coordinated steps and needs a single umbrella ticket to track ordering.

### How should we improve?

Strategy: **leaves first, root last** — migrate concrete test classes module by module, keeping the shared `amoro-common` helpers on JUnit 4 until every consumer is gone. CI stays green at every step. Each step is a separate PR with body `Closes part of #THIS_ISSUE`.

- [x] **Step 1** — `amoro-common` self-contained tests (#4199)
- [x] **Step 2** — `amoro-optimizer-common` (5 files, smallest leaf, validates the pattern with reviewers) (#4204)
- [ ] **Step 3** — `amoro-format-iceberg` (~55 files; first time exercising the `@RunWith(Parameterized.class)` → `@ParameterizedTest @MethodSource` rewrite, plus iceberg-side consumers of `TestAmoroCatalogBase`/`TestServerTableDescriptor`) (#4205)
- [ ] **Step 4** — `amoro-format-mixed-hive` (~23 files)
- [ ] **Step 5** — `amoro-format-mixed-spark` 3.3 / 3.4 / 3.5 (~13 files)
- [ ] **Step 6** — `amoro-format-mixed-flink-common` (~52 files; converts the `AmoroRunListener` surefire wiring to a JUnit 5 `TestExecutionListener` at the same time) (#4206)
- [ ] **Step 7** — `amoro-format-paimon` + `amoro-mixed-trino` + `amoro-openapi-sdk` + `amoro-mixed-spark-3-common` (~8 files)
- [ ] **Step 8** — `amoro-ams` (~81 files, largest)
- [ ] **Step 9** — Closing PR: convert the 6 `amoro-common` helpers (`TestAms`, `TestHMS`, `AmoroCatalogTestBase`, `TestAmoroCatalogBase`, `TestServerTableDescriptor`, `AmoroRunListener`) to JUnit 5 Extensions / plain classes, then remove `junit:junit` and `junit-vintage-engine` from the parent POM. Also closes #3974.

### Notes on the `Parameterized` rewrite

Subclasses such as `TestIcebergAmoroCatalog`, `TestMixedIcebergFormatCatalog`, and `TestPaimonAmoroCatalog` currently use `@RunWith(Parameterized.class)` with constructor injection of `AmoroCatalogTestHelper`. JUnit 5 has no equivalent of constructor parameter injection without a custom `ParameterResolver`. Per @czy006's preference expressed on #4004, these will be refactored to `@ParameterizedTest` + `@MethodSource` rather than registering a `ParameterResolver` — the latter would be carry-over tech debt.

### Are you willing to submit PR?

- [x] Yes I am willing to submit a PR!

### Code of Conduct

- [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)

Contributor guide

Open the contributing guide

Research direction

Start with the Step 3 entry point, amoro-format-iceberg and its roughly 55 test files, especially TestIcebergAmoroCatalog and TestMixedIcebergFormatCatalog. Read the existing JUnit 4 parameterized tests and the migration pattern from completed steps, then run that module's CI checks. Done means the module no longer depends on the targeted JUnit 4 usages while CI remains green.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
build-system, testing
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.