Adapt @EnabledOnJre, @DisabledOnJre, @EnabledForJreRange and @DisabledForJreRange for JUnit 6's Java 17 minimum#795
Conversation
src/main/java/org/openrewrite/java/testing/junit6/MinimumJreConditions.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit6/MinimumJreConditions.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit6/MinimumJreConditions.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit6/MinimumJreConditions.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit6/MinimumJreConditions.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit6/MinimumJreConditions.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit6/MinimumJreConditions.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit6/MinimumJreConditions.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java
Outdated
Show resolved
Hide resolved
| 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"); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Those other test annotations can be identified generically by looking for the @TestTemplate meta-annotation.
There was a problem hiding this comment.
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.
src/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java
Show resolved
Hide resolved
| 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) | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
I think ideally here we'd want to set a new minimum of 17 going forward right, for open ranges?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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
src/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java
Outdated
Show resolved
Hide resolved
- Added more Test annotations support - Unwrap single value arrays - Do not remove condition annotations - ...
@EnabledOnJre, @DisabledOnJre, @EnabledForJreRange and @DisabledForJreRange for JUnit 6's Java 17 minimum
src/main/java/org/openrewrite/java/testing/junit6/MinimumJreConditions.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit6/MinimumJreConditions.java
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit6/MinimumJreConditions.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit6/MinimumJreConditions.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit6/MinimumJreConditions.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit6/MinimumJreConditions.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/testing/junit6/MinimumJreConditions.java
Outdated
Show resolved
Hide resolved
|
This last commit won't work I guess as versions can also be an J.Array of versions. |
timtebeek
left a comment
There was a problem hiding this comment.
Great to see! Looking forward to seeing how this will pan out in practice; did you already run against a larger suite of projects?
Ah then best reverted with an appropriate test added to guard against regressions. |
|
No,not yet! Do you mean this one only? |
|
I was wrong... I knew there was something to it. |
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. |
|
What would we want to do in this situation @timtebeek? Can we safely assume the comment on same line will be Rest is looking good though (ran against Apache ~ 1200 repos, no errors and 20'ish validated results)! 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 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.
Great! Thanks for looking through those.
I agree it looks odd, but perhaps it helps to look at the deprecated enum values? So then the way I see it they are there only to avoid compilation issues, but not actually used with the recommendation to use |
…rom 3.15.0 to 3.16.0 [skip ci] Bumps [org.openrewrite.recipe:rewrite-testing-frameworks](https://github.com/openrewrite/rewrite-testing-frameworks) from 3.15.0 to 3.16.0. Release notes *Sourced from [org.openrewrite.recipe:rewrite-testing-frameworks's releases](https://github.com/openrewrite/rewrite-testing-frameworks/releases).* > 3.16.0 > ------ > > What's Changed > -------------- > > * Use `@ValueSource` instead of `@CsvSource` where there's a single arg by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-testing-frameworks#797](https://github.com/openrewrite/rewrite-testing-frameworks/pull/797) > * Adapt `@EnabledOnJre`, `@DisabledOnJre`, `@EnabledForJreRange` and `@DisabledForJreRange` for JUnit 6's Java 17 minimum by [`@Jenson3210`](https://github.com/Jenson3210) in [openrewrite/rewrite-testing-frameworks#795](https://github.com/openrewrite/rewrite-testing-frameworks/pull/795) > > **Full Changelog**: <openrewrite/rewrite-testing-frameworks@v3.15.1...v3.16.0> > > v3.15.1 > ------- > > What's Changed > -------------- > > * Update with rewrite 8.60.2 > * Add `TestMethodsShouldBeVoid` recipe to fix tests not discovered by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-testing-frameworks#791](https://github.com/openrewrite/rewrite-testing-frameworks/pull/791) > * Recipes to migrate from JUnit 5 to JUnit 6 by [`@Jenson3210`](https://github.com/Jenson3210) in [openrewrite/rewrite-testing-frameworks#792](https://github.com/openrewrite/rewrite-testing-frameworks/pull/792) > * Remove lineSeparator annotation attribute by [`@Jenson3210`](https://github.com/Jenson3210) in [openrewrite/rewrite-testing-frameworks#793](https://github.com/openrewrite/rewrite-testing-frameworks/pull/793) > * Use classpath from resource for Refaster by [`@jevanlingen`](https://github.com/jevanlingen) in [openrewrite/rewrite-testing-frameworks#794](https://github.com/openrewrite/rewrite-testing-frameworks/pull/794) > * Add `JUnitTryFailToAssertThatThrownBy` by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-testing-frameworks#781](https://github.com/openrewrite/rewrite-testing-frameworks/pull/781) > * JUnit5BestPractices - Upgrade mockwebserver3-junit5 to 5.1.0 version by [`@philippe-granet`](https://github.com/philippe-granet) in [openrewrite/rewrite-testing-frameworks#796](https://github.com/openrewrite/rewrite-testing-frameworks/pull/796) > > **Full Changelog**: <openrewrite/rewrite-testing-frameworks@v3.15.0...v3.15.1> Commits * [`d3f66c7`](openrewrite/rewrite-testing-frameworks@d3f66c7) Update documentation examples * [`f9e03d5`](openrewrite/rewrite-testing-frameworks@f9e03d5) Adapt `@EnabledOnJre`, `@DisabledOnJre`, `@EnabledForJreRange` and `@Disabled... * [`2f840cb`](openrewrite/rewrite-testing-frameworks@2f840cb) Use `@ValueSource` instead of `@CsvSource` where there's a single arg ([#797](https://github.com/openrewrite/rewrite-testing-frameworks/issues/797)) * [`46caa79`](openrewrite/rewrite-testing-frameworks@46caa79) JUnit5BestPractices - Upgrade mockwebserver3-junit5 to 5.1.0 version ([#796](https://github.com/openrewrite/rewrite-testing-frameworks/issues/796)) * [`31da639`](openrewrite/rewrite-testing-frameworks@31da639) Add `JUnitTryFailToAssertThatThrownBy` ([#781](https://github.com/openrewrite/rewrite-testing-frameworks/issues/781)) * [`ec45a7a`](openrewrite/rewrite-testing-frameworks@ec45a7a) Use `FindDependency` from rewrite-java-dependencies in `AnyToNullable` * [`fec110e`](openrewrite/rewrite-testing-frameworks@fec110e) Use classpath from resource for Refaster ([#794](https://github.com/openrewrite/rewrite-testing-frameworks/issues/794)) * [`b5bc87a`](openrewrite/rewrite-testing-frameworks@b5bc87a) Remove lineSeparator annotation attribute ([#793](https://github.com/openrewrite/rewrite-testing-frameworks/issues/793)) * [`a9f1463`](openrewrite/rewrite-testing-frameworks@a9f1463) Recipes to migrate from JUnit 5 to JUnit 6 ([#792](https://github.com/openrewrite/rewrite-testing-frameworks/issues/792)) * [`bf91b26`](openrewrite/rewrite-testing-frameworks@bf91b26) Add `TestMethodsShouldBeVoid` recipe to fix tests not discovered ([#791](https://github.com/openrewrite/rewrite-testing-frameworks/issues/791)) * See full diff in [compare view](openrewrite/rewrite-testing-frameworks@v3.15.0...v3.16.0) [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)




Add recipe to simplify JRE version conditions for JUnit 6 migration
Summary
This PR introduces a new recipe
MinimumJreConditionsthat 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:
MinimumJreConditionsThe recipe processes JUnit Jupiter's JRE conditional annotations (
@EnabledOnJre,@DisabledOnJre,@EnabledForJreRange,@DisabledForJreRange) and:Key Features
Supports multiple annotation formats:
JRE.JAVA_8,JRE.JAVA_17, etc. (also the statically imported ones)value = JRE.JAVA_17versions = 17orversions = { 17, 21 }minVersion = 17, maxVersion = 21Handles range conditions:
Smart annotation cleanup:
@EnabledOnJre(JRE.JAVA_17)when 17 is the minimum)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 implementationsrc/main/resources/META-INF/rewrite/junit6.yml- Added recipe to JUnit 6 migrationsrc/test/java/org/openrewrite/java/testing/junit6/MinimumJreConditionsTest.java- Comprehensive test coverageExample Transformations
Before
After (with minimum JRE 17)
Testing
Comprehensive test coverage includes:
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.