Skip to content

Fixes #13135: ensure completeness of the reactor summary, privilege f… #13173

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gnodet merged 2 commits into from
Sep 18, 2026
+193 −35
Merged

Fixes #13135: ensure completeness of the reactor summary, privilege f… #13173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6985d8a
Fixes #13135: ensure completeness of the reactor summary, privilege f…
rmannibucau Sep 17, 2026
1794551
Address review: local StringBuilder, UNKNOWN group=2
gnodet Sep 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 89 additions & 25 deletions maven-embedder/src/main/java/org/apache/maven/cli/event/ExecutionEventLogger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import java.io.File;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

Expand Down Expand Up @@ -170,39 +171,48 @@ private void logReactorSummary(MavenSession session) {
String failureMessage = builder().failure("FAILURE").build();
String unknownMessage = builder().warning("UNKNOWN").build();

boolean lastWasSkipped = false;
List<ReactorSummaryEntry> entries = new ArrayList<>(projects.size());
for (MavenProject project : projects) {
BuildSummary buildSummary = result.getBuildSummary(project);

String statusMessage;
boolean shouldSkip = result.hasExceptions();
if (buildSummary == null) {
statusMessage = skippedMessage;
} else if (buildSummary instanceof BuildSuccess) {
int group;
if (buildSummary instanceof BuildSuccess) {
statusMessage = successMessage;
group = 1;
} else if (buildSummary instanceof BuildFailure) {
statusMessage = failureMessage;
shouldSkip = false;
group = 2;
} else if (buildSummary == null) {
statusMessage = skippedMessage;
group = 0;

@gnodet-bot gnodet-bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ UNKNOWN buildSummary silently grouped with SKIPPED — consider group = 2 for safety

BuildSummary is currently abstract with only BuildSuccess and BuildFailure as concrete subclasses, so this branch is dead code in practice. But if a third subclass (e.g. BuildCancelled) is ever added, the result will land in group 0 alongside SKIPPED modules and be logged at info level, with no visual distinction from a module that was never scheduled to run.

Assigning group = 2 makes this branch safe by default at zero cost:

Suggested change
group = 0;
statusMessage = unknownMessage;
group = 2;

Same concern applies to the second else branch at line 191.

} else {
statusMessage = unknownMessage;
group = 2;
}
entries.add(new ReactorSummaryEntry(project, buildSummary, group, statusMessage));
}

ReactorSummaryRequest request = new ReactorSummaryRequest(entries, isSingleVersion);

if (shouldSkip) {
lastWasSkipped = true;
logReactorSummaryGroup(request, 0);
logReactorSummaryGroup(request, 1);
logReactorSummaryGroup(request, 2);
}

private void logReactorSummaryGroup(ReactorSummaryRequest request, int group) {
StringBuilder buffer = new StringBuilder(128);

for (ReactorSummaryEntry entry : request.getEntries()) {
if (entry.getGroup() != group) {
continue;
}
if (lastWasSkipped) {
logger.info("...");
lastWasSkipped = false;
}

StringBuilder buffer = new StringBuilder(128);

buffer.append(project.getName());
buffer.append(entry.getProject().getName());
buffer.append(' ');

if (!isSingleVersion) {
buffer.append(project.getVersion());
if (!request.isSingleVersion()) {
buffer.append(entry.getProject().getVersion());
buffer.append(' ');
}

Expand All @@ -213,16 +223,65 @@ private void logReactorSummary(MavenSession session) {
buffer.append(' ');
}

buffer.append(statusMessage);
if (buildSummary != null) {
formatBuildTime(buffer, buildSummary);
buffer.append(entry.getStatusMessage());
if (entry.getBuildSummary() != null) {
formatBuildTime(buffer, entry.getBuildSummary());
}

logger.info(buffer.toString());
if (entry.getBuildSummary() instanceof BuildFailure) {
logger.error(buffer.toString());
} else {
logger.info(buffer.toString());
}
buffer.setLength(0);
}
}

private static final class ReactorSummaryRequest {
private final List<ReactorSummaryEntry> entries;
private final boolean singleVersion;

private ReactorSummaryRequest(List<ReactorSummaryEntry> entries, boolean singleVersion) {
this.entries = entries;
this.singleVersion = singleVersion;
}

if (lastWasSkipped) {
logger.info("...");
private List<ReactorSummaryEntry> getEntries() {
return entries;
}

private boolean isSingleVersion() {
return singleVersion;
}
}

private static final class ReactorSummaryEntry {
private final MavenProject project;
private final BuildSummary buildSummary;
private final int group;
private final String statusMessage;

private ReactorSummaryEntry(MavenProject project, BuildSummary buildSummary, int group, String statusMessage) {
this.project = project;
this.buildSummary = buildSummary;
this.group = group;
this.statusMessage = statusMessage;
}

private MavenProject getProject() {
return project;
}

private BuildSummary getBuildSummary() {
return buildSummary;
}

private int getGroup() {
return group;
}

private String getStatusMessage() {
return statusMessage;
}
}

Expand All @@ -241,12 +300,17 @@ private void logResult(MavenSession session) {
infoLine('-');
MessageBuilder buffer = MessageUtils.builder();

if (session.getResult().hasExceptions()) {
boolean failure = session.getResult().hasExceptions();
if (failure) {
buffer.failure("BUILD FAILURE");
} else {
buffer.success("BUILD SUCCESS");
}
logger.info(buffer.toString());
if (failure) {
logger.error(buffer.toString());
} else {
logger.info(buffer.toString());
}
}

private void logStats(MavenSession session) {
Expand Down
114 changes: 104 additions & 10 deletions maven-embedder/src/test/java/org/apache/maven/cli/event/ExecutionEventLoggerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,11 +279,11 @@ public void testSessionEndedFailureMultimodule() {
inOrder.verify(logger).info("------------------------------------------------------------------------");
inOrder.verify(logger).info("Reactor Summary for Maven Project artifact1 1.0.0-SNAPSHOT:");
inOrder.verify(logger).info("");
inOrder.verify(logger).info("...");
inOrder.verify(logger).info("Maven Project artifact2 ............................ FAILURE [ 2.000 s]");
inOrder.verify(logger).info("...");
inOrder.verify(logger).info("Maven Project artifact3 ............................ SKIPPED");
inOrder.verify(logger).info("Maven Project artifact1 ............................ SUCCESS [ 1.000 s]");
inOrder.verify(logger).error("Maven Project artifact2 ............................ FAILURE [ 2.000 s]");
inOrder.verify(logger).info("------------------------------------------------------------------------");
inOrder.verify(logger).info("BUILD FAILURE");
inOrder.verify(logger).error("BUILD FAILURE");
inOrder.verify(logger).info("------------------------------------------------------------------------");
inOrder.verify(logger).info(eq("Total time: {}{}"), anyString(), anyString());
inOrder.verify(logger).info(eq("Finished at: {}"), anyString());
Expand Down Expand Up @@ -335,13 +335,107 @@ public void testSessionEndedFailureMultimoduleWithSeparatedFailures() {
inOrder.verify(logger).info("------------------------------------------------------------------------");
inOrder.verify(logger).info("Reactor Summary for Maven Project artifact1 1.0.0-SNAPSHOT:");
inOrder.verify(logger).info("");
inOrder.verify(logger).info("...");
inOrder.verify(logger).info("Maven Project artifact2 ............................ FAILURE [ 2.000 s]");
inOrder.verify(logger).info("...");
inOrder.verify(logger).info("Maven Project artifact5 ............................ FAILURE [ 5.000 s]");
inOrder.verify(logger).info("...");
inOrder.verify(logger).info("Maven Project artifact6 ............................ SKIPPED");
inOrder.verify(logger).info("Maven Project artifact1 ............................ SUCCESS [ 1.000 s]");
inOrder.verify(logger).info("Maven Project artifact3 ............................ SUCCESS [ 3.000 s]");
inOrder.verify(logger).info("Maven Project artifact4 ............................ SUCCESS [ 4.000 s]");
inOrder.verify(logger).error("Maven Project artifact2 ............................ FAILURE [ 2.000 s]");
inOrder.verify(logger).error("Maven Project artifact5 ............................ FAILURE [ 5.000 s]");
inOrder.verify(logger).info("------------------------------------------------------------------------");
inOrder.verify(logger).error("BUILD FAILURE");
inOrder.verify(logger).info("------------------------------------------------------------------------");
inOrder.verify(logger).info(eq("Total time: {}{}"), anyString(), anyString());
inOrder.verify(logger).info(eq("Finished at: {}"), anyString());
inOrder.verify(logger).info("------------------------------------------------------------------------");
}

@Test
public void testSessionEndedSuccessWithSkippedModules() {
// prepare
MavenProject project1 = aProject("artifact1");
MavenProject project2 = aProject("artifact2");
MavenProject project3 = aProject("artifact3");

MavenExecutionResult executionResult = new DefaultMavenExecutionResult();
executionResult.addBuildSummary(new BuildSuccess(project1, 1000));
executionResult.addBuildSummary(new BuildSuccess(project3, 3000));

MavenExecutionRequest executionRequest = new DefaultMavenExecutionRequest();
executionRequest.setStartTime(new Date());

ProjectDependencyGraph projectDependencyGraph = mock(ProjectDependencyGraph.class);
when(projectDependencyGraph.getSortedProjects()).thenReturn(Arrays.asList(project1, project2, project3));

MavenSession mavenSession = mock(MavenSession.class);
when(mavenSession.getResult()).thenReturn(executionResult);
when(mavenSession.getRequest()).thenReturn(executionRequest);
when(mavenSession.getProjects()).thenReturn(Arrays.asList(project1, project2, project3));
when(mavenSession.getTopLevelProject()).thenReturn(project1);
when(mavenSession.getProjectDependencyGraph()).thenReturn(projectDependencyGraph);

ExecutionEvent event = mock(ExecutionEvent.class);
when(event.getSession()).thenReturn(mavenSession);

// execute
executionEventLogger.sessionEnded(event);

// verify
InOrder inOrder = inOrder(logger);
inOrder.verify(logger).info("------------------------------------------------------------------------");
inOrder.verify(logger).info("Reactor Summary for Maven Project artifact1 1.0.0-SNAPSHOT:");
inOrder.verify(logger).info("");
inOrder.verify(logger).info("Maven Project artifact2 ............................ SKIPPED");
inOrder.verify(logger).info("Maven Project artifact1 ............................ SUCCESS [ 1.000 s]");
inOrder.verify(logger).info("Maven Project artifact3 ............................ SUCCESS [ 3.000 s]");
inOrder.verify(logger).info("------------------------------------------------------------------------");
inOrder.verify(logger).info("BUILD SUCCESS");
inOrder.verify(logger).info("------------------------------------------------------------------------");
inOrder.verify(logger).info(eq("Total time: {}{}"), anyString(), anyString());
inOrder.verify(logger).info(eq("Finished at: {}"), anyString());
inOrder.verify(logger).info("------------------------------------------------------------------------");
}

@Test
public void testSessionEndedFailureMixedWithSkippedModules() {
// prepare
MavenProject project1 = aProject("artifact1");
MavenProject project2 = aProject("artifact2");
MavenProject project3 = aProject("artifact3");

MavenExecutionResult executionResult = new DefaultMavenExecutionResult();
executionResult.addBuildSummary(new BuildSuccess(project1, 1000));
executionResult.addBuildSummary(new BuildSuccess(project3, 3000));
executionResult.addException(new Exception("Failure"));

MavenExecutionRequest executionRequest = new DefaultMavenExecutionRequest();
executionRequest.setStartTime(new Date());

ProjectDependencyGraph projectDependencyGraph = mock(ProjectDependencyGraph.class);
when(projectDependencyGraph.getSortedProjects()).thenReturn(Arrays.asList(project1, project2, project3));

MavenSession mavenSession = mock(MavenSession.class);
when(mavenSession.getResult()).thenReturn(executionResult);
when(mavenSession.getRequest()).thenReturn(executionRequest);
when(mavenSession.getProjects()).thenReturn(Arrays.asList(project1, project2, project3));
when(mavenSession.getTopLevelProject()).thenReturn(project1);
when(mavenSession.getProjectDependencyGraph()).thenReturn(projectDependencyGraph);

ExecutionEvent event = mock(ExecutionEvent.class);
when(event.getSession()).thenReturn(mavenSession);

// execute
executionEventLogger.sessionEnded(event);

// verify
InOrder inOrder = inOrder(logger);
inOrder.verify(logger).info("------------------------------------------------------------------------");
inOrder.verify(logger).info("Reactor Summary for Maven Project artifact1 1.0.0-SNAPSHOT:");
inOrder.verify(logger).info("");
inOrder.verify(logger).info("Maven Project artifact2 ............................ SKIPPED");
inOrder.verify(logger).info("Maven Project artifact1 ............................ SUCCESS [ 1.000 s]");
inOrder.verify(logger).info("Maven Project artifact3 ............................ SUCCESS [ 3.000 s]");
inOrder.verify(logger).info("------------------------------------------------------------------------");
inOrder.verify(logger).info("BUILD FAILURE");
inOrder.verify(logger).error("BUILD FAILURE");
inOrder.verify(logger).info("------------------------------------------------------------------------");
inOrder.verify(logger).info(eq("Total time: {}{}"), anyString(), anyString());
inOrder.verify(logger).info(eq("Finished at: {}"), anyString());
Expand Down
Loading