Copilot commented on code in PR #12705:
URL: https://github.com/apache/maven/pull/12705#discussion_r3740261514
##########
impl/maven-cli/src/test/java/org/apache/maven/cling/event/ExecutionEventLoggerTest.java:
##########
@@ -446,6 +447,66 @@ void
testSessionEndedFailureMultimoduleWithSeparatedFailures() {
inOrder.verify(logger).info("------------------------------------------------------------------------");
}
+ @Test
+ void testProjectSkippedBecauseADependencyFailed() {
+ // prepare
+ MavenProject failed = generateMavenProject("Maven Project artifact1");
+ MavenProject skipped = generateMavenProject("Maven Project artifact2");
+
+ DefaultMavenExecutionResult executionResult = new
DefaultMavenExecutionResult();
+ executionResult.addBuildSummary(new BuildFailure(failed, 1000, new
Exception("Failure")));
+
+ ExecutionEvent event = skipEvent(skipped, executionResult,
Arrays.asList(failed));
+
+ // execute
+ executionEventLogger.projectSkipped(event);
+
+ // verify
+ InOrder inOrder = inOrder(logger);
+ inOrder.verify(logger).info("Skipping Maven Project artifact2");
+ inOrder.verify(logger)
+ .info("{} was not built because a module it depends on failed
to build.", "Maven Project artifact2");
+ }
+
+ @Test
+ void testProjectSkippedBecauseTheBuildWasStopped() {
Review Comment:
The new skip-reason logic has an important “unknown” case described in the
PR: when upstream projects exist but their outcomes cannot be established yet,
the logger should keep the dependency wording. Adding a unit test for this case
will prevent regressions (and will fail with the current implementation that
treats “no BuildFailure summaries yet” as “build was stopped”).
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/event/ExecutionEventLogger.java:
##########
@@ -316,12 +316,34 @@ public void projectSkipped(ExecutionEvent event) {
infoLine('-');
String name = event.getProject().getName();
infoMain("Skipping " + name);
- logger.info("{} was not built because a module it depends on
failed to build.", name);
+ if (dependsOnFailedProject(event)) {
+ logger.info("{} was not built because a module it depends on
failed to build.", name);
+ } else {
+ logger.info("{} was not built because the build was stopped
after an earlier failure.", name);
+ }
infoLine('-');
}
}
+ /**
+ * A project can be skipped for two different reasons: one of the modules
it depends on failed,
+ * or the reactor was stopped after an unrelated module failed. Only the
first one lets us blame
+ * a dependency, so tell them apart instead of always reporting the same
cause.
+ * When the answer cannot be established, the dependency wording is kept.
+ */
+ private boolean dependsOnFailedProject(ExecutionEvent event) {
+ MavenSession session = event.getSession();
+ MavenProject project = event.getProject();
+ if (session == null || project == null ||
session.getProjectDependencyGraph() == null) {
+ return true;
+ }
+ MavenExecutionResult result = session.getResult();
+ return result == null
+ ||
session.getProjectDependencyGraph().getUpstreamProjects(project, true).stream()
+ .anyMatch(upstream -> result.getBuildSummary(upstream)
instanceof BuildFailure);
+ }
Review Comment:
`dependsOnFailedProject` can misclassify dependency-related skips as “build
was stopped” when upstream build summaries have not been recorded yet (common
in concurrent execution). In that situation the cause is not established, and
per the method javadoc/PR description the logger should keep the dependency
wording instead of switching to the “stopped” wording.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
