Changes following review

junit-team · Jun 21, 2023 · 6b2168b · 6b2168b
1 parent 1fd2d4a
commit 6b2168b
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 39 deletions.
diff --git a/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ExceptionUtils.java b/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ExceptionUtils.java
@@ -94,11 +94,10 @@ public static String readStackTrace(Throwable throwable) {
 	}
 
 	/**
-	 * Prune the stack trace of the supplied {@link Throwable} by filtering its
-	 * elements using the supplied {@link Predicate}, except for
-	 * {@code org.junit.jupiter.api.Assertions} and
-	 * {@code org.junit.jupiter.api.Assumptions} that will always remain
-	 * present.
+	 * Prune the stack trace of the supplied {@link Throwable} by removing
+	 * elements from the {@code org.junit}, {@code jdk.internal.reflect} and
+	 * {@code sun.reflect} packages. If an element matching one of the supplied
+	 * class names is encountered, all following elements will be kept regardless.
 	 *
 	 * <p>Additionally, all elements prior to and including the first
 	 * JUnit Launcher call will be removed.
@@ -113,6 +112,7 @@ public static String readStackTrace(Throwable throwable) {
 	@API(status = INTERNAL, since = "5.10")
 	public static void pruneStackTrace(Throwable throwable, List<String> testClassNames) {
 		Preconditions.notNull(throwable, "Throwable must not be null");
+		Preconditions.notNull(testClassNames, "List of test class names must not be null");
 
 		Predicate<String> stackTraceElementFilter = ClassNamePatternFilterUtils //
 				.excludeMatchingClassNames(STACK_TRACE_ELEMENTS_TO_EXCLUDE);
@@ -126,17 +126,17 @@ public static void pruneStackTrace(Throwable throwable, List<String> testClassNa
 			StackTraceElement element = stackTrace.get(i);
 			String className = element.getClassName();
 
-			if (className.startsWith(JUNIT_PLATFORM_LAUNCHER_PACKAGE_PREFIX)) {
+			if (testClassNames.contains(className)) {
+				// Include all elements called by the test
+				prunedStackTrace.addAll(stackTrace.subList(i, stackTrace.size()));
+				break;
+			}
+			else if (className.startsWith(JUNIT_PLATFORM_LAUNCHER_PACKAGE_PREFIX)) {
 				prunedStackTrace.clear();
 			}
 			else if (stackTraceElementFilter.test(className)) {
 				prunedStackTrace.add(element);
 			}
-			else if (testClassNames.contains(className)) {
-				// Include all elements called by the test
-				prunedStackTrace.addAll(stackTrace.subList(i, stackTrace.size()));
-				break;
-			}
 		}
 
 		Collections.reverse(prunedStackTrace);

diff --git a/.../main/java/org/junit/platform/launcher/core/StackTracePruningEngineExecutionListener.java b/.../main/java/org/junit/platform/launcher/core/StackTracePruningEngineExecutionListener.java
@@ -11,6 +11,7 @@
 package org.junit.platform.launcher.core;
 
 import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -61,6 +62,7 @@ else if (source instanceof MethodSource) {
 						return null;
 					}
 				}) //
+				.filter(Objects::nonNull) //
 				.collect(Collectors.toList());
 	}
 

diff --git a/platform-tests/src/test/java/org/junit/platform/commons/util/ExceptionUtilsTests.java b/platform-tests/src/test/java/org/junit/platform/commons/util/ExceptionUtilsTests.java
@@ -18,7 +18,7 @@
 import static org.junit.platform.commons.util.ExceptionUtils.throwAsUncheckedException;
 
 import java.io.IOException;
-import java.util.Collections;
+import java.util.List;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -70,42 +70,35 @@ void readStackTraceForLocalJUnitException() {
 	@ParameterizedTest
 	@ValueSource(strings = { "org.junit.", "jdk.internal.reflect.", "sun.reflect." })
 	void pruneStackTraceOfCallsFromSpecificPackage(String shouldBePruned) {
-		try {
-			throw new JUnitException("expected");
-		}
-		catch (JUnitException e) {
-			pruneStackTrace(e, Collections.emptyList());
-			assertThat(e.getStackTrace()) //
-					.noneMatch(element -> element.toString().contains(shouldBePruned));
-		}
+		JUnitException exception = new JUnitException("expected");
+
+		pruneStackTrace(exception, List.of());
+
+		assertThat(exception.getStackTrace()) //
+				.noneMatch(element -> element.toString().contains(shouldBePruned));
 	}
 
 	@Test
 	void pruneStackTraceOfAllLauncherCalls() {
-		try {
-			throw new JUnitException("expected");
-		}
-		catch (JUnitException e) {
-			pruneStackTrace(e, Collections.emptyList());
-			assertThat(e.getStackTrace()) //
-					.noneMatch(element -> element.toString().contains("org.junit.platform.launcher."));
-		}
+		JUnitException exception = new JUnitException("expected");
+
+		pruneStackTrace(exception, List.of());
+
+		assertThat(exception.getStackTrace()) //
+				.noneMatch(element -> element.toString().contains("org.junit.platform.launcher."));
 	}
 
 	@Test
 	void pruneStackTraceOfEverythingPriorToFirstLauncherCall() {
-		try {
-			throw new JUnitException("expected");
-		}
-		catch (JUnitException e) {
-			StackTraceElement[] stackTrace = e.getStackTrace();
-			stackTrace[stackTrace.length - 1] = new StackTraceElement("org.example.Class", "method", "file", 123);
-			e.setStackTrace(stackTrace);
+		JUnitException exception = new JUnitException("expected");
+		StackTraceElement[] stackTrace = exception.getStackTrace();
+		stackTrace[stackTrace.length - 1] = new StackTraceElement("org.example.Class", "method", "file", 123);
+		exception.setStackTrace(stackTrace);
 
-			pruneStackTrace(e, Collections.emptyList());
-			assertThat(e.getStackTrace()) //
-					.noneMatch(element -> element.toString().contains("org.example.Class.method(file:123)"));
-		}
+		pruneStackTrace(exception, List.of());
+
+		assertThat(exception.getStackTrace()) //
+				.noneMatch(element -> element.toString().contains("org.example.Class.method(file:123)"));
 	}
 
 	@Test