gortiz opened a new pull request, #18781: URL: https://github.com/apache/pinot/pull/18781
## Run Pinot's tests on the JUnit Platform (TestNG + Jupiter side by side) ### TL;DR This moves the test build onto the **JUnit Platform** so we can write new tests with the modern **JUnit 5 (Jupiter)** API — **without rewriting any of the ~10,000 existing TestNG tests**. They keep running, unchanged, through the [TestNG engine for the JUnit Platform](https://github.com/junit-team/testng-engine). New and old tests run together in a single Surefire execution. This is intentionally a **foundation-only** PR: build wiring + the two unavoidable suite-file translations + one example test + a short guide. No bulk migration. ### Why TestNG has served us well, but JUnit 5 is where the ecosystem and tooling have moved. The JUnit Platform is *engine-pluggable* — JUnit Vintage runs JUnit 4, Jupiter runs JUnit 5, and the TestNG engine runs TestNG — so adopting it is a **build change, not a test rewrite**. Once we're on the platform, a lot of capability we don't have today becomes available incrementally, test-by-test, at each author's discretion: - **Extensions instead of inheritance.** Jupiter's `@ExtendWith` / `RegisterExtension` model replaces today's deep `ControllerTest → ClusterTest → BaseClusterIntegrationTest` base-class chains with composable behavior. Cross-cutting setup (clusters, ZK, leak detection) becomes a reusable extension rather than something every test must inherit. - **`@Nested` inner classes with their own `@BeforeEach`/`@AfterEach`.** Group related cases with scoped, layered setup instead of one giant flat test class. - **Declarative resource cleanup with `@AutoClose`** (and `@TempDir`) — fields are closed automatically after the test, removing hand-written teardown and a class of leaks. - **Parameterized tests as a first-class feature** — `@ParameterizedTest` with `@ValueSource`, `@CsvSource`, `@MethodSource`, `@EnumSource`. A cleaner, less boilerplate-heavy replacement for `@DataProvider` (and great for exhaustive type/null-handling coverage, which we care about a lot). - **Dynamic tests** (`@TestFactory`) — generate cases at runtime, e.g. one test per query file, per segment format, or per discovered fixture. - **Built-in parallel execution** — Jupiter's `junit-platform.properties` parallelism (per method/class, with `@Execution`/`@ResourceLock` for safety) gives a standard knob for speeding up suites, instead of TestNG's bespoke config. - **Better tooling reach.** Being on the JUnit Platform unlocks tools that target it — for example [**Fray**](https://github.com/cmu-pasta/fray), a concurrency-testing / deterministic-scheduling framework that hunts for races and deadlocks. Pinot has a lot of concurrency-heavy code (upsert metadata, consumer coordination, Helix state); a JUnit 5 integration for that kind of tooling would be valuable and is only practical once we're on the platform. Crucially, **none of this is forced.** Existing tests stay as TestNG and keep their `org.testng.Assert` semantics. We adopt the new capabilities where they pay off. ### What's in this PR 1. **Root `pom.xml`** — import the JUnit BOM (unified 6.x versioning), add `testng-engine`, `junit-jupiter`, and `junit-platform-launcher` as inherited test dependencies, and drop the `surefire-testng` provider so Surefire auto-selects its `junit-platform` provider. The launcher is on the test classpath so it stays version-aligned with the engines in the forked JVM. 2. **`pinot-controller` / `pinot-integration-tests`** — the TestNG engine does **not** support `testng.xml` suite files (the only two in the build). TestNG `@Test(groups=...)` are exposed to the platform as **tags**, so the controller stateful/stateless split becomes two tag-filtered Surefire executions, and the custom-cluster integration suite becomes an include glob. The controller executions use `reuseForks=true` to preserve the shared-cluster setup the single-JVM suites relied on (verified: the second stateful class reuses the cluster, starting zero new controllers). The now-dead `ControllerTestSetup` (`@BeforeGroups`/`@AfterGroups`, no `@Test`) is removed. 3. **`BooleanUtilsTest`** — a small, real JUnit 5 test (adds genuine coverage) demonstrating `@ParameterizedTest` and `assertThrows`, running in the same module as 685 TestNG tests to prove coexistence. 4. **`docs/junit5-migration.md`** — conventions for new Jupiter tests and a TestNG→Jupiter mapping table. ### Verification - `pinot-spi`: 685 existing TestNG tests pass under the new provider; **698** with the new Jupiter test — both engines run together in one execution. - `pinot-controller`: both tag-filtered executions fire; group→tag mapping confirmed; the shared-cluster optimization is preserved. - `@Listeners` (e.g. `NettyTestNGListener` for Netty leak detection) is honored by the engine. - Coverage is unaffected: JaCoCo attaches via `@{argLine}` (provider-agnostic), and `surefire-report` reads the standard `TEST-*.xml` the platform also emits. - Enforcer `dependencyConvergence`, spotless, checkstyle, and license all pass. ### Compatibility / risk - **No behavior change for existing tests** — they run as TestNG via the engine. - Engine compatibility is confirmed: `testng-engine` 1.1.0 is CI-tested against JUnit 6, supports TestNG ≥ 6.14.3 (we use 7.12.0), and needs Surefire ≥ 3.5.2 (we use 3.5.4). - The one thing to watch on merge is a full CI run confirming per-module test counts and that coverage artifacts upload exactly as before. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
