From bf5495d98bbdac8e7de2a1d76c9c4f1debdac839 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 6 Jul 2026 14:56:52 +0200 Subject: [PATCH 1/3] Skip AddMockitoJavaAgentToMavenSurefirePlugin when agent already configured in build/plugins (#1164) --- ...MockitoJavaAgentToMavenSurefirePlugin.java | 32 +++++++- ...itoJavaAgentToMavenSurefirePluginTest.java | 80 +++++++++++++++++++ 2 files changed, 109 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java b/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java index b75ac5150b..786f855ec2 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; @@ -102,11 +103,36 @@ private void maybeAddMavenDependencyPluginWithPropertiesGoal() { } } + private @Nullable String surefireArgLineWithAgent(List plugins) { + return plugins.stream() + .filter(plugin -> "org.apache.maven.plugins".equals(plugin.getGroupId()) && + "maven-surefire-plugin".equals(plugin.getArtifactId())) + .map(plugin -> plugin.getConfigurationStringValue("argLine")) + .filter(argLine -> argLine != null && argLine.contains(getArgLineJavaAgentArgument())) + .findFirst().orElse(null); + } + + private boolean mavenDependencyPluginHasPropertiesGoal() { + return getResolutionResult().getPom().getPlugins().stream() + .filter(plugin -> "org.apache.maven.plugins".equals(plugin.getGroupId()) && + "maven-dependency-plugin".equals(plugin.getArtifactId())) + .flatMap(plugin -> plugin.getExecutions().stream()) + .anyMatch(execution -> execution.getGoals() != null && execution.getGoals().contains("properties")); + } + @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()))) { + // When a parent pom manages the surefire plugin with the agent, leave the whole reactor alone. + if (surefireArgLineWithAgent(getResolutionResult().getPom().getPluginManagement()) != null) { + return document; + } + // When the surefire plugin (possibly inherited from a parent's `build/plugins`) already carries + // the agent, the supporting `maven-dependency-plugin` properties goal is present, and any + // `@{argLine}` reference can already resolve, there is nothing left to add; adding an `argLine` + // property here would be redundant (see #1164). + String pluginArgLine = surefireArgLineWithAgent(getResolutionResult().getPom().getPlugins()); + if (pluginArgLine != null && mavenDependencyPluginHasPropertiesGoal() && + (!pluginArgLine.contains("@{argLine}") || getResolutionResult().getPom().getProperties().containsKey("argLine"))) { return document; } diff --git a/src/test/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePluginTest.java b/src/test/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePluginTest.java index 6b7f991907..0947b5ef67 100644 --- a/src/test/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePluginTest.java +++ b/src/test/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePluginTest.java @@ -1372,6 +1372,86 @@ 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 augmentsSurefirePluginDeclaredInPluginManagement() { rewriteRun( From 9f7a0652481b28357cc103b3450b958f62d82b39 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 6 Jul 2026 15:04:47 +0200 Subject: [PATCH 2/3] Minimize comments --- .../migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java b/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java index 786f855ec2..0fe05dbd3d 100644 --- a/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java +++ b/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java @@ -122,14 +122,11 @@ private boolean mavenDependencyPluginHasPropertiesGoal() { @Override public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { - // When a parent pom manages the surefire plugin with the agent, leave the whole reactor alone. if (surefireArgLineWithAgent(getResolutionResult().getPom().getPluginManagement()) != null) { return document; } - // When the surefire plugin (possibly inherited from a parent's `build/plugins`) already carries - // the agent, the supporting `maven-dependency-plugin` properties goal is present, and any - // `@{argLine}` reference can already resolve, there is nothing left to add; adding an `argLine` - // property here would be redundant (see #1164). + // 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"))) { From 0a97c2fbaf1487aaa44615ac6c131f340b894ef6 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 6 Jul 2026 15:20:21 +0200 Subject: [PATCH 3/3] Simplify plugin matching; tolerate implied plugin groupId --- ...MockitoJavaAgentToMavenSurefirePlugin.java | 35 +++++++----- ...itoJavaAgentToMavenSurefirePluginTest.java | 54 +++++++++++++++++++ 2 files changed, 77 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java b/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java index 0fe05dbd3d..b7925d2544 100644 --- a/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java +++ b/src/main/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePlugin.java @@ -50,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"; @@ -86,17 +87,16 @@ 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))); @@ -104,20 +104,17 @@ private void maybeAddMavenDependencyPluginWithPropertiesGoal() { } private @Nullable String surefireArgLineWithAgent(List plugins) { + String agentArgument = getArgLineJavaAgentArgument(); return plugins.stream() - .filter(plugin -> "org.apache.maven.plugins".equals(plugin.getGroupId()) && - "maven-surefire-plugin".equals(plugin.getArtifactId())) + .filter(plugin -> isMavenPlugin(plugin, MAVEN_SUREFIRE_PLUGIN_ARTIFACT_ID)) .map(plugin -> plugin.getConfigurationStringValue("argLine")) - .filter(argLine -> argLine != null && argLine.contains(getArgLineJavaAgentArgument())) + .filter(argLine -> argLine != null && argLine.contains(agentArgument)) .findFirst().orElse(null); } private boolean mavenDependencyPluginHasPropertiesGoal() { return getResolutionResult().getPom().getPlugins().stream() - .filter(plugin -> "org.apache.maven.plugins".equals(plugin.getGroupId()) && - "maven-dependency-plugin".equals(plugin.getArtifactId())) - .flatMap(plugin -> plugin.getExecutions().stream()) - .anyMatch(execution -> execution.getGoals() != null && execution.getGoals().contains("properties")); + .anyMatch(plugin -> isMavenPlugin(plugin, MAVEN_DEPENDENCY_PLUGIN_ARTIFACT_ID) && hasPropertiesGoal(plugin)); } @Override @@ -191,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 0947b5ef67..cf1275308f 100644 --- a/src/test/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePluginTest.java +++ b/src/test/java/org/openrewrite/java/migrate/AddMockitoJavaAgentToMavenSurefirePluginTest.java @@ -1452,6 +1452,60 @@ void makesNoChangesWhenParentPomDeclaresSurefirePluginWithAgentConfiguration() { ); } + @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(