Copilot commented on code in PR #3489:
URL: https://github.com/apache/maven-surefire/pull/3489#discussion_r4346508831
##########
surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/LauncherAdapter.java:
##########
@@ -57,15 +66,73 @@ void execute(LauncherDiscoveryRequest discoveryRequest,
TestExecutionListener...
void executeWithoutCancellationToken(
LauncherDiscoveryRequest discoveryRequest,
TestExecutionListener... listeners) {
- delegate.execute(discoveryRequest, listeners);
+ try {
+ delegate.execute(discoveryRequest, listeners);
+ } catch (JUnitException e) {
+ throw withRootCauseMessage(e);
+ }
}
private void executeWithCancellationToken(
LauncherDiscoveryRequest discoveryRequest,
TestExecutionListener... listeners) {
Method executeMethod =
requireNonNull(EXECUTE_METHOD_WITH_LAUNCHER_EXECUTION_REQUEST_PARAMETER);
Object executionRequest = createExecutionRequest(discoveryRequest,
listeners);
- ReflectionUtils.invokeMethodWithArray(delegate, executeMethod,
executionRequest);
+ try {
+ ReflectionUtils.invokeMethodWithArray(delegate, executeMethod,
executionRequest);
+ } catch (JUnitException e) {
+ throw withRootCauseMessage(e);
+ } catch (SurefireReflectionException e) {
+ JUnitException junitException = findJUnitException(e);
+ if (junitException != null) {
+ throw withRootCauseMessage(junitException);
+ }
+ throw e;
+ }
+ }
+
+ static JUnitException findJUnitException(Throwable failure) {
+ Set<Throwable> visited = Collections.newSetFromMap(new
IdentityHashMap<Throwable, Boolean>());
+ Throwable cause = failure;
+ while (cause != null && visited.add(cause)) {
+ if (cause instanceof JUnitException) {
+ return (JUnitException) cause;
+ }
+ cause = cause.getCause();
+ }
+ return null;
+ }
+
+ private static JUnitException withRootCauseMessage(JUnitException failure)
{
+ Throwable rootCause = failure;
+ Set<Throwable> visited = Collections.newSetFromMap(new
IdentityHashMap<Throwable, Boolean>());
+ visited.add(rootCause);
+ while (rootCause.getCause() != null &&
visited.add(rootCause.getCause())) {
+ rootCause = rootCause.getCause();
+ }
+
+ if (rootCause == failure) {
+ return failure;
+ }
+
+ String causeMessage = rootCause.getMessage();
+ String rootCauseDescription = rootCause.toString();
+ String message = failure.getMessage();
+ if (message != null) {
+ if (message.contains(rootCauseDescription)) {
+ return failure;
+ }
+ if (causeMessage != null && !causeMessage.isEmpty() &&
message.contains(causeMessage)) {
+ String rootCauseType = rootCause.getClass().getName();
+ if (message.contains(rootCauseType)) {
+ return failure;
+ }
+ return new JUnitException(message + ": " + rootCauseType,
failure);
+ }
+ }
+ return new JUnitException(
+ message == null || message.isEmpty() ? rootCauseDescription :
message + ": " + rootCauseDescription,
+ failure);
}
Review Comment:
When `withRootCauseMessage` creates a new `JUnitException`, the top-level
stack trace and suppressed exceptions change/are lost, which can reduce the
diagnostic value compared to the original failure. Consider copying the
original stack trace (e.g., `newEx.setStackTrace(failure.getStackTrace())`) and
propagating suppressed exceptions from `failure` to the new exception before
throwing, so log output remains actionable while preserving original debugging
context.
##########
surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/LauncherAdapter.java:
##########
@@ -44,7 +49,11 @@ final class LauncherAdapter {
}
TestPlan discover(LauncherDiscoveryRequest discoveryRequest) {
- return delegate.discover(discoveryRequest);
+ try {
+ return delegate.discover(discoveryRequest);
+ } catch (JUnitException e) {
+ throw withRootCauseMessage(e);
+ }
}
Review Comment:
The same `try/catch (JUnitException) { throw withRootCauseMessage(...) }`
pattern is duplicated across `discover(...)` and
`executeWithoutCancellationToken(...)` (and partially in the cancellation-token
path). To reduce repetition and keep behavior consistent, consider extracting a
small private helper to run an action and translate `JUnitException`s in one
place (e.g., a `translateJUnitException(Supplier<T>)` /
`translateJUnitException(Runnable)` style helper).
##########
surefire-its/src/test/resources/surefire-3463-testng-extension-failure/src/test/resources/META-INF/services/org.testng.ITestNGListener:
##########
@@ -0,0 +1 @@
+issue3463.FailingSuiteListener
Review Comment:
Service provider configuration files typically should end with a trailing
newline to avoid tooling edge-cases (some editors and resource filters can
mishandle the last line without it). Consider ensuring this file ends with a
newline.
##########
surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/LauncherAdapter.java:
##########
@@ -57,15 +66,73 @@ void execute(LauncherDiscoveryRequest discoveryRequest,
TestExecutionListener...
void executeWithoutCancellationToken(
LauncherDiscoveryRequest discoveryRequest,
TestExecutionListener... listeners) {
- delegate.execute(discoveryRequest, listeners);
+ try {
+ delegate.execute(discoveryRequest, listeners);
+ } catch (JUnitException e) {
+ throw withRootCauseMessage(e);
+ }
}
Review Comment:
The same `try/catch (JUnitException) { throw withRootCauseMessage(...) }`
pattern is duplicated across `discover(...)` and
`executeWithoutCancellationToken(...)` (and partially in the cancellation-token
path). To reduce repetition and keep behavior consistent, consider extracting a
small private helper to run an action and translate `JUnitException`s in one
place (e.g., a `translateJUnitException(Supplier<T>)` /
`translateJUnitException(Runnable)` style helper).
--
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]
