diff --git a/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java b/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java index b75ac5150b..b7925d2544 100644 --- a/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java +++ b/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java @@ -18,6 +18,7 @@ import lombok.Getter; import lombok.RequiredArgsConstructor; import org.intellij.lang.annotations.Language; +import org.jspecify.annotations.Nullable; import org.openrewrite.ExecutionContext; import org.openrewrite.Preconditions; import org.openrewrite.Recipe; @@ -49,6 +50,7 @@ public class AddMockitoJavaAgentToMavenSurefirePlugin extends Recipe { private static final String MAVEN_PLUGINS_GROUP_ID = "org.apache.maven.plugins"; private static final String MAVEN_SUREFIRE_PLUGIN_ARTIFACT_ID = "maven-surefire-plugin"; + private static final String MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID = "maven-dependency-plugin"; @Language("xpath") private static final String MAVEN_DEPENDENCY_PLUGIN_EXECUTION_MATCHER = "/project/build/plugins/plugin[artifactId='maven-dependency-plugin']/executions/execution"; @@ -85,28 +87,46 @@ private Xml.Tag buildConfigurationTag(String argLineJavaAgentParam, boolean hasE private void maybeAddMavenDependencyPluginWithPropertiesGoal() { Optional mavenDependencyPlugin = getResolutionResult().getPom().getPlugins().stream() - .filter(plugin -> "org.apache.maven.plugins".equals(plugin.getGroupId()) && - "maven-dependency-plugin".equals(plugin.getArtifactId())).findFirst(); + .filter(plugin -> isMavenPlugin(plugin, MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID)).findFirst(); if (!mavenDependencyPlugin.isPresent()) { - doAfterVisit(new AddPlugin("org.apache.maven.plugins", "maven-dependency-plugin", null, null, null, + doAfterVisit(new AddPlugin(MAVEN_PLUGINS_GROUP_ID, MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID, null, null, null, "" + MAVEN_DEPENDENCY_PLUGIN_EXECUTION_TAG + "", "**/pom.xml").getVisitor()); } else if (mavenDependencyPlugin.get().getExecutions().isEmpty()) { - doAfterVisit(new ChangePluginExecutions("org.apache.maven.plugins", "maven-dependency-plugin", MAVEN_DEPENDENCY_PLUGIN_EXECUTION_TAG).getVisitor()); + doAfterVisit(new ChangePluginExecutions(MAVEN_PLUGINS_GROUP_ID, MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID, MAVEN_DEPENDENCY_PLUGIN_EXECUTION_TAG).getVisitor()); } else if (mavenDependencyPlugin.get().getExecutions().stream().noneMatch(execution -> execution.getGoals() != null)) { doAfterVisit(new AddOrUpdateChildTag(MAVEN_DEPENDENCY_PLUGIN_EXECUTION_MATCHER, "" + MAVEN_DEPENDENCY_PLUGIN_PROPERTIES_GOAL + "", false).getVisitor()); - } else if (mavenDependencyPlugin.get().getExecutions().stream().noneMatch(execution -> execution.getGoals() != null && execution.getGoals().contains("properties"))) { + } else if (!hasPropertiesGoal(mavenDependencyPlugin.get())) { doAfterVisit(new AppendChildTagToParentVisitor( new XPathMatcher(MAVEN_DEPENDENCY_PLUGIN_EXECUTION_MATCHER + "/goals"), Xml.Tag.build(MAVEN_DEPENDENCY_PLUGIN_PROPERTIES_GOAL))); } } + private @Nullable String surefireArgLineWithAgent(List plugins) { + String agentArgument = getArgLineJavaAgentArgument(); + return plugins.stream() + .filter(plugin -> isMavenPlugin(plugin, MAVEN_SUREFIRE_PLUGIN_ARTIFACT_ID)) + .map(plugin -> plugin.getConfigurationStringValue("argLine")) + .filter(argLine -> argLine != null && argLine.contains(agentArgument)) + .findFirst().orElse(null); + } + + private boolean mavenDependencyPluginHasPropertiesGoal() { + return getResolutionResult().getPom().getPlugins().stream() + .anyMatch(plugin -> isMavenPlugin(plugin, MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID) && hasPropertiesGoal(plugin)); + } + @Override public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { - if (getResolutionResult().getPom().getPluginManagement().stream().anyMatch( - plugin -> "org.apache.maven.plugins".equals(plugin.getGroupId()) && "maven-surefire-plugin" - .equals(plugin.getArtifactId()) && plugin.getConfigurationStringValue("argLine") != null && plugin.getConfigurationStringValue("argLine").contains(getArgLineJavaAgentArgument()))) { + if (surefireArgLineWithAgent(getResolutionResult().getPom().getPluginManagement()) != null) { + return document; + } + // Nothing to add when the surefire agent, the properties goal, and any `@{argLine}` property + // are already present, whether declared here or inherited from a parent's `build/plugins` (#1164). + String pluginArgLine = surefireArgLineWithAgent(getResolutionResult().getPom().getPlugins()); + if (pluginArgLine != null && mavenDependencyPluginHasPropertiesGoal() && + (!pluginArgLine.contains("@{argLine}") || getResolutionResult().getPom().getProperties().containsKey("argLine"))) { return document; } @@ -168,6 +188,20 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { }); } + /** + * Matches a plugin under the {@code org.apache.maven.plugins} group, tolerating an implied (omitted) groupId, + * which Maven defaults to that group for {@code maven-*-plugin} declarations. + */ + private static boolean isMavenPlugin(Plugin plugin, String artifactId) { + return artifactId.equals(plugin.getArtifactId()) && + (plugin.getGroupId() == null || MAVEN_PLUGINS_GROUP_ID.equals(plugin.getGroupId())); + } + + private static boolean hasPropertiesGoal(Plugin plugin) { + return plugin.getExecutions().stream() + .anyMatch(execution -> execution.getGoals() != null && execution.getGoals().contains("properties")); + } + @RequiredArgsConstructor private static class AppendChildTagToParentVisitor extends XmlIsoVisitor { private final XPathMatcher parentXPathMatcher; diff --git a/src/test/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePluginTest.java b/src/test/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePluginTest.java index 6b7f991907..cf1275308f 100644 --- a/src/test/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePluginTest.java +++ b/src/test/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePluginTest.java @@ -1372,6 +1372,140 @@ void updatesIndividualPomsWhenParentPomManagesSurefirePluginWithoutAgentConfigur ); } + @Test + void makesNoChangesWhenParentPomDeclaresSurefirePluginWithAgentConfiguration() { + rewriteRun( + mavenProject("test-project", + pomXml( + """ + + 4.0.0 + org.sample + test + 1.0 + pom + + + test-module1 + + + + org.springframework.boot + spring-boot-starter-parent + 3.5.4 + + + + + + org.springframework.boot + spring-boot-starter-test + test + + + + + + org.apache.maven.plugins + maven-dependency-plugin + + + + properties + + + + + + org.apache.maven.plugins + maven-surefire-plugin + + + -Xmx204m -javaagent:${org.mockito:mockito-core:jar} + + + + + + """, + spec -> spec.path("pom.xml") + ), + pomXml( + """ + + 4.0.0 + org.sample + test-module1 + 1.0 + + + org.sample + test + 1.0 + ../pom.xml + + + """, + spec -> spec.path("test-module1/pom.xml") + ) + ) + ); + } + + @Test + void makesNoChangesWhenAgentConfiguredOnPluginsWithImpliedGroupId() { + rewriteRun( + mavenProject("test-project", + pomXml( + """ + + 4.0.0 + org.sample + test + 1.0 + + + org.springframework.boot + spring-boot-starter-parent + 3.5.4 + + + + + + org.springframework.boot + spring-boot-starter-test + test + + + + + + maven-dependency-plugin + + + + properties + + + + + + maven-surefire-plugin + + + -Xmx204m -javaagent:${org.mockito:mockito-core:jar} + + + + + + """ + ) + ) + ); + } + @Test void augmentsSurefirePluginDeclaredInPluginManagement() { rewriteRun(