From 3c56cf71b27469ebddce7772e38f59509a115852 Mon Sep 17 00:00:00 2001 From: Steve Elliott Date: Mon, 7 Jul 2025 17:27:05 -0400 Subject: [PATCH 01/15] Adding dependency guards to the Junit 4 -> 5 migration for when the POM or build.gradle have a dependency on `org.testng:testng`. Relies on https://github.com/openrewrite/rewrite/pull/5725 --- .../resources/META-INF/rewrite/junit5.yml | 6 ++ .../testing/junit5/JUnit5MigrationTest.java | 66 ++++++++++++++++++- 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/main/resources/META-INF/rewrite/junit5.yml b/src/main/resources/META-INF/rewrite/junit5.yml index bcdff33ed..718ae6f33 100755 --- a/src/main/resources/META-INF/rewrite/junit5.yml +++ b/src/main/resources/META-INF/rewrite/junit5.yml @@ -61,6 +61,12 @@ preconditions: - org.openrewrite.java.search.DoesNotUseType: fullyQualifiedTypeName: org.testng..* includeImplicit: true + - org.openrewrite.gradle.search.DoesNotIncludeDependency: + groupId: org.testng + artifactId: testng + - org.openrewrite.maven.search.DoesNotIncludeDependency: + groupId: org.testng + artifactId: testng recipeList: - org.openrewrite.java.testing.junit5.EnvironmentVariables - org.openrewrite.java.testing.junit5.UseWiremockExtension diff --git a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java index 9153e2709..14f83bdf0 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java @@ -40,7 +40,7 @@ class JUnit5MigrationTest implements RewriteTest { public void defaults(RecipeSpec spec) { spec .parser(JavaParser.fromJavaVersion() - .classpathFromResources(new InMemoryExecutionContext(), "junit-4")) + .classpathFromResources(new InMemoryExecutionContext(), "junit-4", "testng")) .recipe(Environment.builder() .scanRuntimeClasspath("org.openrewrite.java.testing.junit5") .build() @@ -51,11 +51,11 @@ public void defaults(RecipeSpec spec) { @Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/145") @Test void assertThatReceiver() { - //language=java rewriteRun( spec -> spec .parser(JavaParser.fromJavaVersion() .classpathFromResources(new InMemoryExecutionContext(), "junit-4", "hamcrest-3")), + //language=java java( """ import org.junit.Assert; @@ -477,6 +477,68 @@ void noJunitDependencyIfApiAlreadyPresent() { ); } + @Test + void noChangesIfTestNgIncluded() { + rewriteRun( + spec -> spec.beforeRecipe(withToolingApi()), + //language=groovy + buildGradle( + """ + plugins { + id 'java-library' + } + repositories { + mavenCentral() + } + dependencies { + testImplementation 'junit:junit:4.12' + testImplementation 'org.testng:testng:7.8.0' + } + tasks.withType(Test).configureEach { + useJUnitPlatform() + } + """ + ), + //language=xml + pomXml( + """ + + 4.0.0 + dev.ted + testcontainer-migrate + 0.0.1 + + + junit + junit + 4.12 + test + + + org.testng + testng + 7.8.0 + + + + """ + ), + //language=java + java( + """ + import org.junit.Ignore; + import org.testng.annotations.Test; + + class ExampleClass { + @Ignore + @Test + public void testMethod() {} + } + """ + ) + ); + } + @Test void bumpSurefireOnOlderMavenVersions() { rewriteRun( From f800288660c70adac48c548246e94dc79fc6259b Mon Sep 17 00:00:00 2001 From: Steve Elliott Date: Wed, 16 Jul 2025 11:30:12 -0400 Subject: [PATCH 02/15] Updating `DoesNotIncludeDependency` recipe call to be the shared one in `rewrite-java-dependencies` --- src/main/resources/META-INF/rewrite/junit5.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/main/resources/META-INF/rewrite/junit5.yml b/src/main/resources/META-INF/rewrite/junit5.yml index 718ae6f33..de1da2dd9 100755 --- a/src/main/resources/META-INF/rewrite/junit5.yml +++ b/src/main/resources/META-INF/rewrite/junit5.yml @@ -61,10 +61,7 @@ preconditions: - org.openrewrite.java.search.DoesNotUseType: fullyQualifiedTypeName: org.testng..* includeImplicit: true - - org.openrewrite.gradle.search.DoesNotIncludeDependency: - groupId: org.testng - artifactId: testng - - org.openrewrite.maven.search.DoesNotIncludeDependency: + - org.openrewrite.java.dependencies.search.DoesNotIncludeDependency: groupId: org.testng artifactId: testng recipeList: From a059fa828cbd3470ecfe923b465d3c1adcde2444 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 25 Jul 2025 16:07:11 +0200 Subject: [PATCH 03/15] Avoid tests passing for the wrong reason (class present) --- .../testing/junit5/JUnit5MigrationTest.java | 170 +++++++++++------- 1 file changed, 108 insertions(+), 62 deletions(-) diff --git a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java index 14f83bdf0..0f202cfd9 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java @@ -15,6 +15,7 @@ */ package org.openrewrite.java.testing.junit5; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.InMemoryExecutionContext; @@ -477,68 +478,6 @@ void noJunitDependencyIfApiAlreadyPresent() { ); } - @Test - void noChangesIfTestNgIncluded() { - rewriteRun( - spec -> spec.beforeRecipe(withToolingApi()), - //language=groovy - buildGradle( - """ - plugins { - id 'java-library' - } - repositories { - mavenCentral() - } - dependencies { - testImplementation 'junit:junit:4.12' - testImplementation 'org.testng:testng:7.8.0' - } - tasks.withType(Test).configureEach { - useJUnitPlatform() - } - """ - ), - //language=xml - pomXml( - """ - - 4.0.0 - dev.ted - testcontainer-migrate - 0.0.1 - - - junit - junit - 4.12 - test - - - org.testng - testng - 7.8.0 - - - - """ - ), - //language=java - java( - """ - import org.junit.Ignore; - import org.testng.annotations.Test; - - class ExampleClass { - @Ignore - @Test - public void testMethod() {} - } - """ - ) - ); - } - @Test void bumpSurefireOnOlderMavenVersions() { rewriteRun( @@ -673,4 +612,111 @@ public class MyClassTest { ) ); } + + @Nested + class TestngExclude { + @Test + void noChangesIfTestNgGradleDependencyIncluded() { + rewriteRun( + spec -> spec.beforeRecipe(withToolingApi()), + mavenProject("project", + //language=groovy + buildGradle( + """ + plugins { + id 'java-library' + } + repositories { + mavenCentral() + } + dependencies { + testImplementation 'junit:junit:4.12' + testImplementation 'org.testng:testng:7.8.0' + } + tasks.withType(Test).configureEach { + useJUnitPlatform() + } + """ + ), + srcTestJava( + //language=java + java( + """ + import org.junit.Ignore; + + class ExampleClass { + @Ignore + public void testMethod() {} + } + """ + ) + ) + ) + ); + } + + @Test + void noChangesIfTestNgMavenDependencyIncluded() { + rewriteRun( + mavenProject("project", + //language=xml + pomXml( + """ + + 4.0.0 + dev.ted + testcontainer-migrate + 0.0.1 + + + junit + junit + 4.12 + test + + + org.testng + testng + 7.8.0 + + + + """ + ), + srcTestJava( + //language=java + java( + """ + import org.junit.Ignore; + + class ExampleClass { + @Ignore + public void testMethod() {} + } + """ + ) + ) + ) + ); + } + + @Test + void noChangesIfTestNgCassIncluded() { + rewriteRun( + //language=java + java( + """ + import org.junit.Ignore; + import org.testng.annotations.Test; + + class ExampleClass { + @Ignore + @Test + public void testMethod() {} + } + """ + ) + ); + } + } } From c307680faf3692800b2688fd2f32623acaa38c9b Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 25 Jul 2025 16:08:20 +0200 Subject: [PATCH 04/15] Only pass testng dependency into the single test that needs it --- .../openrewrite/java/testing/junit5/JUnit5MigrationTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java index 0f202cfd9..58114aeb5 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java @@ -41,7 +41,7 @@ class JUnit5MigrationTest implements RewriteTest { public void defaults(RecipeSpec spec) { spec .parser(JavaParser.fromJavaVersion() - .classpathFromResources(new InMemoryExecutionContext(), "junit-4", "testng")) + .classpathFromResources(new InMemoryExecutionContext(), "junit-4")) .recipe(Environment.builder() .scanRuntimeClasspath("org.openrewrite.java.testing.junit5") .build() @@ -703,6 +703,9 @@ public void testMethod() {} @Test void noChangesIfTestNgCassIncluded() { rewriteRun( + spec -> spec + .parser(JavaParser.fromJavaVersion() + .classpathFromResources(new InMemoryExecutionContext(), "junit-4", "testng")), //language=java java( """ From 57ad02e24c190e29df1785d1570cc5f3a633cebf Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 25 Jul 2025 16:12:21 +0200 Subject: [PATCH 05/15] Match any testng dependency --- src/main/resources/META-INF/rewrite/junit5.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/META-INF/rewrite/junit5.yml b/src/main/resources/META-INF/rewrite/junit5.yml index de1da2dd9..d4e934add 100755 --- a/src/main/resources/META-INF/rewrite/junit5.yml +++ b/src/main/resources/META-INF/rewrite/junit5.yml @@ -63,7 +63,7 @@ preconditions: includeImplicit: true - org.openrewrite.java.dependencies.search.DoesNotIncludeDependency: groupId: org.testng - artifactId: testng + artifactId: testng* recipeList: - org.openrewrite.java.testing.junit5.EnvironmentVariables - org.openrewrite.java.testing.junit5.UseWiremockExtension From 199cc3eb0fefdb5f77ec7ac94835b6750c12562d Mon Sep 17 00:00:00 2001 From: Steve Elliott Date: Mon, 28 Jul 2025 17:46:16 -0400 Subject: [PATCH 06/15] Adding TestNgGuard scanning recipe. Needs refinement, but this is a first pass --- .../java/testing/junit5/TestNgGuard.java | 156 ++++ .../resources/META-INF/rewrite/junit5.yml | 7 +- .../testing/junit5/JUnit5MigrationTest.java | 781 ++++++++++-------- 3 files changed, 577 insertions(+), 367 deletions(-) create mode 100644 src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java diff --git a/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java b/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java new file mode 100644 index 000000000..4950b6a2f --- /dev/null +++ b/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java @@ -0,0 +1,156 @@ +package org.openrewrite.java.testing.junit5; + +import lombok.Value; +import org.jspecify.annotations.Nullable; +import org.openrewrite.*; +import org.openrewrite.java.dependencies.search.DoesNotIncludeDependency; +import org.openrewrite.java.marker.JavaProject; +import org.openrewrite.java.search.DoesNotUseType; +import org.openrewrite.java.search.UsesType; +import org.openrewrite.marker.SearchResult; + +import java.util.HashSet; +import java.util.Optional; +import java.util.Set; + +public class TestNgGuard extends ScanningRecipe { + @Override + public String getDisplayName() { + return "JUnit Jupiter migration from JUnit 4.x"; + } + + @Override + public String getDescription() { + return "Migrates JUnit 4.x tests to JUnit Jupiter."; + } + + @Value + public static class Accumulator { + Set projectsWithoutTestNgDependency; + Set projectsWithoutTestNgTypeUsage; + Set projectsWithTestNgDependency; + Set projectsWithTestNgTypeUsage; + } + + @Override + public Accumulator getInitialValue(ExecutionContext ctx) { + return new Accumulator(new HashSet<>(), new HashSet<>(), new HashSet<>(), new HashSet<>()); + } + + @Override + public TreeVisitor getScanner(Accumulator acc) { + return new TreeVisitor() { + private final TreeVisitor dnut = new DoesNotUseType("org.testng..*", true).getVisitor(); + private final TreeVisitor ut = new UsesType<>("org.testng..*", true); + private final TreeVisitor dnid = new DoesNotIncludeDependency("org.testng", "testng*", null, null, null).getVisitor(); + + @Override + public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { + assert tree != null; + if (!(tree instanceof SourceFile)) { + return tree; + } + SourceFile s = (SourceFile) tree; + if (dnid.isAcceptable(s, ctx)) { + Tree after = dnid.visit(tree, ctx); + if (after != tree) { + tree + .getMarkers() + .findFirst(JavaProject.class) + .ifPresent(acc.projectsWithoutTestNgDependency::add); + } else { + tree + .getMarkers() + .findFirst(JavaProject.class) + .ifPresent(acc.projectsWithTestNgDependency::add); + } + } else if (ut.isAcceptable(s, ctx)) { + Tree after = ut.visit(tree, ctx); + if (after == tree) { + tree + .getMarkers() + .findFirst(JavaProject.class) + .ifPresent(acc.projectsWithoutTestNgTypeUsage::add); + } else { + tree + .getMarkers() + .findFirst(JavaProject.class) + .ifPresent(acc.projectsWithTestNgTypeUsage::add); + } + } + return tree; + } + }; + } + + @Override + public TreeVisitor getVisitor(Accumulator acc) { + return new TreeVisitor() { + private final TreeVisitor ut = new UsesType<>("org.testng..*", true); + private final TreeVisitor dnid = new DoesNotIncludeDependency("org.testng", "testng*", null, null, null).getVisitor(); + + @Override + public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) { + return ut.isAcceptable(sourceFile, ctx) || dnid.isAcceptable(sourceFile, ctx); + } + + @Override + public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { + assert tree != null; + Optional maybeJp = tree.getMarkers().findFirst(JavaProject.class); + if (!maybeJp.isPresent()) { + return tree; + } + JavaProject jp = maybeJp.get(); + boolean pwoTngd = acc.getProjectsWithoutTestNgDependency().contains(jp); + boolean pwoTngtu = acc.getProjectsWithoutTestNgTypeUsage().contains(jp); + boolean pwTngd = acc.getProjectsWithTestNgDependency().contains(jp); + boolean pwTngtu = acc.getProjectsWithTestNgTypeUsage().contains(jp); + if (pwTngtu || pwTngd) { + return tree; + } + if (!pwoTngd && !pwoTngtu) { + return tree; + } + if ( + (pwoTngd || acc.getProjectsWithoutTestNgDependency().isEmpty()) + && (pwoTngtu || acc.getProjectsWithoutTestNgTypeUsage().isEmpty()) + ) { + return SearchResult.found(tree); + } + return tree; + } + }; + } + + // @Override +// public TreeVisitor getVisitor() { +// return new TreeVisitor() { +// private final TreeVisitor dnut = new DoesNotUseType("org.testng..*", true).getVisitor(); +// private final TreeVisitor dnid = new DoesNotIncludeDependency("org.testng", "testng*", null, null, null).getVisitor(); +// +// @Override +// public boolean isAcceptable(SourceFile sourceFile, ExecutionContext executionContext) { +// return dnut.isAcceptable(sourceFile, executionContext) || dnid.isAcceptable(sourceFile, executionContext); +// } +// +// @Override +// public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) { +// if (!(tree instanceof SourceFile)) { +// return tree; +// } +// SourceFile s = (SourceFile) tree; +// SourceFile after = s; +// if (dnid.isAcceptable(s, ctx)) { +// after = (SourceFile) dnid.visitNonNull(s, ctx); +// } else if (dnut.isAcceptable(s, ctx)) { +// after = (SourceFile) dnut.visitNonNull(s, ctx); +// } +// if (after == s) { +// return s; +// } +// return SearchResult.found(after); +// } +// }; +// } +} diff --git a/src/main/resources/META-INF/rewrite/junit5.yml b/src/main/resources/META-INF/rewrite/junit5.yml index d4e934add..e8f35dade 100755 --- a/src/main/resources/META-INF/rewrite/junit5.yml +++ b/src/main/resources/META-INF/rewrite/junit5.yml @@ -58,12 +58,7 @@ tags: - testing - junit preconditions: - - org.openrewrite.java.search.DoesNotUseType: - fullyQualifiedTypeName: org.testng..* - includeImplicit: true - - org.openrewrite.java.dependencies.search.DoesNotIncludeDependency: - groupId: org.testng - artifactId: testng* + - org.openrewrite.java.testing.junit5.TestNgGuard recipeList: - org.openrewrite.java.testing.junit5.EnvironmentVariables - org.openrewrite.java.testing.junit5.UseWiremockExtension diff --git a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java index 58114aeb5..acd2beab7 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java @@ -56,40 +56,42 @@ void assertThatReceiver() { spec -> spec .parser(JavaParser.fromJavaVersion() .classpathFromResources(new InMemoryExecutionContext(), "junit-4", "hamcrest-3")), - //language=java - java( - """ - import org.junit.Assert; - import org.junit.Test; - - import static java.util.Arrays.asList; - import static org.hamcrest.Matchers.containsInAnyOrder; - - public class SampleTest { - @SuppressWarnings("ALL") - @Test - public void filterShouldRemoveUnusedConfig() { - Assert.assertThat(asList("1", "2", "3"), - containsInAnyOrder("3", "2", "1")); - } - } - """, - """ - import org.junit.jupiter.api.Test; - - import static java.util.Arrays.asList; - import static org.hamcrest.MatcherAssert.assertThat; - import static org.hamcrest.Matchers.containsInAnyOrder; - - public class SampleTest { - @SuppressWarnings("ALL") - @Test - public void filterShouldRemoveUnusedConfig() { - assertThat(asList("1", "2", "3"), - containsInAnyOrder("3", "2", "1")); - } - } + mavenProject("project", + //language=java + java( """ + import org.junit.Assert; + import org.junit.Test; + + import static java.util.Arrays.asList; + import static org.hamcrest.Matchers.containsInAnyOrder; + + public class SampleTest { + @SuppressWarnings("ALL") + @Test + public void filterShouldRemoveUnusedConfig() { + Assert.assertThat(asList("1", "2", "3"), + containsInAnyOrder("3", "2", "1")); + } + } + """, + """ + import org.junit.jupiter.api.Test; + + import static java.util.Arrays.asList; + import static org.hamcrest.MatcherAssert.assertThat; + import static org.hamcrest.Matchers.containsInAnyOrder; + + public class SampleTest { + @SuppressWarnings("ALL") + @Test + public void filterShouldRemoveUnusedConfig() { + assertThat(asList("1", "2", "3"), + containsInAnyOrder("3", "2", "1")); + } + } + """ + ) ) ); } @@ -98,26 +100,28 @@ public void filterShouldRemoveUnusedConfig() { @Test void classReference() { rewriteRun( - //language=java - java( - """ - import org.junit.Test; - - public class Sample { - void method() { - Class c = Test.class; - } - } - """, - """ - import org.junit.jupiter.api.Test; - - public class Sample { - void method() { - Class c = Test.class; - } - } + mavenProject("project", + //language=java + java( + """ + import org.junit.Test; + + public class Sample { + void method() { + Class c = Test.class; + } + } + """, """ + import org.junit.jupiter.api.Test; + + public class Sample { + void method() { + Class c = Test.class; + } + } + """ + ) ) ); } @@ -126,34 +130,36 @@ void method() { @Test void upgradeMavenPluginVersions() { rewriteRun( - pomXml( - //language=xml - """ - + mavenProject("project", + pomXml( + //language=xml + """ + 4.0.0 com.example.jackson test-plugins 1.0.0 - - - org.apache.maven.plugins - maven-surefire-plugin - 2.20.1 - - - org.apache.maven.plugins - maven-failsafe-plugin - 2.20.1 - - + + + org.apache.maven.plugins + maven-surefire-plugin + 2.20.1 + + + org.apache.maven.plugins + maven-failsafe-plugin + 2.20.1 + + - - """, - spec -> spec.after(actual -> { - assertThat(Pattern.compile("3\\.(.*)").matcher(actual).results().toList()).hasSize(2); - return actual; - }) + + """, + spec -> spec.after(actual -> { + assertThat(Pattern.compile("3\\.(.*)").matcher(actual).results().toList()).hasSize(2); + return actual; + }) + ) ) ); } @@ -164,65 +170,67 @@ void excludeJunit4Dependency() { // In practice, this would probably just break it, I assume. //language=xml rewriteRun( - pomXml( - """ - - 4.0.0 - - org.springframework.boot - spring-boot-starter-parent - 3.2.1 - - - dev.ted - needs-exclusion - 0.0.1 - - - org.springframework.boot - spring-boot-starter - - - com.typesafe.play - play-test_2.13 - 2.9.6 - test - - - - """, - """ - - 4.0.0 - - org.springframework.boot - spring-boot-starter-parent - 3.2.1 - - - dev.ted - needs-exclusion - 0.0.1 - - - org.springframework.boot - spring-boot-starter - - - com.typesafe.play - play-test_2.13 - 2.9.6 - test - - - junit - junit - - - - - + mavenProject("project", + pomXml( """ + + 4.0.0 + + org.springframework.boot + spring-boot-starter-parent + 3.2.1 + + + dev.ted + needs-exclusion + 0.0.1 + + + org.springframework.boot + spring-boot-starter + + + com.typesafe.play + play-test_2.13 + 2.9.6 + test + + + + """, + """ + + 4.0.0 + + org.springframework.boot + spring-boot-starter-parent + 3.2.1 + + + dev.ted + needs-exclusion + 0.0.1 + + + org.springframework.boot + spring-boot-starter + + + com.typesafe.play + play-test_2.13 + 2.9.6 + test + + + junit + junit + + + + + + """ + ) ) ); } @@ -232,23 +240,25 @@ void excludeJunit4Dependency() { void dontExcludeJunit4DependencyFromTestcontainers() { //language=xml rewriteRun( - pomXml( - """ - - 4.0.0 - com.example.jackson - test-plugins - 1.0.0 - - - org.testcontainers - testcontainers - 1.18.3 - test - - - + mavenProject("project", + pomXml( """ + + 4.0.0 + com.example.jackson + test-plugins + 1.0.0 + + + org.testcontainers + testcontainers + 1.18.3 + test + + + + """ + ) ) ); } @@ -258,23 +268,25 @@ void dontExcludeJunit4DependencyFromTestcontainers() { void dontExcludeJunit4DependencyFromTestcontainersJupiter() { //language=xml rewriteRun( - pomXml( - """ - - 4.0.0 - com.example.jackson - test-plugins - 1.0.0 - - - org.testcontainers - junit-jupiter - 1.18.3 - test - - - + mavenProject("project", + pomXml( """ + + 4.0.0 + com.example.jackson + test-plugins + 1.0.0 + + + org.testcontainers + junit-jupiter + 1.18.3 + test + + + + """ + ) ) ); } @@ -283,43 +295,45 @@ void dontExcludeJunit4DependencyFromTestcontainersJupiter() { @Test void dontExcludeJunit4DependencyFromSpringBootTestcontainers() { rewriteRun( - //language=xml - pomXml( - """ - - 4.0.0 - - org.springframework.boot - spring-boot-starter-parent - 3.2.1 - - - dev.ted - testcontainer-migrate - 0.0.1 - - - org.springframework.boot - spring-boot-starter - - - org.springframework.boot - spring-boot-starter-test - test - - - org.springframework.boot - spring-boot-testcontainers - test - - - org.testcontainers - junit-jupiter - test - - - + mavenProject("project", + //language=xml + pomXml( """ + + 4.0.0 + + org.springframework.boot + spring-boot-starter-parent + 3.2.1 + + + dev.ted + testcontainer-migrate + 0.0.1 + + + org.springframework.boot + spring-boot-starter + + + org.springframework.boot + spring-boot-starter-test + test + + + org.springframework.boot + spring-boot-testcontainers + test + + + org.testcontainers + junit-jupiter + test + + + + """ + ) ) ); } @@ -330,26 +344,28 @@ void dontExcludeJunit4DependencyFromSpringBootTestcontainers() { @Test void assertEqualsWithArrayArgumentToAssertArrayEquals() { rewriteRun( - //language=java - java( - """ - import org.junit.Assert; - - class MyTest { - void test() { - Assert.assertEquals(new Object[1], new Object[1]); - } - } - """, - """ - import org.junit.jupiter.api.Assertions; - - class MyTest { - void test() { - Assertions.assertArrayEquals(new Object[1], new Object[1]); - } - } + mavenProject("project", + //language=java + java( + """ + import org.junit.Assert; + + class MyTest { + void test() { + Assert.assertEquals(new Object[1], new Object[1]); + } + } + """, """ + import org.junit.jupiter.api.Assertions; + + class MyTest { + void test() { + Assertions.assertArrayEquals(new Object[1], new Object[1]); + } + } + """ + ) ) ); } @@ -359,78 +375,82 @@ void test() { void migrateInheritedTestBeforeAfterAnnotations() { //language=java rewriteRun( - java( - """ - import org.junit.After; - import org.junit.Before; - import org.junit.Test; - - public class AbstractTest { - @Before - public void before() { - } - - @After - public void after() { - } + mavenProject("project", + java( + """ + import org.junit.After; + import org.junit.Before; + import org.junit.Test; - @Test - public void test() { - } - } - """, - """ - import org.junit.jupiter.api.AfterEach; - import org.junit.jupiter.api.BeforeEach; - import org.junit.jupiter.api.Test; - - public class AbstractTest { - @BeforeEach - public void before() { - } + public class AbstractTest { + @Before + public void before() { + } - @AfterEach - public void after() { - } + @After + public void after() { + } - @Test - public void test() { - } - } + @Test + public void test() { + } + } + """, """ - ), - java( - """ - public class A extends AbstractTest { - public void before() { - } + import org.junit.jupiter.api.AfterEach; + import org.junit.jupiter.api.BeforeEach; + import org.junit.jupiter.api.Test; - public void after() { - } + public class AbstractTest { + @BeforeEach + public void before() { + } - public void test() { - } - } - """, - """ - import org.junit.jupiter.api.AfterEach; - import org.junit.jupiter.api.BeforeEach; - import org.junit.jupiter.api.Test; - - public class A extends AbstractTest { - @BeforeEach - public void before() { - } + @AfterEach + public void after() { + } - @AfterEach - public void after() { - } + @Test + public void test() { + } + } + """ + ) + ), + mavenProject("project", + java( + """ + public class A extends AbstractTest { + public void before() { + } - @Test - public void test() { - } - } + public void after() { + } + + public void test() { + } + } + """, """ + import org.junit.jupiter.api.AfterEach; + import org.junit.jupiter.api.BeforeEach; + import org.junit.jupiter.api.Test; + + public class A extends AbstractTest { + @BeforeEach + public void before() { + } + + @AfterEach + public void after() { + } + + @Test + public void test() { + } + } + """ + ) ) ); } @@ -439,41 +459,45 @@ public void test() { void noJunitDependencyIfApiAlreadyPresent() { rewriteRun( spec -> spec.beforeRecipe(withToolingApi()), - //language=groovy - buildGradle( - """ - plugins { - id 'java-library' - } - repositories { - mavenCentral() - } - dependencies { - testImplementation 'org.junit.jupiter:junit-jupiter-api:5.7.2' - } - tasks.withType(Test).configureEach { - useJUnitPlatform() - } + mavenProject("project", + //language=groovy + buildGradle( """ + plugins { + id 'java-library' + } + repositories { + mavenCentral() + } + dependencies { + testImplementation 'org.junit.jupiter:junit-jupiter-api:5.7.2' + } + tasks.withType(Test).configureEach { + useJUnitPlatform() + } + """ + ) ), - //language=xml - pomXml( - """ - - 4.0.0 - dev.ted - testcontainer-migrate - 0.0.1 - - - org.junit.jupiter - junit-jupiter-api - 5.7.2 - test - - - + mavenProject("project", + //language=xml + pomXml( """ + + 4.0.0 + dev.ted + testcontainer-migrate + 0.0.1 + + + org.junit.jupiter + junit-jupiter-api + 5.7.2 + test + + + + """ + ) ) ); } @@ -482,42 +506,44 @@ void noJunitDependencyIfApiAlreadyPresent() { void bumpSurefireOnOlderMavenVersions() { rewriteRun( spec -> spec.recipeFromResource("/META-INF/rewrite/junit5.yml", "org.openrewrite.java.testing.junit5.UpgradeSurefirePlugin"), - pomXml( - //language=xml - """ - - 4.0.0 - dev.ted - testcontainer-migrate - 0.0.1 - - """, - //language=xml - """ - - 4.0.0 - dev.ted - testcontainer-migrate - 0.0.1 - - - - org.apache.maven.plugins - maven-surefire-plugin - 3.2.5 - - - org.junit.platform - junit-platform-surefire-provider - 1.1.0 - - - - - - - """, - spec -> spec.markers(new BuildTool(Tree.randomId(), BuildTool.Type.Maven, "3.5.4")) + mavenProject("project", + pomXml( + //language=xml + """ + + 4.0.0 + dev.ted + testcontainer-migrate + 0.0.1 + + """, + //language=xml + """ + + 4.0.0 + dev.ted + testcontainer-migrate + 0.0.1 + + + + org.apache.maven.plugins + maven-surefire-plugin + 3.2.5 + + + org.junit.platform + junit-platform-surefire-provider + 1.1.0 + + + + + + + """, + spec -> spec.markers(new BuildTool(Tree.randomId(), BuildTool.Type.Maven, "3.5.4")) + ) ) ); } @@ -655,6 +681,37 @@ public void testMethod() {} ); } +// @Test +// void blah() { +// rewriteRun( +// spec -> spec.recipe(new DoesNotUseType("org.testng..*", true)), +// //language=xml +// pomXml( +// """ +// +// 4.0.0 +// dev.ted +// testcontainer-migrate +// 0.0.1 +// +// +// junit +// junit +// 4.12 +// test +// +// +// org.testng +// testng +// 7.8.0 +// +// +// +// """ +// ) +// ); +// } + @Test void noChangesIfTestNgMavenDependencyIncluded() { rewriteRun( @@ -706,18 +763,20 @@ void noChangesIfTestNgCassIncluded() { spec -> spec .parser(JavaParser.fromJavaVersion() .classpathFromResources(new InMemoryExecutionContext(), "junit-4", "testng")), - //language=java - java( - """ - import org.junit.Ignore; - import org.testng.annotations.Test; - - class ExampleClass { - @Ignore - @Test - public void testMethod() {} - } + mavenProject("project", + //language=java + java( """ + import org.junit.Ignore; + import org.testng.annotations.Test; + + class ExampleClass { + @Ignore + @Test + public void testMethod() {} + } + """ + ) ) ); } From 172b6f0f4ce6285a0b8ebc64c852638ed06c3080 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 28 Jul 2025 23:53:34 +0200 Subject: [PATCH 07/15] Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../java/testing/junit5/TestNgGuard.java | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java b/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java index 4950b6a2f..23e8e545b 100644 --- a/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java +++ b/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java @@ -1,3 +1,18 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.openrewrite.java.testing.junit5; import lombok.Value; @@ -113,8 +128,8 @@ public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { return tree; } if ( - (pwoTngd || acc.getProjectsWithoutTestNgDependency().isEmpty()) - && (pwoTngtu || acc.getProjectsWithoutTestNgTypeUsage().isEmpty()) + (pwoTngd || acc.getProjectsWithoutTestNgDependency().isEmpty()) && + (pwoTngtu || acc.getProjectsWithoutTestNgTypeUsage().isEmpty()) ) { return SearchResult.found(tree); } From 6d580da8d43655e5db34073ea26401ab99b3168a Mon Sep 17 00:00:00 2001 From: Steve Elliott Date: Tue, 29 Jul 2025 15:48:24 -0400 Subject: [PATCH 08/15] Making separate `TestNgGuard` tests and cleaning up other code --- .../java/testing/junit5/TestNgGuard.java | 80 ++--- .../testing/junit5/JUnit5MigrationTest.java | 42 +-- .../java/testing/junit5/TestNgGuardTest.java | 274 ++++++++++++++++++ 3 files changed, 299 insertions(+), 97 deletions(-) create mode 100644 src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java diff --git a/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java b/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java index 23e8e545b..308024ca0 100644 --- a/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java +++ b/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java @@ -20,7 +20,6 @@ import org.openrewrite.*; import org.openrewrite.java.dependencies.search.DoesNotIncludeDependency; import org.openrewrite.java.marker.JavaProject; -import org.openrewrite.java.search.DoesNotUseType; import org.openrewrite.java.search.UsesType; import org.openrewrite.marker.SearchResult; @@ -31,12 +30,13 @@ public class TestNgGuard extends ScanningRecipe { @Override public String getDisplayName() { - return "JUnit Jupiter migration from JUnit 4.x"; + return "Find `TestNG`-free Maven / Gradle and Java files"; } @Override public String getDescription() { - return "Migrates JUnit 4.x tests to JUnit Jupiter."; + return "Meant to be used as a precondition, it will return results for Maven / Gradle and Java files " + + "that are part of a project that does not have TestNG dependencies nor usage of TestNG classes."; } @Value @@ -55,7 +55,6 @@ public Accumulator getInitialValue(ExecutionContext ctx) { @Override public TreeVisitor getScanner(Accumulator acc) { return new TreeVisitor() { - private final TreeVisitor dnut = new DoesNotUseType("org.testng..*", true).getVisitor(); private final TreeVisitor ut = new UsesType<>("org.testng..*", true); private final TreeVisitor dnid = new DoesNotIncludeDependency("org.testng", "testng*", null, null, null).getVisitor(); @@ -69,28 +68,17 @@ public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { if (dnid.isAcceptable(s, ctx)) { Tree after = dnid.visit(tree, ctx); if (after != tree) { - tree - .getMarkers() - .findFirst(JavaProject.class) - .ifPresent(acc.projectsWithoutTestNgDependency::add); + tree.getMarkers().findFirst(JavaProject.class).ifPresent(acc.projectsWithoutTestNgDependency::add); } else { - tree - .getMarkers() - .findFirst(JavaProject.class) - .ifPresent(acc.projectsWithTestNgDependency::add); + tree.getMarkers().findFirst(JavaProject.class).ifPresent(acc.projectsWithTestNgDependency::add); } } else if (ut.isAcceptable(s, ctx)) { Tree after = ut.visit(tree, ctx); + // Note: Uses `UsesType` for inverted checking because `DoesNotUseType` will mark non Java-code files as well if (after == tree) { - tree - .getMarkers() - .findFirst(JavaProject.class) - .ifPresent(acc.projectsWithoutTestNgTypeUsage::add); + tree.getMarkers().findFirst(JavaProject.class).ifPresent(acc.projectsWithoutTestNgTypeUsage::add); } else { - tree - .getMarkers() - .findFirst(JavaProject.class) - .ifPresent(acc.projectsWithTestNgTypeUsage::add); + tree.getMarkers().findFirst(JavaProject.class).ifPresent(acc.projectsWithTestNgTypeUsage::add); } } return tree; @@ -117,55 +105,25 @@ public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { return tree; } JavaProject jp = maybeJp.get(); - boolean pwoTngd = acc.getProjectsWithoutTestNgDependency().contains(jp); - boolean pwoTngtu = acc.getProjectsWithoutTestNgTypeUsage().contains(jp); - boolean pwTngd = acc.getProjectsWithTestNgDependency().contains(jp); - boolean pwTngtu = acc.getProjectsWithTestNgTypeUsage().contains(jp); - if (pwTngtu || pwTngd) { + boolean noTestNgDep = acc.getProjectsWithoutTestNgDependency().contains(jp); + boolean noTestNgUsage = acc.getProjectsWithoutTestNgTypeUsage().contains(jp); + boolean testNgDep = acc.getProjectsWithTestNgDependency().contains(jp); + boolean testNgUsage = acc.getProjectsWithTestNgTypeUsage().contains(jp); + // if any TestNG, break + if (testNgUsage || testNgDep) { return tree; } - if (!pwoTngd && !pwoTngtu) { + // if not a scanned project + if (!noTestNgDep && !noTestNgUsage) { return tree; } - if ( - (pwoTngd || acc.getProjectsWithoutTestNgDependency().isEmpty()) && - (pwoTngtu || acc.getProjectsWithoutTestNgTypeUsage().isEmpty()) - ) { + boolean depFreeOrVacuous = noTestNgDep || acc.getProjectsWithoutTestNgDependency().isEmpty(); + boolean usageFreeOrVacuous = noTestNgUsage || acc.getProjectsWithoutTestNgTypeUsage().isEmpty(); + if (depFreeOrVacuous && usageFreeOrVacuous) { return SearchResult.found(tree); } return tree; } }; } - - // @Override -// public TreeVisitor getVisitor() { -// return new TreeVisitor() { -// private final TreeVisitor dnut = new DoesNotUseType("org.testng..*", true).getVisitor(); -// private final TreeVisitor dnid = new DoesNotIncludeDependency("org.testng", "testng*", null, null, null).getVisitor(); -// -// @Override -// public boolean isAcceptable(SourceFile sourceFile, ExecutionContext executionContext) { -// return dnut.isAcceptable(sourceFile, executionContext) || dnid.isAcceptable(sourceFile, executionContext); -// } -// -// @Override -// public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) { -// if (!(tree instanceof SourceFile)) { -// return tree; -// } -// SourceFile s = (SourceFile) tree; -// SourceFile after = s; -// if (dnid.isAcceptable(s, ctx)) { -// after = (SourceFile) dnid.visitNonNull(s, ctx); -// } else if (dnut.isAcceptable(s, ctx)) { -// after = (SourceFile) dnut.visitNonNull(s, ctx); -// } -// if (after == s) { -// return s; -// } -// return SearchResult.found(after); -// } -// }; -// } } diff --git a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java index acd2beab7..7df05643c 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java @@ -168,9 +168,9 @@ void upgradeMavenPluginVersions() { void excludeJunit4Dependency() { // Just using play-test_2.13 as an example because it appears to still depend on junit. // In practice, this would probably just break it, I assume. - //language=xml rewriteRun( mavenProject("project", + //language=xml pomXml( """ @@ -238,9 +238,9 @@ void excludeJunit4Dependency() { @Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/429") @Test void dontExcludeJunit4DependencyFromTestcontainers() { - //language=xml rewriteRun( mavenProject("project", + //language=xml pomXml( """ @@ -266,9 +266,9 @@ void dontExcludeJunit4DependencyFromTestcontainers() { @Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/429") @Test void dontExcludeJunit4DependencyFromTestcontainersJupiter() { - //language=xml rewriteRun( mavenProject("project", + //language=xml pomXml( """ @@ -373,9 +373,9 @@ void test() { @Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/443") @Test void migrateInheritedTestBeforeAfterAnnotations() { - //language=java rewriteRun( mavenProject("project", + //language=java java( """ import org.junit.After; @@ -418,6 +418,7 @@ public void test() { ) ), mavenProject("project", + //language=java java( """ public class A extends AbstractTest { @@ -559,8 +560,8 @@ void addMockitoJupiterDependencyIfExtendWithPresent() { .build() .activateRecipes("org.openrewrite.java.testing.junit5.UseMockitoExtension")), mavenProject("sample", - //language=java srcMainJava( + //language=java java( """ import org.junit.runner.RunWith; @@ -681,37 +682,6 @@ public void testMethod() {} ); } -// @Test -// void blah() { -// rewriteRun( -// spec -> spec.recipe(new DoesNotUseType("org.testng..*", true)), -// //language=xml -// pomXml( -// """ -// -// 4.0.0 -// dev.ted -// testcontainer-migrate -// 0.0.1 -// -// -// junit -// junit -// 4.12 -// test -// -// -// org.testng -// testng -// 7.8.0 -// -// -// -// """ -// ) -// ); -// } - @Test void noChangesIfTestNgMavenDependencyIncluded() { rewriteRun( diff --git a/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java b/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java new file mode 100644 index 000000000..fab9cb781 --- /dev/null +++ b/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java @@ -0,0 +1,274 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.java.testing.junit5; + +import org.junit.jupiter.api.Test; +import org.openrewrite.InMemoryExecutionContext; +import org.openrewrite.java.JavaParser; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.openrewrite.gradle.Assertions.buildGradle; +import static org.openrewrite.gradle.toolingapi.Assertions.withToolingApi; +import static org.openrewrite.java.Assertions.*; +import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.maven.Assertions.pomXml; + +class TestNgGuardTest implements RewriteTest { + private static final String MavenMarker = ""; + private static final String GradleMarker = "/*~~>*/"; + private static final String JavaMarker = GradleMarker; + + @Override + public void defaults(RecipeSpec spec) { + spec + .parser(JavaParser.fromJavaVersion() + .classpathFromResources(new InMemoryExecutionContext(), "junit-jupiter-api-5", "testng")) + .beforeRecipe(withToolingApi()) + .recipe(new TestNgGuard()); + } + + @Test + void whenTestNgDependencyDoesNotMark() { + rewriteRun( + mavenProject("project-gradle", + //language=groovy + buildGradle( + """ + plugins { + id 'java-library' + } + repositories { + mavenCentral() + } + dependencies { + testImplementation 'org.junit.jupiter:junit-jupiter-api:5.13.3' + testImplementation 'org.testng:testng:7.8.0' + } + """ + ), + srcTestJava( + //language=java + java( + """ + import org.junit.jupiter.api.Test; + + class ExampleClassGradleTest { + @Test + public void testMethod() {} + } + """ + ) + ) + ), + mavenProject("project-maven", + //language=xml + pomXml( + """ + + com.example + project-maven + 0.0.1 + + + org.junit.jupiter + junit-jupiter-api + 5.13.3 + test + + + org.testng + testng + 7.8.0 + + + + """ + ), + srcTestJava( + //language=java + java( + """ + import org.junit.jupiter.api.Test; + + class ExampleClassMavenTest { + @Test + public void testMethod() {} + } + """ + ) + ) + ) + ); + } + + @Test + void whenTestNgTypeUsageDoesNotMark() { + rewriteRun( + mavenProject("project-gradle", + //language=groovy + buildGradle( + """ + plugins { + id 'java-library' + } + repositories { + mavenCentral() + } + dependencies { + testImplementation 'org.junit.jupiter:junit-jupiter-api:5.13.3' + } + """ + ), + srcTestJava( + //language=java + java( + """ + import org.testng.annotations.Test; + + class ExampleClassGradleTest { + @Test + public void testMethod() {} + } + """ + ) + ) + ), + mavenProject("project-maven", + //language=xml + pomXml( + """ + + com.example + project-maven + 0.0.1 + + + org.junit.jupiter + junit-jupiter-api + 5.13.3 + test + + + + """ + ), + srcTestJava( + //language=java + java( + """ + import org.testng.annotations.Test; + + class ExampleClassMavenTest { + @Test + public void testMethod() {} + } + """ + ) + ) + ) + ); + } + + @Test + void whenNoTestNgDependencyNorTypeUsageMarks() { + rewriteRun( + mavenProject("project-gradle", + //language=groovy + buildGradle( + """ + plugins { + id 'java-library' + } + repositories { + mavenCentral() + } + dependencies { + testImplementation 'org.junit.jupiter:junit-jupiter-api:5.13.3' + } + """, + spec -> spec.after(actual -> + assertThat(actual) + .startsWith(GradleMarker) + .actual() + ) + ), + srcTestJava( + //language=java + java( + """ + import org.junit.jupiter.api.Test; + + class ExampleClassGradleTest { + @Test + public void testMethod() {} + } + """, + spec -> spec.after(actual -> + assertThat(actual) + .startsWith(JavaMarker) + .actual() + ) + ) + ) + ), + mavenProject("project-maven", + //language=xml + pomXml( + """ + + com.example + project-maven + 0.0.1 + + + org.junit.jupiter + junit-jupiter-api + 5.13.3 + test + + + + """, + spec -> spec.after(actual -> + assertThat(actual) + .startsWith(MavenMarker) + .actual() + ) + ), + srcTestJava( + //language=java + java( + """ + import org.junit.jupiter.api.Test; + + class ExampleClassMavenTest { + @Test + public void testMethod() {} + } + """, + spec -> spec.after(actual -> + assertThat(actual) + .startsWith(JavaMarker) + .actual() + ) + ) + ) + ) + ); + } +} From cca009619205399de4456282cfd5e7383fb65438 Mon Sep 17 00:00:00 2001 From: steve-aom-elliott Date: Tue, 29 Jul 2025 15:52:06 -0400 Subject: [PATCH 09/15] Cleanup of import Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../org/openrewrite/java/testing/junit5/TestNgGuardTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java b/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java index fab9cb781..83e5ffb4f 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java @@ -25,7 +25,6 @@ import static org.openrewrite.gradle.Assertions.buildGradle; import static org.openrewrite.gradle.toolingapi.Assertions.withToolingApi; import static org.openrewrite.java.Assertions.*; -import static org.openrewrite.java.Assertions.java; import static org.openrewrite.maven.Assertions.pomXml; class TestNgGuardTest implements RewriteTest { From c90630770c4bfcc6cf736766e067b657cb8b84f8 Mon Sep 17 00:00:00 2001 From: Steve Elliott Date: Tue, 29 Jul 2025 17:07:03 -0400 Subject: [PATCH 10/15] Made the `TestNgGuard` more flexible for situations where there isn't an enclosing project for a source file, which was a majority of our existing tests. - If it figures out it's a loose file not in a `JavaProject`, it can't ensure project scope anyway, so falls back to solely whether the file itself has a TestNG dependency or uses TestNG types instead for those --- .../java/testing/junit5/TestNgGuard.java | 45 +- .../testing/junit5/JUnit5MigrationTest.java | 748 +++++++++--------- .../java/testing/junit5/TestNgGuardTest.java | 119 +++ 3 files changed, 519 insertions(+), 393 deletions(-) diff --git a/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java b/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java index 308024ca0..8d7965d0b 100644 --- a/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java +++ b/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java @@ -45,11 +45,20 @@ public static class Accumulator { Set projectsWithoutTestNgTypeUsage; Set projectsWithTestNgDependency; Set projectsWithTestNgTypeUsage; + Set looseUndesirables; + Set looseDesirables; } @Override public Accumulator getInitialValue(ExecutionContext ctx) { - return new Accumulator(new HashSet<>(), new HashSet<>(), new HashSet<>(), new HashSet<>()); + return new Accumulator( + new HashSet<>(), + new HashSet<>(), + new HashSet<>(), + new HashSet<>(), + new HashSet<>(), + new HashSet<>() + ); } @Override @@ -67,18 +76,36 @@ public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { SourceFile s = (SourceFile) tree; if (dnid.isAcceptable(s, ctx)) { Tree after = dnid.visit(tree, ctx); + Optional maybeProject = tree.getMarkers().findFirst(JavaProject.class); if (after != tree) { - tree.getMarkers().findFirst(JavaProject.class).ifPresent(acc.projectsWithoutTestNgDependency::add); + if (maybeProject.isPresent()) { + acc.projectsWithoutTestNgDependency.add(maybeProject.get()); + } else { + acc.looseDesirables.add(s); + } } else { - tree.getMarkers().findFirst(JavaProject.class).ifPresent(acc.projectsWithTestNgDependency::add); + if (maybeProject.isPresent()) { + acc.projectsWithTestNgDependency.add(maybeProject.get()); + } else { + acc.looseUndesirables.add(s); + } } } else if (ut.isAcceptable(s, ctx)) { Tree after = ut.visit(tree, ctx); + Optional maybeProject = tree.getMarkers().findFirst(JavaProject.class); // Note: Uses `UsesType` for inverted checking because `DoesNotUseType` will mark non Java-code files as well if (after == tree) { - tree.getMarkers().findFirst(JavaProject.class).ifPresent(acc.projectsWithoutTestNgTypeUsage::add); + if (maybeProject.isPresent()) { + acc.projectsWithoutTestNgTypeUsage.add(maybeProject.get()); + } else { + acc.looseDesirables.add(s); + } } else { - tree.getMarkers().findFirst(JavaProject.class).ifPresent(acc.projectsWithTestNgTypeUsage::add); + if (maybeProject.isPresent()) { + acc.projectsWithTestNgTypeUsage.add(maybeProject.get()); + } else { + acc.looseUndesirables.add(s); + } } } return tree; @@ -100,9 +127,15 @@ public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) { @Override public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { assert tree != null; + boolean isLooseDesirable = acc.getLooseDesirables().contains((SourceFile) tree); + boolean isLooseUndesirable = acc.getLooseUndesirables().contains((SourceFile) tree); Optional maybeJp = tree.getMarkers().findFirst(JavaProject.class); if (!maybeJp.isPresent()) { - return tree; + // if has TestNG or didn't scan + if (isLooseUndesirable || !isLooseDesirable) { + return tree; + } + return SearchResult.found(tree); } JavaProject jp = maybeJp.get(); boolean noTestNgDep = acc.getProjectsWithoutTestNgDependency().contains(jp); diff --git a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java index 7df05643c..f41fbf5db 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java @@ -56,42 +56,40 @@ void assertThatReceiver() { spec -> spec .parser(JavaParser.fromJavaVersion() .classpathFromResources(new InMemoryExecutionContext(), "junit-4", "hamcrest-3")), - mavenProject("project", - //language=java - java( - """ - import org.junit.Assert; - import org.junit.Test; - - import static java.util.Arrays.asList; - import static org.hamcrest.Matchers.containsInAnyOrder; - - public class SampleTest { - @SuppressWarnings("ALL") - @Test - public void filterShouldRemoveUnusedConfig() { - Assert.assertThat(asList("1", "2", "3"), - containsInAnyOrder("3", "2", "1")); - } - } - """, + //language=java + java( + """ + import org.junit.Assert; + import org.junit.Test; + + import static java.util.Arrays.asList; + import static org.hamcrest.Matchers.containsInAnyOrder; + + public class SampleTest { + @SuppressWarnings("ALL") + @Test + public void filterShouldRemoveUnusedConfig() { + Assert.assertThat(asList("1", "2", "3"), + containsInAnyOrder("3", "2", "1")); + } + } + """, + """ + import org.junit.jupiter.api.Test; + + import static java.util.Arrays.asList; + import static org.hamcrest.MatcherAssert.assertThat; + import static org.hamcrest.Matchers.containsInAnyOrder; + + public class SampleTest { + @SuppressWarnings("ALL") + @Test + public void filterShouldRemoveUnusedConfig() { + assertThat(asList("1", "2", "3"), + containsInAnyOrder("3", "2", "1")); + } + } """ - import org.junit.jupiter.api.Test; - - import static java.util.Arrays.asList; - import static org.hamcrest.MatcherAssert.assertThat; - import static org.hamcrest.Matchers.containsInAnyOrder; - - public class SampleTest { - @SuppressWarnings("ALL") - @Test - public void filterShouldRemoveUnusedConfig() { - assertThat(asList("1", "2", "3"), - containsInAnyOrder("3", "2", "1")); - } - } - """ - ) ) ); } @@ -100,28 +98,26 @@ public void filterShouldRemoveUnusedConfig() { @Test void classReference() { rewriteRun( - mavenProject("project", - //language=java - java( - """ - import org.junit.Test; - - public class Sample { - void method() { - Class c = Test.class; - } - } - """, + //language=java + java( + """ + import org.junit.Test; + + public class Sample { + void method() { + Class c = Test.class; + } + } + """, + """ + import org.junit.jupiter.api.Test; + + public class Sample { + void method() { + Class c = Test.class; + } + } """ - import org.junit.jupiter.api.Test; - - public class Sample { - void method() { - Class c = Test.class; - } - } - """ - ) ) ); } @@ -130,36 +126,34 @@ void method() { @Test void upgradeMavenPluginVersions() { rewriteRun( - mavenProject("project", - pomXml( - //language=xml - """ - - 4.0.0 - com.example.jackson - test-plugins - 1.0.0 - - - - org.apache.maven.plugins - maven-surefire-plugin - 2.20.1 - - - org.apache.maven.plugins - maven-failsafe-plugin - 2.20.1 - - - - - """, - spec -> spec.after(actual -> { - assertThat(Pattern.compile("3\\.(.*)").matcher(actual).results().toList()).hasSize(2); - return actual; - }) - ) + pomXml( + //language=xml + """ + + 4.0.0 + com.example.jackson + test-plugins + 1.0.0 + + + + org.apache.maven.plugins + maven-surefire-plugin + 2.20.1 + + + org.apache.maven.plugins + maven-failsafe-plugin + 2.20.1 + + + + + """, + spec -> spec.after(actual -> { + assertThat(Pattern.compile("3\\.(.*)").matcher(actual).results().toList()).hasSize(2); + return actual; + }) ) ); } @@ -169,68 +163,66 @@ void excludeJunit4Dependency() { // Just using play-test_2.13 as an example because it appears to still depend on junit. // In practice, this would probably just break it, I assume. rewriteRun( - mavenProject("project", - //language=xml - pomXml( - """ - - 4.0.0 - - org.springframework.boot - spring-boot-starter-parent - 3.2.1 - - - dev.ted - needs-exclusion - 0.0.1 - - - org.springframework.boot - spring-boot-starter - - - com.typesafe.play - play-test_2.13 - 2.9.6 - test - - - - """, + //language=xml + pomXml( + """ + + 4.0.0 + + org.springframework.boot + spring-boot-starter-parent + 3.2.1 + + + dev.ted + needs-exclusion + 0.0.1 + + + org.springframework.boot + spring-boot-starter + + + com.typesafe.play + play-test_2.13 + 2.9.6 + test + + + + """, + """ + + 4.0.0 + + org.springframework.boot + spring-boot-starter-parent + 3.2.1 + + + dev.ted + needs-exclusion + 0.0.1 + + + org.springframework.boot + spring-boot-starter + + + com.typesafe.play + play-test_2.13 + 2.9.6 + test + + + junit + junit + + + + + """ - - 4.0.0 - - org.springframework.boot - spring-boot-starter-parent - 3.2.1 - - - dev.ted - needs-exclusion - 0.0.1 - - - org.springframework.boot - spring-boot-starter - - - com.typesafe.play - play-test_2.13 - 2.9.6 - test - - - junit - junit - - - - - - """ - ) ) ); } @@ -239,26 +231,24 @@ void excludeJunit4Dependency() { @Test void dontExcludeJunit4DependencyFromTestcontainers() { rewriteRun( - mavenProject("project", - //language=xml - pomXml( + //language=xml + pomXml( + """ + + 4.0.0 + com.example.jackson + test-plugins + 1.0.0 + + + org.testcontainers + testcontainers + 1.18.3 + test + + + """ - - 4.0.0 - com.example.jackson - test-plugins - 1.0.0 - - - org.testcontainers - testcontainers - 1.18.3 - test - - - - """ - ) ) ); } @@ -267,26 +257,24 @@ void dontExcludeJunit4DependencyFromTestcontainers() { @Test void dontExcludeJunit4DependencyFromTestcontainersJupiter() { rewriteRun( - mavenProject("project", - //language=xml - pomXml( + //language=xml + pomXml( + """ + + 4.0.0 + com.example.jackson + test-plugins + 1.0.0 + + + org.testcontainers + junit-jupiter + 1.18.3 + test + + + """ - - 4.0.0 - com.example.jackson - test-plugins - 1.0.0 - - - org.testcontainers - junit-jupiter - 1.18.3 - test - - - - """ - ) ) ); } @@ -295,45 +283,43 @@ void dontExcludeJunit4DependencyFromTestcontainersJupiter() { @Test void dontExcludeJunit4DependencyFromSpringBootTestcontainers() { rewriteRun( - mavenProject("project", - //language=xml - pomXml( + //language=xml + pomXml( + """ + + 4.0.0 + + org.springframework.boot + spring-boot-starter-parent + 3.2.1 + + + dev.ted + testcontainer-migrate + 0.0.1 + + + org.springframework.boot + spring-boot-starter + + + org.springframework.boot + spring-boot-starter-test + test + + + org.springframework.boot + spring-boot-testcontainers + test + + + org.testcontainers + junit-jupiter + test + + + """ - - 4.0.0 - - org.springframework.boot - spring-boot-starter-parent - 3.2.1 - - - dev.ted - testcontainer-migrate - 0.0.1 - - - org.springframework.boot - spring-boot-starter - - - org.springframework.boot - spring-boot-starter-test - test - - - org.springframework.boot - spring-boot-testcontainers - test - - - org.testcontainers - junit-jupiter - test - - - - """ - ) ) ); } @@ -344,28 +330,26 @@ void dontExcludeJunit4DependencyFromSpringBootTestcontainers() { @Test void assertEqualsWithArrayArgumentToAssertArrayEquals() { rewriteRun( - mavenProject("project", - //language=java - java( - """ - import org.junit.Assert; - - class MyTest { - void test() { - Assert.assertEquals(new Object[1], new Object[1]); - } - } - """, + //language=java + java( + """ + import org.junit.Assert; + + class MyTest { + void test() { + Assert.assertEquals(new Object[1], new Object[1]); + } + } + """, + """ + import org.junit.jupiter.api.Assertions; + + class MyTest { + void test() { + Assertions.assertArrayEquals(new Object[1], new Object[1]); + } + } """ - import org.junit.jupiter.api.Assertions; - - class MyTest { - void test() { - Assertions.assertArrayEquals(new Object[1], new Object[1]); - } - } - """ - ) ) ); } @@ -374,84 +358,80 @@ void test() { @Test void migrateInheritedTestBeforeAfterAnnotations() { rewriteRun( - mavenProject("project", - //language=java - java( - """ - import org.junit.After; - import org.junit.Before; - import org.junit.Test; - - public class AbstractTest { - @Before - public void before() { - } - - @After - public void after() { - } + //language=java + java( + """ + import org.junit.After; + import org.junit.Before; + import org.junit.Test; + + public class AbstractTest { + @Before + public void before() { + } - @Test - public void test() { - } - } - """, - """ - import org.junit.jupiter.api.AfterEach; - import org.junit.jupiter.api.BeforeEach; - import org.junit.jupiter.api.Test; + @After + public void after() { + } - public class AbstractTest { - @BeforeEach - public void before() { - } + @Test + public void test() { + } + } + """, + """ + import org.junit.jupiter.api.AfterEach; + import org.junit.jupiter.api.BeforeEach; + import org.junit.jupiter.api.Test; + + public class AbstractTest { + @BeforeEach + public void before() { + } - @AfterEach - public void after() { - } + @AfterEach + public void after() { + } - @Test - public void test() { - } - } - """ - ) - ), - mavenProject("project", - //language=java - java( + @Test + public void test() { + } + } """ - public class A extends AbstractTest { - public void before() { - } - - public void after() { - } + ), + //language=java + java( + """ + public class A extends AbstractTest { + public void before() { + } - public void test() { - } - } - """, - """ - import org.junit.jupiter.api.AfterEach; - import org.junit.jupiter.api.BeforeEach; - import org.junit.jupiter.api.Test; + public void after() { + } - public class A extends AbstractTest { - @BeforeEach - public void before() { - } + public void test() { + } + } + """, + """ + import org.junit.jupiter.api.AfterEach; + import org.junit.jupiter.api.BeforeEach; + import org.junit.jupiter.api.Test; + + public class A extends AbstractTest { + @BeforeEach + public void before() { + } - @AfterEach - public void after() { - } + @AfterEach + public void after() { + } - @Test - public void test() { - } - } - """ - ) + @Test + public void test() { + } + } + """ ) ); } @@ -460,45 +440,41 @@ public void test() { void noJunitDependencyIfApiAlreadyPresent() { rewriteRun( spec -> spec.beforeRecipe(withToolingApi()), - mavenProject("project", - //language=groovy - buildGradle( + //language=groovy + buildGradle( + """ + plugins { + id 'java-library' + } + repositories { + mavenCentral() + } + dependencies { + testImplementation 'org.junit.jupiter:junit-jupiter-api:5.7.2' + } + tasks.withType(Test).configureEach { + useJUnitPlatform() + } """ - plugins { - id 'java-library' - } - repositories { - mavenCentral() - } - dependencies { - testImplementation 'org.junit.jupiter:junit-jupiter-api:5.7.2' - } - tasks.withType(Test).configureEach { - useJUnitPlatform() - } - """ - ) ), - mavenProject("project", - //language=xml - pomXml( + //language=xml + pomXml( + """ + + 4.0.0 + dev.ted + testcontainer-migrate + 0.0.1 + + + org.junit.jupiter + junit-jupiter-api + 5.7.2 + test + + + """ - - 4.0.0 - dev.ted - testcontainer-migrate - 0.0.1 - - - org.junit.jupiter - junit-jupiter-api - 5.7.2 - test - - - - """ - ) ) ); } @@ -507,44 +483,42 @@ void noJunitDependencyIfApiAlreadyPresent() { void bumpSurefireOnOlderMavenVersions() { rewriteRun( spec -> spec.recipeFromResource("/META-INF/rewrite/junit5.yml", "org.openrewrite.java.testing.junit5.UpgradeSurefirePlugin"), - mavenProject("project", - pomXml( - //language=xml - """ - - 4.0.0 - dev.ted - testcontainer-migrate - 0.0.1 - - """, - //language=xml - """ - - 4.0.0 - dev.ted - testcontainer-migrate - 0.0.1 - - - - org.apache.maven.plugins - maven-surefire-plugin - 3.2.5 - - - org.junit.platform - junit-platform-surefire-provider - 1.1.0 - - - - - - - """, - spec -> spec.markers(new BuildTool(Tree.randomId(), BuildTool.Type.Maven, "3.5.4")) - ) + pomXml( + //language=xml + """ + + 4.0.0 + dev.ted + testcontainer-migrate + 0.0.1 + + """, + //language=xml + """ + + 4.0.0 + dev.ted + testcontainer-migrate + 0.0.1 + + + + org.apache.maven.plugins + maven-surefire-plugin + 3.2.5 + + + org.junit.platform + junit-platform-surefire-provider + 1.1.0 + + + + + + + """, + spec -> spec.markers(new BuildTool(Tree.randomId(), BuildTool.Type.Maven, "3.5.4")) ) ); } diff --git a/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java b/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java index 83e5ffb4f..f035aa4db 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java @@ -270,4 +270,123 @@ public void testMethod() {} ) ); } + + @Test + void whenTestNgLooseFilesDoesNotMark() { + rewriteRun( + //language=groovy + buildGradle( + """ + plugins { + id 'java-library' + } + repositories { + mavenCentral() + } + dependencies { + testImplementation 'org.junit.jupiter:junit-jupiter-api:5.13.3' + testImplementation 'org.testng:testng:7.8.0' + } + """ + ), + //language=xml + pomXml( + """ + + com.example + project-maven + 0.0.1 + + + org.junit.jupiter + junit-jupiter-api + 5.13.3 + test + + + org.testng + testng + 7.8.0 + + + + """ + ), + //language=java + java( + """ + import org.testng.annotations.Test; + + class ExampleClassTest { + @Test + public void testMethod() {} + } + """ + ) + ); + } + + @Test + void whenNoTestNgLooseFilesMarks() { + rewriteRun( + //language=groovy + buildGradle( + """ + plugins { + id 'java-library' + } + repositories { + mavenCentral() + } + dependencies { + testImplementation 'org.junit.jupiter:junit-jupiter-api:5.13.3' + } + """, + spec -> spec.after(actual -> + assertThat(actual) + .startsWith(GradleMarker) + .actual() + ) + ), + //language=xml + pomXml( + """ + + com.example + project-maven + 0.0.1 + + + org.junit.jupiter + junit-jupiter-api + 5.13.3 + test + + + + """, + spec -> spec.after(actual -> + assertThat(actual) + .startsWith(MavenMarker) + .actual() + ) + ), + //language=java + java( + """ + import org.junit.jupiter.api.Test; + + class ExampleClassTest { + @Test + public void testMethod() {} + } + """, + spec -> spec.after(actual -> + assertThat(actual) + .startsWith(JavaMarker) + .actual() + ) + ) + ); + } } From c69b79d60b32464c24a73bad59e91be638ce6d0c Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 29 Jul 2025 23:49:59 +0200 Subject: [PATCH 11/15] Do not use getters internally --- .../java/testing/junit5/TestNgGuard.java | 16 ++++++++-------- .../java/testing/testng/TestNgToAssertJTest.java | 14 +++++++++++--- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java b/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java index 8d7965d0b..80198ae0a 100644 --- a/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java +++ b/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java @@ -127,8 +127,8 @@ public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) { @Override public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { assert tree != null; - boolean isLooseDesirable = acc.getLooseDesirables().contains((SourceFile) tree); - boolean isLooseUndesirable = acc.getLooseUndesirables().contains((SourceFile) tree); + boolean isLooseDesirable = acc.looseDesirables.contains((SourceFile) tree); + boolean isLooseUndesirable = acc.looseUndesirables.contains((SourceFile) tree); Optional maybeJp = tree.getMarkers().findFirst(JavaProject.class); if (!maybeJp.isPresent()) { // if has TestNG or didn't scan @@ -138,10 +138,10 @@ public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { return SearchResult.found(tree); } JavaProject jp = maybeJp.get(); - boolean noTestNgDep = acc.getProjectsWithoutTestNgDependency().contains(jp); - boolean noTestNgUsage = acc.getProjectsWithoutTestNgTypeUsage().contains(jp); - boolean testNgDep = acc.getProjectsWithTestNgDependency().contains(jp); - boolean testNgUsage = acc.getProjectsWithTestNgTypeUsage().contains(jp); + boolean noTestNgDep = acc.projectsWithoutTestNgDependency.contains(jp); + boolean noTestNgUsage = acc.projectsWithoutTestNgTypeUsage.contains(jp); + boolean testNgDep = acc.projectsWithTestNgDependency.contains(jp); + boolean testNgUsage = acc.projectsWithTestNgTypeUsage.contains(jp); // if any TestNG, break if (testNgUsage || testNgDep) { return tree; @@ -150,8 +150,8 @@ public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { if (!noTestNgDep && !noTestNgUsage) { return tree; } - boolean depFreeOrVacuous = noTestNgDep || acc.getProjectsWithoutTestNgDependency().isEmpty(); - boolean usageFreeOrVacuous = noTestNgUsage || acc.getProjectsWithoutTestNgTypeUsage().isEmpty(); + boolean depFreeOrVacuous = noTestNgDep || acc.projectsWithoutTestNgDependency.isEmpty(); + boolean usageFreeOrVacuous = noTestNgUsage || acc.projectsWithoutTestNgTypeUsage.isEmpty(); if (depFreeOrVacuous && usageFreeOrVacuous) { return SearchResult.found(tree); } diff --git a/src/test/java/org/openrewrite/java/testing/testng/TestNgToAssertJTest.java b/src/test/java/org/openrewrite/java/testing/testng/TestNgToAssertJTest.java index d3797a0ca..a3adc5c39 100644 --- a/src/test/java/org/openrewrite/java/testing/testng/TestNgToAssertJTest.java +++ b/src/test/java/org/openrewrite/java/testing/testng/TestNgToAssertJTest.java @@ -18,6 +18,7 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.java.JavaParser; +import org.openrewrite.java.TreeVisitingPrinter; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; @@ -52,16 +53,23 @@ void test() { } """, """ + import org.assertj.core.api.Assertions; + import static org.assertj.core.api.Assertions.fail; class Test { void test() { - fail("foo"); + Assertions.fail("foo"); fail("foo", new IllegalStateException()); - fail(); + Assertions.fail(); } } - """ + """, + spec -> spec.afterRecipe(cu -> { + String s = TreeVisitingPrinter.printTree(cu); + System.out.println(s); + System.out.println(s); + }) ) ); } From fd977e21fb8f43d974c881cd18ff4b10f396f581 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 30 Jul 2025 00:16:03 +0200 Subject: [PATCH 12/15] Adopt `ModuleHasDependency` and drop odd test: only look at dependencies Assuming classes can not be present without those dependencies --- .../java/testing/junit5/TestNgGuard.java | 123 ++---------------- .../testing/junit5/JUnit5MigrationTest.java | 24 ---- 2 files changed, 12 insertions(+), 135 deletions(-) diff --git a/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java b/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java index 80198ae0a..e9da77375 100644 --- a/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java +++ b/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java @@ -15,19 +15,20 @@ */ package org.openrewrite.java.testing.junit5; -import lombok.Value; import org.jspecify.annotations.Nullable; -import org.openrewrite.*; -import org.openrewrite.java.dependencies.search.DoesNotIncludeDependency; +import org.openrewrite.ExecutionContext; +import org.openrewrite.ScanningRecipe; +import org.openrewrite.Tree; +import org.openrewrite.TreeVisitor; +import org.openrewrite.java.dependencies.search.ModuleHasDependency; import org.openrewrite.java.marker.JavaProject; -import org.openrewrite.java.search.UsesType; import org.openrewrite.marker.SearchResult; import java.util.HashSet; import java.util.Optional; import java.util.Set; -public class TestNgGuard extends ScanningRecipe { +public class TestNgGuard extends ScanningRecipe> { @Override public String getDisplayName() { return "Find `TestNG`-free Maven / Gradle and Java files"; @@ -39,123 +40,23 @@ public String getDescription() { "that are part of a project that does not have TestNG dependencies nor usage of TestNG classes."; } - @Value - public static class Accumulator { - Set projectsWithoutTestNgDependency; - Set projectsWithoutTestNgTypeUsage; - Set projectsWithTestNgDependency; - Set projectsWithTestNgTypeUsage; - Set looseUndesirables; - Set looseDesirables; - } - @Override - public Accumulator getInitialValue(ExecutionContext ctx) { - return new Accumulator( - new HashSet<>(), - new HashSet<>(), - new HashSet<>(), - new HashSet<>(), - new HashSet<>(), - new HashSet<>() - ); + public Set getInitialValue(ExecutionContext ctx) { + return new HashSet<>(); } @Override - public TreeVisitor getScanner(Accumulator acc) { - return new TreeVisitor() { - private final TreeVisitor ut = new UsesType<>("org.testng..*", true); - private final TreeVisitor dnid = new DoesNotIncludeDependency("org.testng", "testng*", null, null, null).getVisitor(); - - @Override - public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { - assert tree != null; - if (!(tree instanceof SourceFile)) { - return tree; - } - SourceFile s = (SourceFile) tree; - if (dnid.isAcceptable(s, ctx)) { - Tree after = dnid.visit(tree, ctx); - Optional maybeProject = tree.getMarkers().findFirst(JavaProject.class); - if (after != tree) { - if (maybeProject.isPresent()) { - acc.projectsWithoutTestNgDependency.add(maybeProject.get()); - } else { - acc.looseDesirables.add(s); - } - } else { - if (maybeProject.isPresent()) { - acc.projectsWithTestNgDependency.add(maybeProject.get()); - } else { - acc.looseUndesirables.add(s); - } - } - } else if (ut.isAcceptable(s, ctx)) { - Tree after = ut.visit(tree, ctx); - Optional maybeProject = tree.getMarkers().findFirst(JavaProject.class); - // Note: Uses `UsesType` for inverted checking because `DoesNotUseType` will mark non Java-code files as well - if (after == tree) { - if (maybeProject.isPresent()) { - acc.projectsWithoutTestNgTypeUsage.add(maybeProject.get()); - } else { - acc.looseDesirables.add(s); - } - } else { - if (maybeProject.isPresent()) { - acc.projectsWithTestNgTypeUsage.add(maybeProject.get()); - } else { - acc.looseUndesirables.add(s); - } - } - } - return tree; - } - }; + public TreeVisitor getScanner(Set acc) { + return new ModuleHasDependency("org.testng", "testng*", null, null).getScanner(acc); } @Override - public TreeVisitor getVisitor(Accumulator acc) { + public TreeVisitor getVisitor(Set acc) { return new TreeVisitor() { - private final TreeVisitor ut = new UsesType<>("org.testng..*", true); - private final TreeVisitor dnid = new DoesNotIncludeDependency("org.testng", "testng*", null, null, null).getVisitor(); - - @Override - public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) { - return ut.isAcceptable(sourceFile, ctx) || dnid.isAcceptable(sourceFile, ctx); - } - @Override public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { - assert tree != null; - boolean isLooseDesirable = acc.looseDesirables.contains((SourceFile) tree); - boolean isLooseUndesirable = acc.looseUndesirables.contains((SourceFile) tree); Optional maybeJp = tree.getMarkers().findFirst(JavaProject.class); - if (!maybeJp.isPresent()) { - // if has TestNG or didn't scan - if (isLooseUndesirable || !isLooseDesirable) { - return tree; - } - return SearchResult.found(tree); - } - JavaProject jp = maybeJp.get(); - boolean noTestNgDep = acc.projectsWithoutTestNgDependency.contains(jp); - boolean noTestNgUsage = acc.projectsWithoutTestNgTypeUsage.contains(jp); - boolean testNgDep = acc.projectsWithTestNgDependency.contains(jp); - boolean testNgUsage = acc.projectsWithTestNgTypeUsage.contains(jp); - // if any TestNG, break - if (testNgUsage || testNgDep) { - return tree; - } - // if not a scanned project - if (!noTestNgDep && !noTestNgUsage) { - return tree; - } - boolean depFreeOrVacuous = noTestNgDep || acc.projectsWithoutTestNgDependency.isEmpty(); - boolean usageFreeOrVacuous = noTestNgUsage || acc.projectsWithoutTestNgTypeUsage.isEmpty(); - if (depFreeOrVacuous && usageFreeOrVacuous) { - return SearchResult.found(tree); - } - return tree; + return maybeJp.isPresent() && acc.contains(maybeJp.get()) ? tree : SearchResult.found(tree); } }; } diff --git a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java index f41fbf5db..dfa6bb4f8 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/JUnit5MigrationTest.java @@ -700,29 +700,5 @@ public void testMethod() {} ) ); } - - @Test - void noChangesIfTestNgCassIncluded() { - rewriteRun( - spec -> spec - .parser(JavaParser.fromJavaVersion() - .classpathFromResources(new InMemoryExecutionContext(), "junit-4", "testng")), - mavenProject("project", - //language=java - java( - """ - import org.junit.Ignore; - import org.testng.annotations.Test; - - class ExampleClass { - @Ignore - @Test - public void testMethod() {} - } - """ - ) - ) - ); - } } } From 8bc828152ab2e5814333189e41f87a3f906769f2 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 30 Jul 2025 00:33:03 +0200 Subject: [PATCH 13/15] Revert change to `TestNgToAssertJTest` --- .../java/testing/testng/TestNgToAssertJTest.java | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/test/java/org/openrewrite/java/testing/testng/TestNgToAssertJTest.java b/src/test/java/org/openrewrite/java/testing/testng/TestNgToAssertJTest.java index a3adc5c39..d3797a0ca 100644 --- a/src/test/java/org/openrewrite/java/testing/testng/TestNgToAssertJTest.java +++ b/src/test/java/org/openrewrite/java/testing/testng/TestNgToAssertJTest.java @@ -18,7 +18,6 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.java.JavaParser; -import org.openrewrite.java.TreeVisitingPrinter; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; @@ -53,23 +52,16 @@ void test() { } """, """ - import org.assertj.core.api.Assertions; - import static org.assertj.core.api.Assertions.fail; class Test { void test() { - Assertions.fail("foo"); + fail("foo"); fail("foo", new IllegalStateException()); - Assertions.fail(); + fail(); } } - """, - spec -> spec.afterRecipe(cu -> { - String s = TreeVisitingPrinter.printTree(cu); - System.out.println(s); - System.out.println(s); - }) + """ ) ); } From a0aa295dad32ffc4f55c1879fec78b30c5edc488 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 30 Jul 2025 00:35:05 +0200 Subject: [PATCH 14/15] Drop tests that were showing improbable use of types --- .../java/testing/junit5/TestNgGuardTest.java | 123 ------------------ 1 file changed, 123 deletions(-) diff --git a/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java b/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java index f035aa4db..9ce4d3a15 100644 --- a/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java +++ b/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java @@ -115,74 +115,6 @@ public void testMethod() {} ); } - @Test - void whenTestNgTypeUsageDoesNotMark() { - rewriteRun( - mavenProject("project-gradle", - //language=groovy - buildGradle( - """ - plugins { - id 'java-library' - } - repositories { - mavenCentral() - } - dependencies { - testImplementation 'org.junit.jupiter:junit-jupiter-api:5.13.3' - } - """ - ), - srcTestJava( - //language=java - java( - """ - import org.testng.annotations.Test; - - class ExampleClassGradleTest { - @Test - public void testMethod() {} - } - """ - ) - ) - ), - mavenProject("project-maven", - //language=xml - pomXml( - """ - - com.example - project-maven - 0.0.1 - - - org.junit.jupiter - junit-jupiter-api - 5.13.3 - test - - - - """ - ), - srcTestJava( - //language=java - java( - """ - import org.testng.annotations.Test; - - class ExampleClassMavenTest { - @Test - public void testMethod() {} - } - """ - ) - ) - ) - ); - } - @Test void whenNoTestNgDependencyNorTypeUsageMarks() { rewriteRun( @@ -271,61 +203,6 @@ public void testMethod() {} ); } - @Test - void whenTestNgLooseFilesDoesNotMark() { - rewriteRun( - //language=groovy - buildGradle( - """ - plugins { - id 'java-library' - } - repositories { - mavenCentral() - } - dependencies { - testImplementation 'org.junit.jupiter:junit-jupiter-api:5.13.3' - testImplementation 'org.testng:testng:7.8.0' - } - """ - ), - //language=xml - pomXml( - """ - - com.example - project-maven - 0.0.1 - - - org.junit.jupiter - junit-jupiter-api - 5.13.3 - test - - - org.testng - testng - 7.8.0 - - - - """ - ), - //language=java - java( - """ - import org.testng.annotations.Test; - - class ExampleClassTest { - @Test - public void testMethod() {} - } - """ - ) - ); - } - @Test void whenNoTestNgLooseFilesMarks() { rewriteRun( From b504c95e08bd147fd852c7b86f883ce713571454 Mon Sep 17 00:00:00 2001 From: Steve Elliott Date: Thu, 31 Jul 2025 17:25:37 -0400 Subject: [PATCH 15/15] Switching to using the new `invertMarking` option on `ModuleHasDependency` for the TestNG guard for JUnit --- .../java/testing/junit5/TestNgGuard.java | 63 ---- .../resources/META-INF/rewrite/junit5.yml | 5 +- .../java/testing/junit5/TestNgGuardTest.java | 269 ------------------ 3 files changed, 4 insertions(+), 333 deletions(-) delete mode 100644 src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java delete mode 100644 src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java diff --git a/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java b/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java deleted file mode 100644 index e9da77375..000000000 --- a/src/main/java/org/openrewrite/java/testing/junit5/TestNgGuard.java +++ /dev/null @@ -1,63 +0,0 @@ -/* - * Copyright 2025 the original author or authors. - *

- * Licensed under the Moderne Source Available License (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - *

- * https://docs.moderne.io/licensing/moderne-source-available-license - *

- * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.openrewrite.java.testing.junit5; - -import org.jspecify.annotations.Nullable; -import org.openrewrite.ExecutionContext; -import org.openrewrite.ScanningRecipe; -import org.openrewrite.Tree; -import org.openrewrite.TreeVisitor; -import org.openrewrite.java.dependencies.search.ModuleHasDependency; -import org.openrewrite.java.marker.JavaProject; -import org.openrewrite.marker.SearchResult; - -import java.util.HashSet; -import java.util.Optional; -import java.util.Set; - -public class TestNgGuard extends ScanningRecipe> { - @Override - public String getDisplayName() { - return "Find `TestNG`-free Maven / Gradle and Java files"; - } - - @Override - public String getDescription() { - return "Meant to be used as a precondition, it will return results for Maven / Gradle and Java files " + - "that are part of a project that does not have TestNG dependencies nor usage of TestNG classes."; - } - - @Override - public Set getInitialValue(ExecutionContext ctx) { - return new HashSet<>(); - } - - @Override - public TreeVisitor getScanner(Set acc) { - return new ModuleHasDependency("org.testng", "testng*", null, null).getScanner(acc); - } - - @Override - public TreeVisitor getVisitor(Set acc) { - return new TreeVisitor() { - @Override - public Tree visit(@Nullable Tree tree, ExecutionContext ctx) { - Optional maybeJp = tree.getMarkers().findFirst(JavaProject.class); - return maybeJp.isPresent() && acc.contains(maybeJp.get()) ? tree : SearchResult.found(tree); - } - }; - } -} diff --git a/src/main/resources/META-INF/rewrite/junit5.yml b/src/main/resources/META-INF/rewrite/junit5.yml index e8f35dade..a2650f8fc 100755 --- a/src/main/resources/META-INF/rewrite/junit5.yml +++ b/src/main/resources/META-INF/rewrite/junit5.yml @@ -58,7 +58,10 @@ tags: - testing - junit preconditions: - - org.openrewrite.java.testing.junit5.TestNgGuard + - org.openrewrite.java.dependencies.search.ModuleHasDependency: + groupIdPattern: org.testng + artifactIdPattern: testng* + invertMarking: true recipeList: - org.openrewrite.java.testing.junit5.EnvironmentVariables - org.openrewrite.java.testing.junit5.UseWiremockExtension diff --git a/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java b/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java deleted file mode 100644 index 9ce4d3a15..000000000 --- a/src/test/java/org/openrewrite/java/testing/junit5/TestNgGuardTest.java +++ /dev/null @@ -1,269 +0,0 @@ -/* - * Copyright 2025 the original author or authors. - *

- * Licensed under the Moderne Source Available License (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - *

- * https://docs.moderne.io/licensing/moderne-source-available-license - *

- * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.openrewrite.java.testing.junit5; - -import org.junit.jupiter.api.Test; -import org.openrewrite.InMemoryExecutionContext; -import org.openrewrite.java.JavaParser; -import org.openrewrite.test.RecipeSpec; -import org.openrewrite.test.RewriteTest; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.openrewrite.gradle.Assertions.buildGradle; -import static org.openrewrite.gradle.toolingapi.Assertions.withToolingApi; -import static org.openrewrite.java.Assertions.*; -import static org.openrewrite.maven.Assertions.pomXml; - -class TestNgGuardTest implements RewriteTest { - private static final String MavenMarker = ""; - private static final String GradleMarker = "/*~~>*/"; - private static final String JavaMarker = GradleMarker; - - @Override - public void defaults(RecipeSpec spec) { - spec - .parser(JavaParser.fromJavaVersion() - .classpathFromResources(new InMemoryExecutionContext(), "junit-jupiter-api-5", "testng")) - .beforeRecipe(withToolingApi()) - .recipe(new TestNgGuard()); - } - - @Test - void whenTestNgDependencyDoesNotMark() { - rewriteRun( - mavenProject("project-gradle", - //language=groovy - buildGradle( - """ - plugins { - id 'java-library' - } - repositories { - mavenCentral() - } - dependencies { - testImplementation 'org.junit.jupiter:junit-jupiter-api:5.13.3' - testImplementation 'org.testng:testng:7.8.0' - } - """ - ), - srcTestJava( - //language=java - java( - """ - import org.junit.jupiter.api.Test; - - class ExampleClassGradleTest { - @Test - public void testMethod() {} - } - """ - ) - ) - ), - mavenProject("project-maven", - //language=xml - pomXml( - """ - - com.example - project-maven - 0.0.1 - - - org.junit.jupiter - junit-jupiter-api - 5.13.3 - test - - - org.testng - testng - 7.8.0 - - - - """ - ), - srcTestJava( - //language=java - java( - """ - import org.junit.jupiter.api.Test; - - class ExampleClassMavenTest { - @Test - public void testMethod() {} - } - """ - ) - ) - ) - ); - } - - @Test - void whenNoTestNgDependencyNorTypeUsageMarks() { - rewriteRun( - mavenProject("project-gradle", - //language=groovy - buildGradle( - """ - plugins { - id 'java-library' - } - repositories { - mavenCentral() - } - dependencies { - testImplementation 'org.junit.jupiter:junit-jupiter-api:5.13.3' - } - """, - spec -> spec.after(actual -> - assertThat(actual) - .startsWith(GradleMarker) - .actual() - ) - ), - srcTestJava( - //language=java - java( - """ - import org.junit.jupiter.api.Test; - - class ExampleClassGradleTest { - @Test - public void testMethod() {} - } - """, - spec -> spec.after(actual -> - assertThat(actual) - .startsWith(JavaMarker) - .actual() - ) - ) - ) - ), - mavenProject("project-maven", - //language=xml - pomXml( - """ - - com.example - project-maven - 0.0.1 - - - org.junit.jupiter - junit-jupiter-api - 5.13.3 - test - - - - """, - spec -> spec.after(actual -> - assertThat(actual) - .startsWith(MavenMarker) - .actual() - ) - ), - srcTestJava( - //language=java - java( - """ - import org.junit.jupiter.api.Test; - - class ExampleClassMavenTest { - @Test - public void testMethod() {} - } - """, - spec -> spec.after(actual -> - assertThat(actual) - .startsWith(JavaMarker) - .actual() - ) - ) - ) - ) - ); - } - - @Test - void whenNoTestNgLooseFilesMarks() { - rewriteRun( - //language=groovy - buildGradle( - """ - plugins { - id 'java-library' - } - repositories { - mavenCentral() - } - dependencies { - testImplementation 'org.junit.jupiter:junit-jupiter-api:5.13.3' - } - """, - spec -> spec.after(actual -> - assertThat(actual) - .startsWith(GradleMarker) - .actual() - ) - ), - //language=xml - pomXml( - """ - - com.example - project-maven - 0.0.1 - - - org.junit.jupiter - junit-jupiter-api - 5.13.3 - test - - - - """, - spec -> spec.after(actual -> - assertThat(actual) - .startsWith(MavenMarker) - .actual() - ) - ), - //language=java - java( - """ - import org.junit.jupiter.api.Test; - - class ExampleClassTest { - @Test - public void testMethod() {} - } - """, - spec -> spec.after(actual -> - assertThat(actual) - .startsWith(JavaMarker) - .actual() - ) - ) - ); - } -}