Skip to content

Fixes #13135, ensure completude of the reactor summary but priviledge failures to be last to stay human efficient #13136

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 3 commits into from
Sep 17, 2026
Merged

Fixes #13135, ensure completude of the reactor summary but priviledge failures to be last to stay human efficient #13136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a36f758
Fixes 13135, ensure completude of the reactor summary but priviledge …
rmannibucau Sep 15, 2026
3ef7ff9
Log reactor summary per-module detail on a nested .detail logger
rmannibucau Sep 15, 2026
75f5320
Always show reactor summary entries, drop extra .detail logger
rmannibucau Sep 15, 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
75 changes: 48 additions & 27 deletions compat/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 @@ -199,44 +200,53 @@ private void logReactorSummary(MavenSession session) {

List<MavenProject> projects = session.getProjects();

StringBuilder buffer = new StringBuilder(128);

String skippedMessage = builder().warning("SKIPPED").build();
String successMessage = builder().success("SUCCESS").build();
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;
} else {
statusMessage = unknownMessage;
group = 0;
Comment thread
rmannibucau marked this conversation as resolved.
Comment thread
rmannibucau marked this conversation as resolved.
}
entries.add(new ReactorSummaryEntry(project, buildSummary, group, statusMessage));
}

ReactorSummaryRequest request = new ReactorSummaryRequest(entries, new StringBuilder(128), isSingleVersion);

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

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

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

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

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

Expand All @@ -247,20 +257,26 @@ private void logReactorSummary(MavenSession session) {
buffer.append(' ');
}

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

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

if (lastWasSkipped) {
logger.info("...");
}
}

private record ReactorSummaryRequest(
List<ReactorSummaryEntry> entries, StringBuilder buffer, boolean isSingleVersion) {}

@gnodet-bot gnodet-bot Sep 15, 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.

🔧 Mutable StringBuilder in record — still present, checkstyle argument doesn't hold

The author previously responded that the StringBuilder is in the record as a checkstyle workaround. But checkstyle method-length rules apply to the method body where the code lives — in this case the relevant restriction would be on logReactorSummary, which is already short. The StringBuilder allocation belongs in logReactorSummaryGroup, which is a newly added, short method — no checkstyle constraint applies there.

The fix:

Suggested change
List<ReactorSummaryEntry> entries, StringBuilder buffer, boolean isSingleVersion) {}
private record ReactorSummaryRequest(
List<ReactorSummaryEntry> entries, boolean isSingleVersion) {}

Then in logReactorSummaryGroup, change StringBuilder buffer = request.buffer(); to StringBuilder buffer = new StringBuilder(128);, and update the call site in logReactorSummary:

ReactorSummaryRequest request = new ReactorSummaryRequest(entries, isSingleVersion);

A record carrying mutable shared state that is mutated by three consecutive callers is a correctness trap — any future refactor that calls logReactorSummaryGroup twice in parallel or reorders the calls will silently corrupt the buffer. Please fix.


private record ReactorSummaryEntry(
MavenProject project, BuildSummary buildSummary, int group, String statusMessage) {}

private void formatBuildTime(StringBuilder buffer, BuildSummary buildSummary) {
buffer.append(" [");
String buildTimeDuration = formatDuration(buildSummary.getTime());
Expand All @@ -276,12 +292,17 @@ private void logResult(MavenSession session) {
infoLine('-');
MessageBuilder buffer = 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 MessageBuilder builder() {
Expand Down
112 changes: 102 additions & 10 deletions compat/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 @@ -343,6 +343,97 @@ void testSessionEndedSuccessMultimodule() {
inOrder.verify(logger).info("------------------------------------------------------------------------");
}

@Test
void testSessionEndedSuccessWithSkippedModules() {
// prepare
MavenProject project1 = generateMavenProject("Maven Project artifact1");
MavenProject project2 = generateMavenProject("Maven Project artifact2");
MavenProject project3 = generateMavenProject("Maven Project artifact3");

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

MavenExecutionRequest executionRequest = new DefaultMavenExecutionRequest();

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 3.5.4-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
void testSessionEndedFailureMixedWithSkippedModules() {
// prepare
MavenProject project1 = generateMavenProject("Maven Project artifact1");
MavenProject project2 = generateMavenProject("Maven Project artifact2");
MavenProject project3 = generateMavenProject("Maven Project 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();

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 3.5.4-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).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
void testSessionEndedFailureMultimodule() {
// prepare
Expand Down Expand Up @@ -379,11 +470,11 @@ void testSessionEndedFailureMultimodule() {
inOrder.verify(logger).info("------------------------------------------------------------------------");
inOrder.verify(logger).info("Reactor Summary for Maven Project artifact1 3.5.4-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 @@ -434,13 +525,14 @@ void testSessionEndedFailureMultimoduleWithSeparatedFailures() {
inOrder.verify(logger).info("------------------------------------------------------------------------");
inOrder.verify(logger).info("Reactor Summary for Maven Project artifact1 3.5.4-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).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
Loading