Skip to content

Migrate the test suite from JUnit 4 to JUnit 5 - #840

Merged
slachiewicz merged 1 commit into
masterfrom
agent/junit5-shade-plugin
Aug 9, 2026
Merged

Migrate the test suite from JUnit 4 to JUnit 5#840
slachiewicz merged 1 commit into
masterfrom
agent/junit5-shade-plugin

Conversation

@slachiewicz

Copy link
Copy Markdown
Member

Moves 18 test classes to Jupiter. The bulk is the OpenRewrite JUnit4to5Migration recipe, but three things needed hand work — the third is the one worth reviewing.

TransformerTesterRule was a custom TestRule. The recipe converted the @Test annotations in PropertiesTransformerTest to Jupiter but left the @Rule field alone. That still compiles, which is the dangerous part: under Jupiter the rule would simply never run, and the four spec-driven tests would have passed while asserting nothing. It is rewritten as an InvocationInterceptor and applied with @ExtendWith instead of a @Rule field.

One behavioural note on that rewrite: the JUnit 4 rule deliberately never called base.evaluate() when a @TransformerTest spec was present — the test body never ran. Jupiter requires invocation.proceed() to be called exactly once, so the interceptor now does. That is safe here only because all four @TransformerTest methods have empty bodies ({}), and PropertiesTransformerTest is the only class using the annotation. Worth knowing if anyone later adds a body to one of them.

MinijarFilterTest — the recipe emitted non-compiling code, turning TemporaryFolder.builder().assureDeletion().build() into File.builder().... Replaced with a plain @TempDir File field.

DefaultShaderTest@ClassRule TemporaryFolder TEMPORARY_FOLDER became @TempDir public static File TEMPORARY_FOLDER, which trips two checkstyle rules at once: VisibilityModifier exempts @Rule/@ClassRule but not @TempDir, and StaticVariableName wants lowerCamelCase for non-final statics. Renamed to a package-private static File temporaryFolder.

Left on JUnit 4 deliberately: ShadeMojoTest. It extends AbstractMojoTestCasePlexusTestCasejunit.framework.TestCase, with methods discovered by the test* naming convention. That inheritance lives in maven-plugin-testing-harness, not here. junit:junit therefore stays (the harness marks it optional, so it must be declared explicitly), and junit-vintage-engine is added so the class stays discoverable once surefire switches to the platform provider.

org.hamcrest:hamcrest also stays — several tests use CoreMatchers/MatcherAssert directly. junit-jupiter-engine is deliberately not declared, since surefire provisions it.

src/it and src/test/resources are untouched — confirmed by diffing the OpenRewrite output so that only pom.xml and src/test/java differ.

Verification

mvn test before: Tests run: 72, Failures: 0, Errors: 0, Skipped: 0 (surefire selecting JUnit4Provider).
mvn test after: Tests run: 72, Failures: 0, Errors: 0, Skipped: 0 (surefire selecting JUnitPlatformProvider, with ShadeMojoTest's 5 tests running via the vintage engine).
checkstyle:check 0 violations; spotless:check clean.

Because "the tests still pass" proves nothing about an extension that might silently do nothing, the interceptor was also checked negatively: corrupting one expected value in PropertiesTransformerTest produces a real AssertionFailedError, so it is genuinely enforcing.

Draft until CI confirms.

Generated-by: Claude Opus 5 (1M context)

Convert the 18 remaining JUnit 4 test classes under src/test/java to
JUnit Jupiter using OpenRewrite's JUnit4to5Migration recipe, with
hand fixes for what the recipe could not handle automatically:

- TransformerTesterRule, a custom @Rule/TestRule, is rewritten as a
  JUnit 5 InvocationInterceptor extension and PropertiesTransformerTest
  now uses @ExtendWith instead of @rule.
- MinijarFilterTest's TemporaryFolder.builder() field is replaced with
  a plain package-private @tempdir File field (the recipe produced a
  non-compiling File.builder() call).
- DefaultShaderTest's public static @ClassRule TemporaryFolder becomes
  a package-private static @tempdir File field, renamed to camelCase
  to satisfy checkstyle's VisibilityModifier and StaticVariableName
  rules, which do not exempt @tempdir the way they exempt @rule.

ShadeMojoTest is deliberately left on JUnit 4: it extends
AbstractMojoTestCase, which extends PlexusTestCase, which extends
junit.framework.TestCase. The pom now declares junit-jupiter-api and
junit-vintage-engine (versions managed by the parent's junit-bom) test
dependencies so both engines are discovered by the JUnit Platform
provider, alongside the existing junit:junit dependency that
ShadeMojoTest still needs.

src/it and src/test/resources are untouched; they hold JUnit 4 based
IT/sample fixtures and JAR/class-file shading inputs, not code to
migrate.

Generated-by: Claude Opus 5 (1M context)
@slachiewicz
slachiewicz marked this pull request as ready for review August 9, 2026 22:01
@slachiewicz
slachiewicz merged commit 12eb8f9 into master Aug 9, 2026
15 checks passed
@slachiewicz
slachiewicz deleted the agent/junit5-shade-plugin branch August 9, 2026 22:01
@github-actions github-actions Bot added this to the 3.6.3 milestone Aug 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant