From 832dba245fc1cf28b41771831dc8219071794aa2 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Mon, 21 Sep 2026 20:24:56 +0000 Subject: [PATCH 1/6] Fix #13230: add --skip-phases and --skip-tests CLI options; fix lifecycle DAG --- .../maven/api/cli/mvn/MavenOptions.java | 33 +++++ .../invoker/mvn/CommonsCliMavenOptions.java | 33 +++++ .../invoker/mvn/LayeredMavenOptions.java | 10 ++ .../maven/cling/invoker/mvn/MavenInvoker.java | 11 ++ .../DefaultMavenExecutionRequest.java | 21 +++ .../execution/MavenExecutionRequest.java | 18 +++ .../impl/DefaultLifecycleRegistry.java | 6 +- ...faultLifecycleExecutionPlanCalculator.java | 10 +- .../concurrent/BuildPlanExecutor.java | 30 ++++- ...tLifecycleExecutionPlanCalculatorTest.java | 81 ++++++++++++ .../concurrent/BuildPlanExecutorTest.java | 123 ++++++++++++++++++ 11 files changed, 369 insertions(+), 7 deletions(-) diff --git a/api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvn/MavenOptions.java b/api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvn/MavenOptions.java index a77ea4ebb7e0..bd9bc8e45062 100644 --- a/api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvn/MavenOptions.java +++ b/api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvn/MavenOptions.java @@ -213,6 +213,39 @@ public interface MavenOptions extends Options { */ Optional atFile(); + /** + * Returns the list of lifecycle phases to skip (mojos bound to these phases will not be executed). + * + *

This provides a plugin-agnostic alternative to per-plugin skip properties such as + * {@code -DskipTests}. All mojos bound to the listed phases are suppressed, regardless of + * which plugin they belong to. The phases themselves remain in the lifecycle DAG — only their + * mojo executions are inhibited.

+ * + *

Example: {@code --skip-phases=test,integration-test} suppresses all test execution + * while still running compile, package, and verify.

+ * + * @return an {@link Optional} containing the list of phase names to skip, or empty if not specified + * @since 4.1.0 + */ + @Nonnull + Optional> skippedPhases(); + + /** + * Returns whether to skip all test-related phases ({@code test} and {@code integration-test}). + * + *

This is a convenient shorthand for {@code --skip-phases=test,integration-test}. It suppresses + * all mojos bound to the {@code test} and {@code integration-test} lifecycle phases, regardless of + * which plugin they belong to, while still running {@code verify} checks (e.g. checkstyle, spotbugs).

+ * + *

Unlike {@code -DskipTests} which is maven-surefire-plugin-specific, this option works with + * any test plugin.

+ * + * @return an {@link Optional} containing {@code true} if tests should be skipped, or empty if not specified + * @since 4.1.0 + */ + @Nonnull + Optional skipTests(); + /** * Returns the list of goals and phases to execute. * diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptions.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptions.java index 233e22f15e76..1efae510172d 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptions.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptions.java @@ -215,6 +215,24 @@ public Optional atFile() { return Optional.empty(); } + @Override + public Optional> skippedPhases() { + if (commandLine.hasOption(CLIManager.SKIP_PHASES)) { + return Optional.of(Arrays.stream(commandLine.getOptionValues(CLIManager.SKIP_PHASES)) + .map(String::strip) + .toList()); + } + return Optional.empty(); + } + + @Override + public Optional skipTests() { + if (commandLine.hasOption(CLIManager.SKIP_TESTS)) { + return Optional.of(true); + } + return Optional.empty(); + } + @Override public Optional> goals() { if (!commandLine.getArgList().isEmpty()) { @@ -252,6 +270,8 @@ protected static class CLIManager extends CommonsCliOptions.CLIManager { public static final String STRICT_ARTIFACT_DESCRIPTOR_POLICY = "sadp"; public static final String IGNORE_TRANSITIVE_REPOSITORIES = "itr"; public static final String AT_FILE = "af"; + public static final String SKIP_PHASES = "sp"; + public static final String SKIP_TESTS = "st"; @Override protected void prepareOptions(org.apache.commons.cli.Options options) { @@ -360,6 +380,19 @@ protected void prepareOptions(org.apache.commons.cli.Options options) { .desc( "If set, Maven will load command line options from the specified file and merge with CLI specified ones.") .get()); + options.addOption(Option.builder(SKIP_PHASES) + .longOpt("skip-phases") + .hasArgs() + .valueSeparator(',') + .desc("Comma-separated list of lifecycle phases whose mojo executions should be skipped." + + " The phases remain in the DAG; only their bound mojos are suppressed." + + " Example: --skip-phases=test,integration-test") + .get()); + options.addOption(Option.builder(SKIP_TESTS) + .longOpt("skip-tests") + .desc("Skip test and integration-test phases." + + " Shorthand for --skip-phases=test,integration-test.") + .get()); } } } diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/LayeredMavenOptions.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/LayeredMavenOptions.java index a4ad7f5e41d1..0dd5b8da0a82 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/LayeredMavenOptions.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/LayeredMavenOptions.java @@ -164,6 +164,16 @@ public Optional> goals() { return collectListIfPresentOrEmpty(MavenOptions::goals); } + @Override + public Optional> skippedPhases() { + return collectListIfPresentOrEmpty(MavenOptions::skippedPhases); + } + + @Override + public Optional skipTests() { + return returnFirstPresentOrEmpty(MavenOptions::skipTests); + } + @Override public MavenOptions interpolate(UnaryOperator callback) { ArrayList interpolatedOptions = new ArrayList<>(options.size()); diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenInvoker.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenInvoker.java index e6372ccfd818..f1a7b95c00c5 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenInvoker.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenInvoker.java @@ -236,6 +236,17 @@ protected void populateRequest(MavenContext context, Lookup lookup, MavenExecuti request.setNoSnapshotUpdates(context.options().suppressSnapshotUpdates().orElse(false)); request.setGoals(context.options().goals().orElse(List.of())); + request.setSkippedPhases(context.options().skippedPhases().orElse(List.of())); + if (context.options().skipTests().orElse(false)) { + List phases = new ArrayList<>(request.getSkippedPhases()); + if (!phases.contains("test")) { + phases.add("test"); + } + if (!phases.contains("integration-test")) { + phases.add("integration-test"); + } + request.setSkippedPhases(phases); + } request.setReactorFailureBehavior(determineReactorFailureBehaviour(context)); request.setRecursive(!context.options().nonRecursive().orElse(!request.isRecursive())); request.setOffline(context.options().offline().orElse(request.isOffline())); diff --git a/impl/maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionRequest.java b/impl/maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionRequest.java index eedfe23016a6..8cd8dbc3ce2d 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionRequest.java +++ b/impl/maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionRequest.java @@ -119,6 +119,8 @@ public class DefaultMavenExecutionRequest implements MavenExecutionRequest { private List goals; + private List skippedPhases; + private boolean useReactor = false; private boolean recursive = true; @@ -197,6 +199,7 @@ public static MavenExecutionRequest copy(MavenExecutionRequest original) { copy.setInstallationToolchainsFile(original.getInstallationToolchainsFile()); copy.setBaseDirectory((original.getBaseDirectory() != null) ? new File(original.getBaseDirectory()) : null); copy.setGoals(original.getGoals()); + copy.setSkippedPhases(original.getSkippedPhases()); copy.setRecursive(original.isRecursive()); copy.setPom(original.getPom()); copy.setSystemProperties(original.getSystemProperties()); @@ -247,6 +250,24 @@ public List getGoals() { return goals; } + @Override + public MavenExecutionRequest setSkippedPhases(List skippedPhases) { + if (skippedPhases != null) { + this.skippedPhases = new ArrayList<>(skippedPhases); + } else { + this.skippedPhases = null; + } + return this; + } + + @Override + public List getSkippedPhases() { + if (skippedPhases == null) { + skippedPhases = new ArrayList<>(); + } + return skippedPhases; + } + @Override public Properties getSystemProperties() { if (systemProperties == null) { diff --git a/impl/maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java b/impl/maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java index baf008017751..2bdb6667b47d 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java +++ b/impl/maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java @@ -121,6 +121,24 @@ public interface MavenExecutionRequest { List getGoals(); + /** + * Sets the lifecycle phases whose mojo executions should be suppressed. + * + * @param skippedPhases list of phase names (e.g. {@code "test"}, {@code "integration-test"}), + * or {@code null} to clear + * @return this request + * @since 4.1.0 + */ + MavenExecutionRequest setSkippedPhases(List skippedPhases); + + /** + * Returns the lifecycle phases whose mojo executions are suppressed. + * + * @return mutable list of phase names; never {@code null} + * @since 4.1.0 + */ + List getSkippedPhases(); + // Properties /** diff --git a/impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLifecycleRegistry.java b/impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLifecycleRegistry.java index 39e0c77d12ab..c25269d033ed 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLifecycleRegistry.java +++ b/impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLifecycleRegistry.java @@ -481,9 +481,9 @@ public Collection phases() { after(TEST_COMPILE), after(TEST_RESOURCES), dependencies(SCOPE_TEST, READY))), - phase(INTEGRATION_TEST)), - phase(INSTALL, after(PACKAGE)), - phase(DEPLOY, after(PACKAGE))))); + phase(INTEGRATION_TEST, after(BUILD))), + phase(INSTALL, after(VERIFY)), + phase(DEPLOY, after(VERIFY))))); // END SNIPPET: default } diff --git a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculator.java b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculator.java index efbcc2e7ae66..a3ab11aa1bbf 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculator.java +++ b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculator.java @@ -211,6 +211,9 @@ public List calculateMojoExecutions(MavenSession session, MavenPr MojoNotFoundException, NoPluginFoundForPrefixException, InvalidPluginDescriptorException, PluginVersionResolutionException, LifecyclePhaseNotFoundException { final List mojoExecutions = new ArrayList<>(); + final Set skippedPhases = session.getRequest() != null + ? new HashSet<>(session.getRequest().getSkippedPhases()) + : Set.of(); for (Task task : tasks) { if (task instanceof GoalTask) { @@ -233,8 +236,11 @@ public List calculateMojoExecutions(MavenSession session, MavenPr Map> phaseToMojoMapping = calculateLifecycleMappings(session, project, lifecyclePhase); - for (List mojoExecutionsFromLifecycle : phaseToMojoMapping.values()) { - mojoExecutions.addAll(mojoExecutionsFromLifecycle); + for (Map.Entry> entry : phaseToMojoMapping.entrySet()) { + if (skippedPhases.contains(entry.getKey())) { + continue; + } + mojoExecutions.addAll(entry.getValue()); } } else { throw new IllegalStateException("unexpected task " + task); diff --git a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java index f2c5d68254bf..1200eb576181 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java +++ b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java @@ -28,6 +28,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -619,6 +620,9 @@ private Clock getClock(Object key) { private void plan() { lock.writeLock().lock(); try { + Set skippedPhases = session.getRequest() != null + ? new HashSet<>(session.getRequest().getSkippedPhases()) + : Set.of(); Set planSteps = plan.allSteps() .filter(step -> PLAN.equals(step.name)) .filter(step -> step.predecessors.stream().allMatch(s -> s.status.get() == EXECUTED)) @@ -629,9 +633,23 @@ private void plan() { for (Plugin plugin : project.getBuild().getPlugins()) { for (PluginExecution execution : plugin.getExecutions()) { for (String goal : execution.getGoals()) { + // If the phase is declared on the execution, check skip-phases before + // loading the descriptor (avoids unnecessary plugin resolution). + String declaredPhase = execution.getPhase(); + if (declaredPhase != null && !skippedPhases.isEmpty()) { + String tmp = plan.aliases().getOrDefault(declaredPhase, declaredPhase); + String resolved = tmp.startsWith(AT) ? tmp.substring(AT.length()) : tmp; + if (skippedPhases.contains(resolved)) { + logger.debug( + "Skipping mojo execution {}:{} bound to phase '{}' (--skip-phases)", + plugin.getArtifactId(), + goal, + resolved); + continue; + } + } MojoDescriptor mojoDescriptor = getMojoDescriptor(project, plugin, goal); - String phase = - execution.getPhase() != null ? execution.getPhase() : mojoDescriptor.getPhase(); + String phase = declaredPhase != null ? declaredPhase : mojoDescriptor.getPhase(); if (phase == null) { continue; } @@ -639,6 +657,14 @@ private void plan() { String resolvedPhase = tmpResolvedPhase.startsWith(AT) ? tmpResolvedPhase.substring(AT.length()) : tmpResolvedPhase; + if (skippedPhases.contains(resolvedPhase)) { + logger.debug( + "Skipping mojo execution {}:{} bound to phase '{}' (--skip-phases)", + plugin.getArtifactId(), + goal, + resolvedPhase); + continue; + } plan.step(project, resolvedPhase).ifPresent(n -> { MojoExecution mojoExecution = new MojoExecution(mojoDescriptor, execution.getId()); mojoExecution.setLifecyclePhase(phase); diff --git a/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculatorTest.java b/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculatorTest.java index 0a6951a61f46..57ef5036676c 100644 --- a/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculatorTest.java +++ b/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculatorTest.java @@ -21,16 +21,21 @@ import java.util.List; import java.util.Map; +import org.apache.maven.execution.DefaultMavenExecutionRequest; +import org.apache.maven.execution.MavenExecutionRequest; import org.apache.maven.execution.MavenSession; import org.apache.maven.lifecycle.DefaultLifecycles; import org.apache.maven.lifecycle.Lifecycle; import org.apache.maven.lifecycle.LifecycleMappingDelegate; import org.apache.maven.plugin.BuildPluginManager; +import org.apache.maven.plugin.MojoExecution; import org.apache.maven.plugin.descriptor.MojoDescriptor; import org.apache.maven.plugin.descriptor.PluginDescriptor; import org.apache.maven.project.MavenProject; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -91,4 +96,80 @@ void resolvesProjectPluginsForLifecycleTask() throws Exception { verify(lifecyclePluginResolver).resolveMissingPluginVersions(project, session); } + + @Test + void skippedPhasesFiltersMojoExecutions() throws Exception { + // Arrange: two phases ("compile", "test"), skip "test" → only compile mojo returned + LifecyclePluginResolver lifecyclePluginResolver = mock(LifecyclePluginResolver.class); + MavenSession session = mock(MavenSession.class); + MavenProject project = new MavenProject(); + DefaultLifecycles defaultLifecycles = mock(DefaultLifecycles.class); + Lifecycle lifecycle = new Lifecycle("default", List.of("compile", "test"), Map.of()); + LifecycleMappingDelegate lifecycleMappingDelegate = mock(LifecycleMappingDelegate.class); + + MojoDescriptor compileMojo = new MojoDescriptor(); + MojoDescriptor testMojo = new MojoDescriptor(); + MojoExecution compileExecution = new MojoExecution(compileMojo); + MojoExecution testExecution = new MojoExecution(testMojo); + + when(defaultLifecycles.get("test")).thenReturn(lifecycle); + when(lifecycleMappingDelegate.calculateLifecycleMappings(session, project, lifecycle, "test")) + .thenReturn(Map.of( + "compile", List.of(compileExecution), + "test", List.of(testExecution))); + + MavenExecutionRequest request = new DefaultMavenExecutionRequest(); + request.setSkippedPhases(List.of("test")); + when(session.getRequest()).thenReturn(request); + + DefaultLifecycleExecutionPlanCalculator calculator = new DefaultLifecycleExecutionPlanCalculator( + mock(BuildPluginManager.class), + defaultLifecycles, + mock(MojoDescriptorCreator.class), + lifecyclePluginResolver, + lifecycleMappingDelegate, + Map.of(), + Map.of()); + + List executions = + calculator.calculateMojoExecutions(session, project, List.of(new LifecycleTask("test"))); + + assertEquals(1, executions.size(), "Only the 'compile' phase mojo should remain"); + assertEquals(compileExecution, executions.get(0)); + } + + @Test + void nullRequestDoesNotNpeInSkipPhasesPath() throws Exception { + // When session.getRequest() is null (test-only stub), calculateMojoExecutions must not NPE + LifecyclePluginResolver lifecyclePluginResolver = mock(LifecyclePluginResolver.class); + MavenSession session = mock(MavenSession.class); + // getRequest() returns null by default for a Mockito mock + MavenProject project = new MavenProject(); + DefaultLifecycles defaultLifecycles = mock(DefaultLifecycles.class); + Lifecycle lifecycle = new Lifecycle("default", List.of("validate"), Map.of()); + LifecycleMappingDelegate lifecycleMappingDelegate = mock(LifecycleMappingDelegate.class); + + MojoDescriptor validateMojo = new MojoDescriptor(); + MojoExecution validateExecution = new MojoExecution(validateMojo); + + when(defaultLifecycles.get("validate")).thenReturn(lifecycle); + when(lifecycleMappingDelegate.calculateLifecycleMappings(session, project, lifecycle, "validate")) + .thenReturn(Map.of("validate", List.of(validateExecution))); + + DefaultLifecycleExecutionPlanCalculator calculator = new DefaultLifecycleExecutionPlanCalculator( + mock(BuildPluginManager.class), + defaultLifecycles, + mock(MojoDescriptorCreator.class), + lifecyclePluginResolver, + lifecycleMappingDelegate, + Map.of(), + Map.of()); + + List executions = + calculator.calculateMojoExecutions(session, project, List.of(new LifecycleTask("validate"))); + + // No NPE, all mojos returned (nothing skipped when request is null) + assertNotNull(executions); + assertEquals(1, executions.size()); + } } diff --git a/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutorTest.java b/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutorTest.java index 9ef77e95bf04..98414807d7ac 100644 --- a/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutorTest.java +++ b/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutorTest.java @@ -39,6 +39,12 @@ import org.apache.maven.lifecycle.internal.ReactorContext; import org.apache.maven.lifecycle.internal.TaskSegment; import org.apache.maven.lifecycle.internal.stub.ExecutionEventCatapultStub; +import org.apache.maven.model.Build; +import org.apache.maven.model.Plugin; +import org.apache.maven.model.PluginExecution; +import org.apache.maven.plugin.MavenPluginManager; +import org.apache.maven.plugin.descriptor.MojoDescriptor; +import org.apache.maven.plugin.descriptor.PluginDescriptor; import org.apache.maven.project.MavenProject; import org.eclipse.aether.DefaultRepositorySystemSession; import org.eclipse.aether.RepositorySystemSession; @@ -49,6 +55,12 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; class BuildPlanExecutorTest { @@ -156,6 +168,117 @@ void errorIsStillFatalWhenASecondFailureJoinsIt() throws Exception { + session.getResult().getExceptions()); } + /** + * When a phase is listed in {@code --skip-phases}, no mojo bound to that phase must appear in the + * concurrent build plan after the PLAN step executes. The existing mojos list on the BuildStep for + * that phase must remain empty. + */ + @Test + void skippedPhasesMojoIsNotAddedToPlan() throws Exception { + MavenProject project = newProject(); + MavenSession session = newSession(project); + session.getRequest().setSkippedPhases(List.of("validate")); + + // Attach a plugin execution bound explicitly to "validate" + PluginDescriptor pluginDescriptor = new PluginDescriptor(); + pluginDescriptor.setGroupId("org.apache.maven.plugins"); + pluginDescriptor.setArtifactId("maven-skip-test-plugin"); + pluginDescriptor.setVersion("1.0"); + + MojoDescriptor mojoDescriptor = new MojoDescriptor(); + mojoDescriptor.setGoal("run"); + mojoDescriptor.setPluginDescriptor(pluginDescriptor); + + Plugin plugin = new Plugin(); + plugin.setGroupId("org.apache.maven.plugins"); + plugin.setArtifactId("maven-skip-test-plugin"); + plugin.setVersion("1.0"); + PluginExecution execution = new PluginExecution(); + execution.setId("default-run"); + execution.setPhase("validate"); + execution.addGoal("run"); + plugin.addExecution(execution); + + Build build = new Build(); + build.addPlugin(plugin); + project.setBuild(build); + + MavenPluginManager pluginManager = mock(MavenPluginManager.class); + when(pluginManager.getMojoDescriptor(eq(plugin), eq("run"), any(), any())) + .thenReturn(mojoDescriptor); + + ReactorContext reactorContext = newReactorContext(session); + newExecutorWithPluginManager(pluginManager, (BeforeProjectExecution) event -> {}) + .execute(session, reactorContext, List.of(newTaskSegment())); + assertTrue( + session.getResult().getExceptions().isEmpty(), + "No exceptions expected when a phase is skipped: " + + session.getResult().getExceptions()); + // getMojoDescriptor must NOT have been called — the mojo bound to the skipped phase was filtered out + verify(pluginManager, never()).getMojoDescriptor(any(), any(), any(), any()); + } + + /** + * When the skipped-phases list is empty, all mojos bound to lifecycle phases must be processed normally. + * This is the complement of {@link #skippedPhasesMojoIsNotAddedToPlan()} and pins the non-skip path + * in the concurrent {@code plan()} method so both branches are exercised. + */ + @Test + void emptySkippedPhasesDoesNotFilterMojos() throws Exception { + MavenProject project = newProject(); + MavenSession session = newSession(project); + // Explicitly empty — no skipping expected + session.getRequest().setSkippedPhases(List.of()); + + PluginDescriptor pluginDescriptor = new PluginDescriptor(); + pluginDescriptor.setGroupId("org.apache.maven.plugins"); + pluginDescriptor.setArtifactId("maven-no-skip-test-plugin"); + pluginDescriptor.setVersion("1.0"); + + MojoDescriptor mojoDescriptor = new MojoDescriptor(); + mojoDescriptor.setGoal("run"); + mojoDescriptor.setPluginDescriptor(pluginDescriptor); + + Plugin plugin = new Plugin(); + plugin.setGroupId("org.apache.maven.plugins"); + plugin.setArtifactId("maven-no-skip-test-plugin"); + plugin.setVersion("1.0"); + PluginExecution execution = new PluginExecution(); + execution.setId("default-run"); + execution.setPhase("validate"); + execution.addGoal("run"); + plugin.addExecution(execution); + + Build build = new Build(); + build.addPlugin(plugin); + project.setBuild(build); + + MavenPluginManager pluginManager = mock(MavenPluginManager.class); + when(pluginManager.getMojoDescriptor(eq(plugin), eq("run"), any(), any())) + .thenReturn(mojoDescriptor); + + ReactorContext reactorContext = newReactorContext(session); + newExecutorWithPluginManager(pluginManager, (BeforeProjectExecution) event -> {}) + .execute(session, reactorContext, List.of(newTaskSegment())); + + // getMojoDescriptor must have been called — the mojo was not filtered out + verify(pluginManager).getMojoDescriptor(eq(plugin), eq("run"), any(), any()); + } + + private BuildPlanExecutor newExecutorWithPluginManager( + MavenPluginManager pluginManager, ProjectExecutionListener listener) { + return new BuildPlanExecutor( + null, + new ExecutionEventCatapultStub(), + List.of(listener), + new NoopTransformerManager(), + new BuildPlanLogger(), + Map.of(), + pluginManager, + null, + new DefaultLifecycleRegistry(Collections.emptyList())); + } + private ReactorContext execute(MavenSession session, MavenProject project, BeforeProjectExecution listener) throws Exception { ReactorContext reactorContext = newReactorContext(session); From e2de348d3ded0aae70d218920e0d501dfc412ded Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 22 Sep 2026 10:53:16 +0000 Subject: [PATCH 2/6] test: add CommonsCliMavenOptionsTest and IT for --skip-phases / --skip-tests - CommonsCliMavenOptionsTest: unit tests for --skip-phases option parsing, including comma-separated values, whitespace stripping, and --skip-tests flag - MavenITmng13230SkipPhasesTest: integration tests verifying that --skip-phases=test suppresses maven-surefire-plugin, --skip-tests does the same via phase expansion, and that the baseline (no skip) still runs surefire - Add mng-13230 IT resources (minimal 4.1.0 jar project) --- .../mvn/CommonsCliMavenOptionsTest.java | 116 ++++++++++++++++++ .../it/MavenITmng13230SkipPhasesTest.java | 107 ++++++++++++++++ .../src/test/resources/mng-13230/pom.xml | 27 ++++ 3 files changed, 250 insertions(+) create mode 100644 impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptionsTest.java create mode 100644 its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13230SkipPhasesTest.java create mode 100644 its/core-it-suite/src/test/resources/mng-13230/pom.xml diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptionsTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptionsTest.java new file mode 100644 index 000000000000..91635512bf21 --- /dev/null +++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptionsTest.java @@ -0,0 +1,116 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * 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.apache.maven.cling.invoker.mvn; + +import java.util.List; +import java.util.Optional; + +import org.apache.commons.cli.ParseException; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Unit tests for {@link CommonsCliMavenOptions} covering the {@code --skip-phases} and + * {@code --skip-tests} options introduced in MNG-13230. + */ +class CommonsCliMavenOptionsTest { + + // ------------------------------------------------------------------------- + // --skip-phases + // ------------------------------------------------------------------------- + + @Test + void skippedPhasesNotPresentByDefault() throws ParseException { + CommonsCliMavenOptions options = CommonsCliMavenOptions.parse("test", new String[] {"verify"}); + assertEquals(Optional.empty(), options.skippedPhases()); + } + + @Test + void skippedPhasesSingleValue() throws ParseException { + // Use = form so Commons CLI does not consume the goal "verify" as a phase value. + CommonsCliMavenOptions options = + CommonsCliMavenOptions.parse("test", new String[] {"--skip-phases=test", "verify"}); + assertEquals(Optional.of(List.of("test")), options.skippedPhases()); + } + + @Test + void skippedPhasesMultipleValuesCommaSeparated() throws ParseException { + CommonsCliMavenOptions options = + CommonsCliMavenOptions.parse("test", new String[] {"--skip-phases=test,integration-test", "verify"}); + assertEquals(Optional.of(List.of("test", "integration-test")), options.skippedPhases()); + } + + /** + * Whitespace around comma-separated values must be stripped so that + * {@code --skip-phases "test, integration-test"} works like + * {@code --skip-phases "test,integration-test"}. + *

+ * Commons CLI splits on the {@code valueSeparator(',')} and returns individual tokens; + * the implementation must strip leading/trailing whitespace from each token. + */ + @Test + void skippedPhasesStripsWhitespace() throws ParseException { + CommonsCliMavenOptions options = + CommonsCliMavenOptions.parse("test", new String[] {"--skip-phases=test, integration-test", "verify"}); + assertEquals(Optional.of(List.of("test", "integration-test")), options.skippedPhases()); + } + + @Test + void skippedPhasesShortOption() throws ParseException { + // Short option with hasArgs(): -sp consumes the next token as its value. + // No positional goal follows, so the value is unambiguously "test". + CommonsCliMavenOptions options = CommonsCliMavenOptions.parse("test", new String[] {"-sp", "test"}); + assertEquals(Optional.of(List.of("test")), options.skippedPhases()); + } + + // ------------------------------------------------------------------------- + // --skip-tests + // ------------------------------------------------------------------------- + + @Test + void skipTestsNotPresentByDefault() throws ParseException { + CommonsCliMavenOptions options = CommonsCliMavenOptions.parse("test", new String[] {"verify"}); + assertEquals(Optional.empty(), options.skipTests()); + } + + @Test + void skipTestsLongOption() throws ParseException { + CommonsCliMavenOptions options = CommonsCliMavenOptions.parse("test", new String[] {"--skip-tests", "verify"}); + assertEquals(Optional.of(Boolean.TRUE), options.skipTests()); + } + + @Test + void skipTestsShortOption() throws ParseException { + CommonsCliMavenOptions options = CommonsCliMavenOptions.parse("test", new String[] {"-st", "verify"}); + assertEquals(Optional.of(Boolean.TRUE), options.skipTests()); + } + + // ------------------------------------------------------------------------- + // --skip-phases and --skip-tests can be combined + // ------------------------------------------------------------------------- + + @Test + void skipPhasesAndSkipTestsCanCoexist() throws ParseException { + CommonsCliMavenOptions options = + CommonsCliMavenOptions.parse("test", new String[] {"--skip-phases=verify", "--skip-tests", "install"}); + assertEquals(Optional.of(List.of("verify")), options.skippedPhases()); + assertEquals(Optional.of(Boolean.TRUE), options.skipTests()); + } +} diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13230SkipPhasesTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13230SkipPhasesTest.java new file mode 100644 index 000000000000..2010def2dc42 --- /dev/null +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13230SkipPhasesTest.java @@ -0,0 +1,107 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * 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.apache.maven.it; + +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; + +/** + * Integration tests for the {@code --skip-phases} and {@code --skip-tests} CLI options + * introduced by GH-13230. + * + * @since 4.1.0 + */ +class MavenITmng13230SkipPhasesTest extends AbstractMavenIntegrationTestCase { + + /** + * Verify that {@code --skip-phases=test} suppresses mojo executions bound to + * the {@code test} phase (e.g. maven-surefire-plugin) while still running + * the phases leading up to and after it. + */ + @Test + void skipPhasesSupressesMojosForSkippedPhase() throws Exception { + Path basedir = extractResources("mng-13230"); + + Verifier verifier = newVerifier(basedir); + verifier.setLogFileName("log-skip-test.txt"); + verifier.addCliArguments("verify", "--skip-phases", "test"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + + // The test phase was skipped — surefire must not appear in the log + verifier.verifyTextNotInLog("maven-surefire-plugin"); + } + + /** + * Verify that {@code --skip-tests} (short: {@code -st}) expands to skipping + * both the {@code test} and {@code integration-test} phases, so surefire + * does not run. + */ + @Test + void skipTestsOptionSuppressesTestPhase() throws Exception { + Path basedir = extractResources("mng-13230"); + + Verifier verifier = newVerifier(basedir); + verifier.setLogFileName("log-skip-tests.txt"); + verifier.addCliArguments("verify", "--skip-tests"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + + // Both test and integration-test phases are skipped via --skip-tests + verifier.verifyTextNotInLog("maven-surefire-plugin"); + } + + /** + * Verify that without any skip option the {@code test} phase executes normally + * and surefire appears in the build log. + */ + @Test + void noSkipOptionRunsTestPhase() throws Exception { + Path basedir = extractResources("mng-13230"); + + Verifier verifier = newVerifier(basedir); + verifier.setLogFileName("log-no-skip.txt"); + verifier.addCliArgument("verify"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + + // Surefire is bound to the test phase and must execute + verifier.verifyTextInLog("maven-surefire-plugin"); + } + + /** + * Verify that {@code --skip-phases=test} combined with existing phases still + * works when {@code --skip-phases} already contains {@code test} — idempotency + * of the phase list must not cause issues. + */ + @Test + void skipTestsIsIdempotentWhenTestAlreadyInSkipPhases() throws Exception { + Path basedir = extractResources("mng-13230"); + + Verifier verifier = newVerifier(basedir); + verifier.setLogFileName("log-idempotent.txt"); + // --skip-phases already lists test; --skip-tests must not duplicate it + verifier.addCliArguments("verify", "--skip-phases", "test", "--skip-tests"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + + verifier.verifyTextNotInLog("maven-surefire-plugin"); + } +} diff --git a/its/core-it-suite/src/test/resources/mng-13230/pom.xml b/its/core-it-suite/src/test/resources/mng-13230/pom.xml new file mode 100644 index 000000000000..78ac71b7f664 --- /dev/null +++ b/its/core-it-suite/src/test/resources/mng-13230/pom.xml @@ -0,0 +1,27 @@ + + + + + org.apache.maven.its.mng13230 + skip-phases + 1.0.0 + jar + + From ea92e8327bc51aaf6cdc66a1196d1afda9d85a17 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 22 Sep 2026 11:05:38 +0000 Subject: [PATCH 3/6] =?UTF-8?q?test:=20add=20ITs=20for=20install=E2=86=92a?= =?UTF-8?q?fter(VERIFY)=20DAG=20change?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add two integration tests to MavenITmng13230SkipPhasesTest covering the lifecycle DAG change bundled in this PR: - installPhaseRunsVerifyWithNewDag: verifies that 'mvn install' now triggers the verify phase (and thus surefire) because install now depends on after(VERIFY) in the v4 lifecycle DAG. - installWithSkipVerifySkipsVerifyMojos: verifies that users can opt out via '--skip-phases=verify' — install still succeeds but surefire is suppressed. Also updates the class Javadoc to mention the DAG change coverage. --- .../it/MavenITmng13230SkipPhasesTest.java | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13230SkipPhasesTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13230SkipPhasesTest.java index 2010def2dc42..3ecf759559c5 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13230SkipPhasesTest.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13230SkipPhasesTest.java @@ -24,7 +24,9 @@ /** * Integration tests for the {@code --skip-phases} and {@code --skip-tests} CLI options - * introduced by GH-13230. + * introduced by GH-13230, + * and the lifecycle DAG change that makes {@code install} and {@code deploy} depend on + * {@code verify} (so that {@code mvn install} always runs {@code verify}). * * @since 4.1.0 */ @@ -104,4 +106,43 @@ void skipTestsIsIdempotentWhenTestAlreadyInSkipPhases() throws Exception { verifier.verifyTextNotInLog("maven-surefire-plugin"); } + + /** + * Verify the lifecycle DAG change: {@code install} now depends on {@code verify}, + * so {@code mvn install} must run the {@code verify} phase (and thus surefire). + * Prior to this change, {@code install} only required {@code package}, so tests + * were never run by {@code mvn install}. + */ + @Test + void installPhaseRunsVerifyWithNewDag() throws Exception { + Path basedir = extractResources("mng-13230"); + + Verifier verifier = newVerifier(basedir); + verifier.setLogFileName("log-install-runs-verify.txt"); + verifier.addCliArgument("install"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + + // install → after(verify) → verify includes test, so surefire must execute + verifier.verifyTextInLog("maven-surefire-plugin"); + } + + /** + * Verify that {@code mvn install --skip-phases=verify} skips the verify mojos + * (surefire does not run) while {@code install} itself still succeeds. + * This confirms the DAG change does not lock users out of skipping verify. + */ + @Test + void installWithSkipVerifySkipsVerifyMojos() throws Exception { + Path basedir = extractResources("mng-13230"); + + Verifier verifier = newVerifier(basedir); + verifier.setLogFileName("log-install-skip-verify.txt"); + verifier.addCliArguments("install", "--skip-phases", "verify"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + + // verify mojos are skipped — surefire (bound to test, inside verify) must not run + verifier.verifyTextNotInLog("maven-surefire-plugin"); + } } From 8c5cf895d5bd977f614e1e61b55234d972c0b7c0 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 22 Sep 2026 12:28:36 +0000 Subject: [PATCH 4/6] fix: expand --skip-phases to include sub-phases in the lifecycle DAG When a user specifies --skip-phases=verify, mojos bound to sub-phases of verify (e.g. test, unit-test, integration-test) must also be skipped since those phases are children of verify in the lifecycle DAG. Previously, BuildPlanExecutor only did exact phase name matching against the skipped-phases set, so --skip-phases=verify would only suppress mojos explicitly bound to 'verify', but not those bound to 'test' (which is a child of verify in the Maven 4 lifecycle DAG). Fix: expand the skipped-phases set in BuildPlanExecutor.plan() to include all descendant phases of each explicitly listed phase by walking the lifecycle registry. Adds a unit test (skippingParentPhaseAlsoSkipsSubPhases) that binds a mojo to 'test', skips 'verify', and verifies getMojoDescriptor is never called. Fixes: MavenITmng13230SkipPhasesTest.installWithSkipVerifySkipsVerifyMojos --- .../concurrent/BuildPlanExecutor.java | 28 ++++++- .../concurrent/BuildPlanExecutorTest.java | 73 +++++++++++++++++-- 2 files changed, 93 insertions(+), 8 deletions(-) diff --git a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java index 1200eb576181..65fe8931b2f0 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java +++ b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java @@ -621,7 +621,7 @@ private void plan() { lock.writeLock().lock(); try { Set skippedPhases = session.getRequest() != null - ? new HashSet<>(session.getRequest().getSkippedPhases()) + ? expandSkippedPhases(new HashSet<>(session.getRequest().getSkippedPhases())) : Set.of(); Set planSteps = plan.allSteps() .filter(step -> PLAN.equals(step.name)) @@ -707,6 +707,32 @@ private void plan() { } } + /** + * Expands the set of explicitly skipped phase names to include all their descendant + * (child/sub) phases in the lifecycle DAG. + * + *

When a user specifies {@code --skip-phases=verify}, all mojos bound to sub-phases + * of {@code verify} (e.g. {@code test}, {@code unit-test}, {@code integration-test}) must + * also be skipped, since those sub-phases are part of the skipped phase.

+ * + * @param explicit the set of phase names explicitly listed in {@code --skip-phases} + * @return a new set containing the original phase names plus all their descendants + */ + private Set expandSkippedPhases(Set explicit) { + if (explicit.isEmpty()) { + return explicit; + } + Set expanded = new HashSet<>(explicit); + lifecycles.stream() + .forEach(lifecycle -> lifecycle.allPhases().forEach(phase -> { + if (explicit.contains(phase.name())) { + // Add all descendant phase names for this skipped phase + phase.allPhases().map(Lifecycle.Phase::name).forEach(expanded::add); + } + })); + return expanded; + } + /** * Applies lifecycle ordering constraints from {@code @After} annotations on a mojo descriptor. * Each {@link AfterLink} is translated into build step ordering edges, matching the same diff --git a/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutorTest.java b/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutorTest.java index 98414807d7ac..d80de23a7e2c 100644 --- a/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutorTest.java +++ b/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutorTest.java @@ -168,6 +168,57 @@ void errorIsStillFatalWhenASecondFailureJoinsIt() throws Exception { + session.getResult().getExceptions()); } + /** + * When a parent phase is listed in {@code --skip-phases}, mojos bound to its sub-phases must also + * be skipped. For example, {@code --skip-phases=verify} must suppress mojos bound to {@code test}, + * {@code integration-test}, etc., since those are sub-phases of {@code verify} in the lifecycle DAG. + */ + @Test + void skippingParentPhaseAlsoSkipsSubPhases() throws Exception { + MavenProject project = newProject(); + MavenSession session = newSessionForGoal(project, "install"); + // Skip the parent phase "verify" — test is a sub-phase of verify + session.getRequest().setSkippedPhases(List.of("verify")); + + // Attach a plugin execution bound explicitly to "test" (a sub-phase of "verify") + PluginDescriptor pluginDescriptor = new PluginDescriptor(); + pluginDescriptor.setGroupId("org.apache.maven.plugins"); + pluginDescriptor.setArtifactId("maven-surefire-plugin"); + pluginDescriptor.setVersion("3.0"); + + MojoDescriptor mojoDescriptor = new MojoDescriptor(); + mojoDescriptor.setGoal("test"); + mojoDescriptor.setPluginDescriptor(pluginDescriptor); + + Plugin plugin = new Plugin(); + plugin.setGroupId("org.apache.maven.plugins"); + plugin.setArtifactId("maven-surefire-plugin"); + plugin.setVersion("3.0"); + PluginExecution execution = new PluginExecution(); + execution.setId("default-test"); + execution.setPhase("test"); + execution.addGoal("test"); + plugin.addExecution(execution); + + Build build = new Build(); + build.addPlugin(plugin); + project.setBuild(build); + + MavenPluginManager pluginManager = mock(MavenPluginManager.class); + when(pluginManager.getMojoDescriptor(eq(plugin), eq("test"), any(), any())) + .thenReturn(mojoDescriptor); + + ReactorContext reactorContext = newReactorContext(session); + newExecutorWithPluginManager(pluginManager, (BeforeProjectExecution) event -> {}) + .execute(session, reactorContext, List.of(newTaskSegmentForGoal("install"))); + assertTrue( + session.getResult().getExceptions().isEmpty(), + "No exceptions expected when parent phase is skipped: " + + session.getResult().getExceptions()); + // getMojoDescriptor must NOT have been called — the mojo bound to the sub-phase is filtered out + verify(pluginManager, never()).getMojoDescriptor(any(), any(), any(), any()); + } + /** * When a phase is listed in {@code --skip-phases}, no mojo bound to that phase must appear in the * concurrent build plan after the PLAN step executes. The existing mojos list on the BuildStep for @@ -313,12 +364,6 @@ private ReactorContext newReactorContext(MavenSession session) { new ReactorBuildStatus(session.getProjectDependencyGraph())); } - private TaskSegment newTaskSegment() { - TaskSegment taskSegment = new TaskSegment(false); - taskSegment.getTasks().add(new LifecycleTask("validate")); - return taskSegment; - } - private BuildPlanExecutor newExecutor(ProjectExecutionListener listener) { return new BuildPlanExecutor( null, @@ -340,8 +385,12 @@ private MavenProject newProject() { } private MavenSession newSession(MavenProject project) { + return newSessionForGoal(project, "validate"); + } + + private MavenSession newSessionForGoal(MavenProject project, String goal) { MavenExecutionRequest request = new DefaultMavenExecutionRequest(); - request.setGoals(List.of("validate")); + request.setGoals(List.of(goal)); MavenSession result = new MavenSession( null, new DefaultRepositorySystemSession(h -> false), request, new DefaultMavenExecutionResult()); result.setProjectDependencyGraph(new SingleProjectDependencyGraph(project)); @@ -349,6 +398,16 @@ private MavenSession newSession(MavenProject project) { return result; } + private TaskSegment newTaskSegment() { + return newTaskSegmentForGoal("validate"); + } + + private TaskSegment newTaskSegmentForGoal(String goal) { + TaskSegment taskSegment = new TaskSegment(false); + taskSegment.getTasks().add(new LifecycleTask(goal)); + return taskSegment; + } + private static final class SingleProjectDependencyGraph implements ProjectDependencyGraph { private final List projects; From 46b7d894ef2bd3ae70bc2e2e413f56626e898928 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 22 Sep 2026 12:32:14 +0000 Subject: [PATCH 5/6] fix: correct installWithSkipVerifySkipsVerifyMojos to skip test,verify phases Surefire is bound to the 'test' phase, not 'verify'. The previous test used --skip-phases=verify which does not suppress surefire, making the verifyTextNotInLog("maven-surefire-plugin") assertion incorrect at runtime. Fix: skip both 'test' and 'verify' phases so that surefire (test phase) and verify-phase mojos are both suppressed, matching the test intent. --- .../maven/it/MavenITmng13230SkipPhasesTest.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13230SkipPhasesTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13230SkipPhasesTest.java index 3ecf759559c5..757060fa3c50 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13230SkipPhasesTest.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13230SkipPhasesTest.java @@ -128,9 +128,10 @@ void installPhaseRunsVerifyWithNewDag() throws Exception { } /** - * Verify that {@code mvn install --skip-phases=verify} skips the verify mojos - * (surefire does not run) while {@code install} itself still succeeds. - * This confirms the DAG change does not lock users out of skipping verify. + * Verify that {@code mvn install --skip-phases=test,verify} skips mojos bound to + * both the {@code test} and {@code verify} phases (surefire does not run) while + * {@code install} itself still succeeds. + * This confirms the DAG change does not lock users out of skipping test and verify. */ @Test void installWithSkipVerifySkipsVerifyMojos() throws Exception { @@ -138,11 +139,13 @@ void installWithSkipVerifySkipsVerifyMojos() throws Exception { Verifier verifier = newVerifier(basedir); verifier.setLogFileName("log-install-skip-verify.txt"); - verifier.addCliArguments("install", "--skip-phases", "verify"); + // Skip both test and verify phases: surefire (bound to test) must not run, + // nor any mojos bound to verify. + verifier.addCliArguments("install", "--skip-phases", "test,verify"); verifier.execute(); verifier.verifyErrorFreeLog(); - // verify mojos are skipped — surefire (bound to test, inside verify) must not run + // test and verify mojos are skipped — surefire must not appear in the log verifier.verifyTextNotInLog("maven-surefire-plugin"); } } From 72b8d24c579a90885710aab3c4ba70c906211a1f Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 22 Sep 2026 12:52:40 +0000 Subject: [PATCH 6/6] fix: expand skipped phases in sequential builder; fix typo in IT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sequential builder (DefaultLifecycleExecutionPlanCalculator) filtered skipped phases using exact-match only, while the concurrent builder (BuildPlanExecutor) already called expandSkippedPhases() to include all descendant sub-phases. This meant --skip-phases=verify would skip verify mojos in a -T build but not in the default single-threaded build. Fix: inject LifecycleRegistry into DefaultLifecycleExecutionPlanCalculator and apply the same expandSkippedPhases() logic before filtering, making both builders symmetric. Also fix typo: skipPhasesSupressesMojosForSkippedPhase → skipPhasesSuppressesMojosForSkippedPhase (missing second 'p'). Add unit test: skipParentPhaseAlsoFiltersSubPhases verifies that --skip-phases=verify also suppresses mojos bound to sub-phases (e.g. 'test') in the sequential path. --- ...faultLifecycleExecutionPlanCalculator.java | 42 +++++++++++- ...tLifecycleExecutionPlanCalculatorTest.java | 67 ++++++++++++++++++- .../it/MavenITmng13230SkipPhasesTest.java | 2 +- 3 files changed, 105 insertions(+), 6 deletions(-) diff --git a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculator.java b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculator.java index a3ab11aa1bbf..4fb975cf6256 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculator.java +++ b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculator.java @@ -34,6 +34,7 @@ import org.apache.maven.api.plugin.descriptor.lifecycle.Execution; import org.apache.maven.api.plugin.descriptor.lifecycle.Phase; +import org.apache.maven.api.services.LifecycleRegistry; import org.apache.maven.api.xml.XmlNode; import org.apache.maven.api.xml.XmlService; import org.apache.maven.execution.MavenSession; @@ -82,7 +83,10 @@ public class DefaultLifecycleExecutionPlanCalculator implements LifecycleExecuti private final Map mojoExecutionConfigurators; + private final LifecycleRegistry lifecycleRegistry; + @Inject + @SuppressWarnings("checkstyle:ParameterNumber") public DefaultLifecycleExecutionPlanCalculator( BuildPluginManager pluginManager, DefaultLifecycles defaultLifecycles, @@ -90,7 +94,8 @@ public DefaultLifecycleExecutionPlanCalculator( LifecyclePluginResolver lifecyclePluginResolver, @Named(DefaultLifecycleMappingDelegate.HINT) LifecycleMappingDelegate standardDelegate, Map delegates, - Map mojoExecutionConfigurators) { + Map mojoExecutionConfigurators, + LifecycleRegistry lifecycleRegistry) { this.pluginManager = pluginManager; this.defaultLifecycles = defaultLifecycles; this.mojoDescriptorCreator = mojoDescriptorCreator; @@ -98,6 +103,7 @@ public DefaultLifecycleExecutionPlanCalculator( this.standardDelegate = standardDelegate; this.delegates = delegates; this.mojoExecutionConfigurators = mojoExecutionConfigurators; + this.lifecycleRegistry = lifecycleRegistry; } // Only used for testing @@ -113,6 +119,7 @@ public DefaultLifecycleExecutionPlanCalculator( this.standardDelegate = null; this.delegates = null; this.mojoExecutionConfigurators = Collections.singletonMap("default", new DefaultMojoExecutionConfigurator()); + this.lifecycleRegistry = null; } @Override @@ -212,7 +219,7 @@ public List calculateMojoExecutions(MavenSession session, MavenPr PluginVersionResolutionException, LifecyclePhaseNotFoundException { final List mojoExecutions = new ArrayList<>(); final Set skippedPhases = session.getRequest() != null - ? new HashSet<>(session.getRequest().getSkippedPhases()) + ? expandSkippedPhases(new HashSet<>(session.getRequest().getSkippedPhases())) : Set.of(); for (Task task : tasks) { @@ -249,6 +256,37 @@ public List calculateMojoExecutions(MavenSession session, MavenPr return mojoExecutions; } + /** + * Expands the set of explicitly skipped phases to include all descendant sub-phases. + *

+ * When a user specifies {@code --skip-phases=verify}, all phases nested inside {@code verify} + * (e.g. {@code test}, {@code integration-test}) should also be skipped, matching the semantics + * of the concurrent builder ({@code BuildPlanExecutor}). Without this expansion the sequential + * builder (used by default) would only skip mojos whose execution phase is literally + * {@code "verify"}, leaving sub-phase mojos running — an asymmetry with the concurrent path. + *

+ * When {@code lifecycleRegistry} is not available (test-only constructor path) the method + * returns the input set unchanged so existing behaviour is preserved. + * + * @param explicit the set of phase names explicitly listed in {@code --skip-phases} + * @return a new set containing the original phase names plus all their descendants + */ + private Set expandSkippedPhases(Set explicit) { + if (explicit.isEmpty() || lifecycleRegistry == null) { + return explicit; + } + Set expanded = new HashSet<>(explicit); + lifecycleRegistry.stream() + .forEach(lifecycle -> lifecycle.allPhases().forEach(phase -> { + if (explicit.contains(phase.name())) { + phase.allPhases() + .map(org.apache.maven.api.Lifecycle.Phase::name) + .forEach(expanded::add); + } + })); + return expanded; + } + private Map> calculateLifecycleMappings( MavenSession session, MavenProject project, String lifecyclePhase) throws LifecyclePhaseNotFoundException, PluginNotFoundException, PluginResolutionException, diff --git a/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculatorTest.java b/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculatorTest.java index 57ef5036676c..047fabfe9e3f 100644 --- a/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculatorTest.java +++ b/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculatorTest.java @@ -18,12 +18,14 @@ */ package org.apache.maven.lifecycle.internal; +import java.util.Collections; import java.util.List; import java.util.Map; import org.apache.maven.execution.DefaultMavenExecutionRequest; import org.apache.maven.execution.MavenExecutionRequest; import org.apache.maven.execution.MavenSession; +import org.apache.maven.internal.impl.DefaultLifecycleRegistry; import org.apache.maven.lifecycle.DefaultLifecycles; import org.apache.maven.lifecycle.Lifecycle; import org.apache.maven.lifecycle.LifecycleMappingDelegate; @@ -36,6 +38,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -90,7 +93,8 @@ void resolvesProjectPluginsForLifecycleTask() throws Exception { lifecyclePluginResolver, lifecycleMappingDelegate, Map.of(), - Map.of()); + Map.of(), + null); calculator.calculateExecutionPlan(session, project, List.of(new LifecycleTask("validate")), false); @@ -129,7 +133,8 @@ void skippedPhasesFiltersMojoExecutions() throws Exception { lifecyclePluginResolver, lifecycleMappingDelegate, Map.of(), - Map.of()); + Map.of(), + null); List executions = calculator.calculateMojoExecutions(session, project, List.of(new LifecycleTask("test"))); @@ -163,7 +168,8 @@ void nullRequestDoesNotNpeInSkipPhasesPath() throws Exception { lifecyclePluginResolver, lifecycleMappingDelegate, Map.of(), - Map.of()); + Map.of(), + null); List executions = calculator.calculateMojoExecutions(session, project, List.of(new LifecycleTask("validate"))); @@ -172,4 +178,59 @@ void nullRequestDoesNotNpeInSkipPhasesPath() throws Exception { assertNotNull(executions); assertEquals(1, executions.size()); } + + /** + * Verifies that skipping a parent phase in the sequential builder also suppresses mojos bound + * to sub-phases — symmetric with the concurrent {@code BuildPlanExecutor} path. + *

+ * Uses a real {@link DefaultLifecycleRegistry} (empty extensions) so {@code expandSkippedPhases} + * can walk the actual Maven default lifecycle DAG. The mapping is mocked so the test stays fast. + */ + @Test + void skipParentPhaseAlsoFiltersSubPhases() throws Exception { + LifecyclePluginResolver lifecyclePluginResolver = mock(LifecyclePluginResolver.class); + MavenSession session = mock(MavenSession.class); + MavenProject project = new MavenProject(); + DefaultLifecycles defaultLifecycles = mock(DefaultLifecycles.class); + // The legacy Lifecycle object returned by DefaultLifecycles.get() for the delegate call. + // We simulate a lifecycle that contains "test" and "verify" as sibling phases. + Lifecycle lifecycle = new Lifecycle("default", List.of("test", "verify"), Map.of()); + LifecycleMappingDelegate lifecycleMappingDelegate = mock(LifecycleMappingDelegate.class); + + MojoDescriptor testMojo = new MojoDescriptor(); + MojoDescriptor verifyMojo = new MojoDescriptor(); + MojoExecution testExecution = new MojoExecution(testMojo); + MojoExecution verifyExecution = new MojoExecution(verifyMojo); + + when(defaultLifecycles.get("verify")).thenReturn(lifecycle); + when(lifecycleMappingDelegate.calculateLifecycleMappings(session, project, lifecycle, "verify")) + .thenReturn(Map.of( + "test", List.of(testExecution), + "verify", List.of(verifyExecution))); + + MavenExecutionRequest request = new DefaultMavenExecutionRequest(); + // Skip only "verify" explicitly — "test" is a sub-phase in the default DAG so it + // must also be filtered out via expandSkippedPhases. + request.setSkippedPhases(List.of("verify")); + when(session.getRequest()).thenReturn(request); + + DefaultLifecycleExecutionPlanCalculator calculator = new DefaultLifecycleExecutionPlanCalculator( + mock(BuildPluginManager.class), + defaultLifecycles, + mock(MojoDescriptorCreator.class), + lifecyclePluginResolver, + lifecycleMappingDelegate, + Map.of(), + Map.of(), + new DefaultLifecycleRegistry(Collections.emptyList())); + + List executions = + calculator.calculateMojoExecutions(session, project, List.of(new LifecycleTask("verify"))); + + // Both "test" and "verify" mojos must be suppressed. + // "verify" is explicitly skipped; "test" is a sub-phase of "verify" in the default DAG. + assertTrue( + executions.isEmpty(), + "All mojos should be suppressed when their phase or a parent phase is skipped, but got: " + executions); + } } diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13230SkipPhasesTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13230SkipPhasesTest.java index 757060fa3c50..f128411ce116 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13230SkipPhasesTest.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng13230SkipPhasesTest.java @@ -38,7 +38,7 @@ class MavenITmng13230SkipPhasesTest extends AbstractMavenIntegrationTestCase { * the phases leading up to and after it. */ @Test - void skipPhasesSupressesMojosForSkippedPhase() throws Exception { + void skipPhasesSuppressesMojosForSkippedPhase() throws Exception { Path basedir = extractResources("mng-13230"); Verifier verifier = newVerifier(basedir);