Skip to content

Conversation

Jenson3210
Copy link
Contributor

@Jenson3210 Jenson3210 commented Aug 20, 2025

Add recipe to simplify JRE version conditions for JUnit 6 migration

Summary

This PR introduces a new recipe MinimumJreConditions that simplifies JUnit Jupiter test conditions based on a minimum JRE version requirement. As part of the JUnit 6 migration path, this recipe helps clean up obsolete JRE version conditions when migrating to a minimum Java version.

What's Changed

New Recipe: MinimumJreConditions

The recipe processes JUnit Jupiter's JRE conditional annotations (@EnabledOnJre, @DisabledOnJre, @EnabledForJreRange, @DisabledForJreRange) and:

  • Removes obsolete test methods that are conditionally enabled only for JRE versions older than the minimum
  • Simplifies annotations by removing references to older JRE versions from multi-version conditions
  • Cleans up annotations that become redundant when only the minimum JRE version remains
  • Preserves necessary conditions for JRE versions at or above the minimum requirement

Key Features

  1. Supports multiple annotation formats:

    • JRE enum values: JRE.JAVA_8, JRE.JAVA_17, etc. (also the statically imported ones)
    • Value attribute: value = JRE.JAVA_17
    • Version integers: versions = 17 or versions = { 17, 21 }
    • Range attributes: minVersion = 17, maxVersion = 21
  2. Handles range conditions:

    • Removes tests enabled only for ranges ending before the minimum JRE
    • Preserves tests for ranges that include or exceed the minimum JRE
    • Removes annotations for disabled ranges that end before the minimum
  3. Smart annotation cleanup:

    • Removes annotations when they become redundant (e.g., @EnabledOnJre(JRE.JAVA_17) when 17 is the minimum)
    • Updates multi-version conditions by filtering out obsolete versions
    • Preserves test structure when conditions are still relevant

Integration with JUnit 6 Migration

The recipe has been added to the main JUnit 6 migration recipe with Java 17 as the default minimum version, aligning with JUnit 6's requirements.

Files Changed

  • src/main/java/org/openrewrite/java/testing/junit6/MinimumJreConditions.java - Recipe implementation
  • src/main/resources/META-INF/rewrite/junit6.yml - Added recipe to JUnit 6 migration
  • src/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java - Comprehensive test coverage

Example Transformations

Before

class MyTest {
    @Test
    @EnabledOnJre(JRE.JAVA_8)
    void testOnlyOnJava8() {
        // This test only runs on Java 8
    }
    
    @Test
    @EnabledOnJre(versions = { 11, 17, 21 })
    void testOnMultipleVersions() {
        // This test runs on Java 11, 17, and 21
    }
    
    @Test
    @EnabledForJreRange(min = JRE.JAVA_8, max = JRE.JAVA_11)
    void testOnOldVersions() {
        // This test runs on Java 8-11
    }
}

After (with minimum JRE 17)

class MyTest {
    // testOnlyOnJava8 removed - obsolete for Java 17+
    
    @Test
    @EnabledOnJre(versions = {17, 21})
    void testOnMultipleVersions() {
        // Updated to only include relevant versions
    }
    
    // testOnOldVersions removed - range is entirely below minimum
}

Testing

Comprehensive test coverage includes:

  • Parameterized tests for various annotation formats
  • Edge cases for range conditions
  • Multi-version annotation handling
  • Preservation of non-test methods
  • Handling of special JRE values (e.g., JRE.OTHER)

Related Issues

This recipe supports the JUnit 6 migration effort by ensuring test suites are properly cleaned up when moving to newer minimum JRE requirements.

@Jenson3210 Jenson3210 self-assigned this Aug 20, 2025
@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Aug 20, 2025
@Jenson3210 Jenson3210 added recipe Recipe request junit labels Aug 20, 2025
@timtebeek timtebeek self-requested a review August 20, 2025 13:30
@Jenson3210 Jenson3210 mentioned this pull request Aug 20, 2025
10 tasks
private static final String DISABLED_ON_JRE = "org.junit.jupiter.api.condition.DisabledOnJre";
private static final String ENABLED_FOR_JRE_RANGE = "org.junit.jupiter.api.condition.EnabledForJreRange";
private static final String DISABLED_FOR_JRE_RANGE = "org.junit.jupiter.api.condition.DisabledForJreRange";
private static final Annotated.Matcher TEST_ANNOTATION_MATCHER = new Annotated.Matcher("@org.junit.jupiter.api.Test");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's quite a few more test annotations we'd want to support, like @ParameterizedTest, @RepeatedTest and others similarly covered for other recipes. Can we add support here?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those other test annotations can be identified generically by looking for the @TestTemplate meta-annotation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see the meta-annotations in the type hierarchy on our lst's (nor do I know exactly where to look), but added it like other recipes are doing it.

Comment on lines 516 to 534
void handleRangeWithOnlyMin(String range) {
rewriteRun(
java(
"""
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.EnabledForJreRange;
import org.junit.jupiter.api.condition.JRE;

class MyTest {
@Test
@EnabledForJreRange(%s)
void testOnJava11AndLater() {
System.out.println("Java 11+");
}
}
""".formatted(range)
)
);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ideally here we'd want to set a new minimum of 17 going forward right, for open ranges?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ranges and adapting them I had mentioned to do in another iteration of this recipe, but will get on this now that I have addressed the other review comments

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure; let me know when this is ready for another round of review and possible merge , or whether anything is still planned or in progress.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timtebeek
Ranges have been implemented causing a bunch of repeated code lines to be extracted into separate methods with a clear name on what they do.
No more pending items on this one

- Added more Test annotations support
- Unwrap single value arrays
- Do not remove condition annotations
- ...
@openrewrite openrewrite deleted a comment from github-actions bot Aug 22, 2025
@timtebeek timtebeek changed the title Initial version of MinimumJreConditions recipe Adapt @EnabledOnJre, @DisabledOnJre, @EnabledForJreRange and @DisabledForJreRange for JUnit 6's Java 17 minimum Aug 22, 2025
@Jenson3210
Copy link
Contributor Author

This last commit won't work I guess as versions can also be an J.Array of versions.

Copy link
Member

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to see! Looking forward to seeing how this will pan out in practice; did you already run against a larger suite of projects?

@github-project-automation github-project-automation bot moved this from In Progress to Ready to Review in OpenRewrite Aug 22, 2025
@timtebeek
Copy link
Member

This last commit won't work I guess as versions can also be an J.Array of versions.

Ah then best reverted with an appropriate test added to guard against regressions.

@Jenson3210
Copy link
Contributor Author

No,not yet!

Do you mean this one only?
As we will need junit 6 for the composite recipe.

@Jenson3210
Copy link
Contributor Author

I was wrong...
It was the case where @SomeAnnotation({JRE.JAVA12, JRE.JAVA_11}) would be tried to be parsed.
The Annotated Trait has a built in support for arrays of Literals, but not for the defaulted version where the array is not of literals but of FieldAccesses.

I knew there was something to it.

@timtebeek
Copy link
Member

No,not yet! Do you mean this one only?

We can likely eye-ball the diffs this one only would produce on an org like Apache, even locally with the CLI, and from there possibly see possible improvements, or have enough confidence to merge this PR already.

@Jenson3210
Copy link
Contributor Author

What would we want to do in this situation @timtebeek? Can we safely assume the comment on same line will be
Screenshot 2025-08-22 at 22 24 24

Rest is looking good though (ran against Apache ~ 1200 repos, no errors and 20'ish validated results)!
Screenshot 2025-08-22 at 22 27 17
Screenshot 2025-08-22 at 22 27 56

I must say... It keeps feeling weird to remove test cases that verify "reflection-java-8/11/15-only-usecases" in certain java versions for a library just because the test framework runs tests in a higher junit version.

@timtebeek
Copy link
Member

What would we want to do in this situation @timtebeek? Can we safely assume the comment on same line will be

I think it's fine not to handle that for now;if we did we should handle it in the visitor that removes the annotation instead.

Rest is looking good though (ran against Apache ~ 1200 repos, no errors and 20'ish validated results)!

Great! Thanks for looking through those.

I must say... It keeps feeling weird to remove test cases that verify "reflection-java-8/11/15-only-usecases" in certain java versions for a library just because the test framework runs tests in a higher junit version.

I agree it looks odd, but perhaps it helps to look at the deprecated enum values?
https://docs.junit.org/6.0.0-RC1/api/org.junit.jupiter.api/org/junit/jupiter/api/condition/JRE.html
image

So then the way I see it they are there only to avoid compilation issues, but not actually used with the recommendation to use JAVA_17 going forward. Effectively there would be no way to run these tests, so then there would be a false sense of security to have them still; better then I think to make folks aware by removing those tests, as opposed to keeping them around.

@timtebeek timtebeek linked an issue Aug 25, 2025 that may be closed by this pull request
10 tasks
@timtebeek timtebeek merged commit f9e03d5 into main Aug 26, 2025
2 checks passed
@timtebeek timtebeek deleted the junit6 branch August 26, 2025 13:36
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Aug 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
junit recipe Recipe request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Recipes to upgrade to JUnit 6
4 participants