Skip to content

[#14105] test: Replace the last JUnit 4 code and declare the test libraries each module uses - #14103

Open
nevzheng wants to merge 1 commit into
apache:mainfrom
nevzheng:claude/13900-tc-00a0-junit4-assertions
Open

nevzheng wants to merge 1 commit into
apache:mainfrom
nevzheng:claude/13900-tc-00a0-junit4-assertions

Conversation

@nevzheng

@nevzheng nevzheng commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

What changes were proposed in this pull request?

The last JUnit 4 code in Gravitino's tests becomes JUnit Jupiter, every module declares the test libraries it uses, and a checkstyle rule keeps JUnit 4 out.

  • 42 test files switch org.junit.Assert to org.junit.jupiter.api.Assertions; 123 calls move the failure message from the first argument to the last, where Jupiter takes it.
  • Two JUnit 4 classes become Jupiter tests:
    • TestOwnerDetails (clients/cli): the module has no vintage engine, so the JUnit Platform skipped it silently; its 2 tests now run, and pass.
    • TestGravitinoCatalogStore (flink-common): it already ran through the vintage engine flink-test-utils brings; its 8 tests now run on Jupiter.
  • Declare what you use. Tests that compiled only because another library brought their test framework now declare it: mockito-core, assertj-core, junit-jupiter-api, junit-platform-commons/-engine, testcontainers-jdbc and commons-lang3, with new catalog entries for AssertJ and testcontainers-jdbc. Resolved test classpaths are unchanged.
  • No JUnit internals. 19 files used JUnit's internal org.junit.platform.commons.util.StringUtils; they use commons-lang3. TestParseType drops Hamcrest for assertInstanceOf.
  • Real overrides in the Ranger E2E tests. TestChainedAuthorizationIT meant to replace 12 of RangerBaseE2EIT's tests with stubs, but the base methods were package-private in another package, so the stubs were new methods, not overrides. JUnit 5.8.1 hid that; JUnit 5.11+ runs both (its release notes call this out), and the Ranger tests then fail under the chained setup. The base methods become protected and the stubs @Override, so the compiler now catches a mismatch.
  • Checkstyle IllegalImport bans org.junit.* outside jupiter and platform, static imports included. It is the only checkstyle rule; Spotless still owns formatting.

How to review. By pattern, not by file:

  1. Imports: org.junit.Assert.X → org.junit.jupiter.api.Assertions.X, same method.
  2. Message last: assertTrue("msg", cond) → assertTrue(cond, "msg").
  3. Annotations: @Before/@After/@Test → Jupiter, in the two classes only.
  4. Build files: each added test dependency is one the module's tests already import.
  5. StringUtils: JUnit's internal copy → commons-lang3, same methods.

Cross-checked against OpenRewrite's JUnit4to5Migration for the assertion files: the same result in 42 of 44 files. In TestTagCommands the recipe's output does not compile; in TestOwnerDetails this PR uses assertThrows instead of try/fail/catch.

Why are the changes needed?

  • A test class was not running. TestOwnerDetails looked like a test and never ran.
  • Transitive test dependencies break silently. These tests compiled only because Testcontainers 1.x brought junit:junit and other libraries brought Mockito, AssertJ or JUnit APIs. Testcontainers 2 ([#13958] improvement(build): Upgrade Testcontainers to 2.0.5 #14084) drops junit:junit; declaring what each module uses keeps its tests compiling when dependencies shift.
  • The ban keeps it that way. A JUnit 4 import fails checkstyle instead of compiling against a stray junit:junit and not running.

Part of #14105

Does this PR introduce any user-facing change?

No. Test code and test dependencies only.

How was this patch tested?

  • Compiles and passes checkstyle with -Werror in every project; re-adding import org.junit.Test fails checkstyle with "Illegal import".
  • Unit tests pass in the touched modules, 0 failures (cli 509, server-common 500, server 684, idp-basic 117, lance-rest-server 113, catalog-jdbc-common 111, a Trino connector 492).
  • Before / after, in CI's build (17) job: on main, :clients:cli:test --tests '*TestOwnerDetails*' finds no tests; here it runs 2.
  • Resolved test classpaths of every project are identical before and after.

Stack: #14103 → #14083 → #14084 → #14085 → #14005 → #14006 → #14052 → #14076 → #14007 → #14008 → #14009 → #14010 → #14011 → #14012 → #14013 → #14079 → #14077 → #14078 → #14086 → #14087 → #14088 → #14089 → #14090 → #14091 → #14094

@nevzheng nevzheng self-assigned this Oct 21, 2026
This was referenced Oct 21, 2026
@nevzheng
nevzheng force-pushed the claude/13900-tc-00a0-junit4-assertions branch from 5caee46 to 426f743 Compare October 21, 2026 21:18
@nevzheng nevzheng changed the title [#14001] improvement(test): Replace JUnit 4 assertions and tests with JUnit Jupiter [#14105] test: Replace the last JUnit 4 assertions and test classes with JUnit Jupiter Oct 21, 2026
@nevzheng
nevzheng force-pushed the claude/13900-tc-00a0-junit4-assertions branch from 426f743 to 3cbcdb6 Compare October 21, 2026 21:37
@nevzheng
nevzheng requested a review from jerryshao October 21, 2026 22:16
@nevzheng
nevzheng requested a review from roryqi October 21, 2026 22:16
@nevzheng
nevzheng marked this pull request as ready for review October 21, 2026 22:16
@nevzheng
nevzheng force-pushed the claude/13900-tc-00a0-junit4-assertions branch from 3cbcdb6 to 8467d9a Compare October 21, 2026 22:27
@nevzheng nevzheng changed the title [#14105] test: Replace the last JUnit 4 assertions and test classes with JUnit Jupiter [#14105] test: Replace the last JUnit 4 code and declare the test libraries each module uses Oct 21, 2026
@nevzheng
nevzheng force-pushed the claude/13900-tc-00a0-junit4-assertions branch 2 times, most recently from 2200e84 to 91464d8 Compare October 22, 2026 03:28
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.

1 participant