Enable NullAway in tests

Author: marcphilipp

gradle/plugins/common/src/main/kotlin/junitbuild.java-nullability-conventions.gradle.kts

@@ -29,5 +29,14 @@ tasks.withType<JavaCompile>().configureEach {
 tasks.compileTestJava {
 	options.errorprone.nullaway {
 		handleTestAssertionLibraries = true
+		excludedFieldAnnotations.addAll(
+			"org.junit.jupiter.api.io.TempDir",
+			"org.junit.jupiter.params.Parameter",
+			"org.junit.runners.Parameterized.Parameter",
+			"org.mockito.Captor",
+			"org.mockito.InjectMocks",
+			"org.mockito.Mock",
+			"org.mockito.Spy",
+		)
 	}
 }

jupiter-tests/jupiter-tests.gradle.kts

@@ -2,6 +2,7 @@ import org.gradle.api.tasks.PathSensitivity.RELATIVE
 
 plugins {
 	id("junitbuild.code-generator")
+	id("junitbuild.java-nullability-conventions")
 	id("junitbuild.kotlin-library-conventions")
 	id("junitbuild.junit4-compatibility")
 	id("junitbuild.testing-conventions")

jupiter-tests/src/test/java/org/junit/jupiter/api/AssertAllAssertionsTests.java

@@ -38,31 +38,37 @@
  */
 class AssertAllAssertionsTests {
 
+	@SuppressWarnings({ "DataFlowIssue", "NullAway" })
 	@Test
 	void assertAllWithNullExecutableArray() {
 		assertPrecondition("executables array must not be null or empty", () -> assertAll((Executable[]) null));
 	}
 
+	@SuppressWarnings({ "DataFlowIssue", "NullAway" })
 	@Test
 	void assertAllWithNullExecutableCollection() {
 		assertPrecondition("executables collection must not be null", () -> assertAll((Collection<Executable>) null));
 	}
 
+	@SuppressWarnings({ "DataFlowIssue", "NullAway" })
 	@Test
 	void assertAllWithNullExecutableStream() {
 		assertPrecondition("executables stream must not be null", () -> assertAll((Stream<Executable>) null));
 	}
 
+	@SuppressWarnings({ "DataFlowIssue", "NullAway" })
 	@Test
 	void assertAllWithNullInExecutableArray() {
 		assertPrecondition("individual executables must not be null", () -> assertAll((Executable) null));
 	}
 
+	@SuppressWarnings({ "NullAway" })
 	@Test
 	void assertAllWithNullInExecutableCollection() {
 		assertPrecondition("individual executables must not be null", () -> assertAll(asList((Executable) null)));
 	}
 
+	@SuppressWarnings({ "DataFlowIssue", "NullAway" })
 	@Test
 	void assertAllWithNullInExecutableStream() {
 		assertPrecondition("individual executables must not be null", () -> assertAll(Stream.of((Executable) null)));

jupiter-tests/src/test/java/org/junit/jupiter/api/AssertInstanceOfAssertionsTests.java

@@ -17,6 +17,7 @@
 
 import java.io.IOException;
 
+import org.jspecify.annotations.Nullable;
 import org.junit.jupiter.api.function.Executable;
 import org.opentest4j.AssertionFailedError;
 
@@ -91,7 +92,7 @@ private <T> void assertInstanceOfSucceeds(Class<T> expectedType, Object actualVa
 		assertSame(res, actualValue);
 	}
 
-	private void assertInstanceOfFails(Class<?> expectedType, Object actualValue, String unexpectedSort) {
+	private void assertInstanceOfFails(Class<?> expectedType, @Nullable Object actualValue, String unexpectedSort) {
 		String valueType = actualValue == null ? "null" : actualValue.getClass().getCanonicalName();
 		String expectedMessage = "Unexpected %s, expected: <%s> but was: <%s>".formatted(unexpectedSort,
 			expectedType.getCanonicalName(), valueType);

jupiter-tests/src/test/java/org/junit/jupiter/api/AssertLinesMatchAssertionsTests.java

@@ -95,7 +95,7 @@ void assertLinesMatchUsingFastForwardMarkerWithLimit3() {
 	}
 
 	@Test
-	@SuppressWarnings({ "unchecked", "rawtypes" })
+	@SuppressWarnings({ "unchecked", "rawtypes", "DataFlowIssue", "NullAway" })
 	void assertLinesMatchWithNullFails() {
 		assertThrows(PreconditionViolationException.class, () -> assertLinesMatch(null, (List) null));
 		assertThrows(PreconditionViolationException.class, () -> assertLinesMatch(null, Collections.emptyList()));

jupiter-tests/src/test/java/org/junit/jupiter/api/AssertNullAssertionsTests.java

@@ -18,6 +18,8 @@
 
 import java.util.function.Supplier;
 
+import org.jspecify.annotations.NullUnmarked;
+import org.jspecify.annotations.Nullable;
 import org.opentest4j.AssertionFailedError;
 
 /**
@@ -63,7 +65,8 @@ void assertNullWithNonNullObjectWithNullStringReturnedFromToStringAndMessageSupp
 	}
 
 	@SuppressWarnings("unused")
-	private void assertNullWithNonNullObjectWithNullStringReturnedFromToString(Supplier<String> messageSupplier) {
+	private void assertNullWithNonNullObjectWithNullStringReturnedFromToString(
+			@Nullable Supplier<String> messageSupplier) {
 		String actual = "null";
 		try {
 			if (messageSupplier == null) {
@@ -94,7 +97,8 @@ void assertNullWithNonNullObjectWithNullReferenceReturnedFromToStringAndMessageS
 	}
 
 	@SuppressWarnings("unused")
-	private void assertNullWithNonNullObjectWithNullReferenceReturnedFromToString(Supplier<String> messageSupplier) {
+	private void assertNullWithNonNullObjectWithNullReferenceReturnedFromToString(
+			@Nullable Supplier<String> messageSupplier) {
 		Object actual = new NullToString();
 		try {
 			if (messageSupplier == null) {
@@ -141,6 +145,7 @@ void assertNullWithNonNullObjectAndMessageSupplier() {
 		}
 	}
 
+	@NullUnmarked
 	private static class NullToString {
 
 		@Override

jupiter-tests/src/test/java/org/junit/jupiter/api/AssertThrowsAssertionsTests.java

@@ -47,6 +47,7 @@ void assertThrowsWithMethodReferenceForNonVoidReturnType() {
 		future.run();
 
 		ExecutionException exception = assertThrows(ExecutionException.class, future::get);
+		assertNotNull(exception.getCause());
 		assertEquals("boom", exception.getCause().getMessage());
 	}
 

jupiter-tests/src/test/java/org/junit/jupiter/api/AssertThrowsExactlyAssertionsTests.java

@@ -87,6 +87,7 @@ void assertThrowsWithMethodReferenceForNonVoidReturnType() {
 		future.run();
 
 		ExecutionException exception = assertThrowsExactly(ExecutionException.class, future::get);
+		assertNotNull(exception.getCause());
 		assertEquals("boom", exception.getCause().getMessage());
 	}
 

jupiter-tests/src/test/java/org/junit/jupiter/api/AssertTimeoutPreemptivelyAssertionsTests.java

@@ -18,7 +18,6 @@
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 import static org.junit.jupiter.api.condition.OS.WINDOWS;
 
@@ -28,6 +27,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 
+import org.jspecify.annotations.Nullable;
 import org.junit.jupiter.api.function.Executable;
 import org.junit.platform.commons.util.ExceptionUtils;
 import org.opentest4j.AssertionFailedError;
@@ -81,7 +81,7 @@ void assertTimeoutPreemptivelyForExecutableThatCompletesAfterTheTimeout() {
 			() -> assertTimeoutPreemptively(PREEMPTIVE_TIMEOUT, this::waitForInterrupt));
 		assertMessageEquals(error, "execution timed out after " + PREEMPTIVE_TIMEOUT.toMillis() + " ms");
 		assertMessageStartsWith(error.getCause(), "Execution timed out in ");
-		assertStackTraceContains(error.getCause().getStackTrace(), "CountDownLatch", "await");
+		assertStackTraceContains(error.getCause(), "CountDownLatch", "await");
 	}
 
 	@Test
@@ -91,7 +91,7 @@ void assertTimeoutPreemptivelyWithMessageForExecutableThatCompletesAfterTheTimeo
 		assertMessageEquals(error,
 			"Tempus Fugit ==> execution timed out after " + PREEMPTIVE_TIMEOUT.toMillis() + " ms");
 		assertMessageStartsWith(error.getCause(), "Execution timed out in ");
-		assertStackTraceContains(error.getCause().getStackTrace(), "CountDownLatch", "await");
+		assertStackTraceContains(error.getCause(), "CountDownLatch", "await");
 	}
 
 	@Test
@@ -102,7 +102,7 @@ void assertTimeoutPreemptivelyWithMessageSupplierForExecutableThatCompletesAfter
 		assertMessageEquals(error,
 			"Tempus Fugit ==> execution timed out after " + PREEMPTIVE_TIMEOUT.toMillis() + " ms");
 		assertMessageStartsWith(error.getCause(), "Execution timed out in ");
-		assertStackTraceContains(error.getCause().getStackTrace(), "CountDownLatch", "await");
+		assertStackTraceContains(error.getCause(), "CountDownLatch", "await");
 	}
 
 	@Test
@@ -156,7 +156,7 @@ void assertTimeoutPreemptivelyForSupplierThatCompletesAfterTheTimeout() {
 
 		assertMessageEquals(error, "execution timed out after " + PREEMPTIVE_TIMEOUT.toMillis() + " ms");
 		assertMessageStartsWith(error.getCause(), "Execution timed out in ");
-		assertStackTraceContains(error.getCause().getStackTrace(), "CountDownLatch", "await");
+		assertStackTraceContains(error.getCause(), "CountDownLatch", "await");
 	}
 
 	@Test
@@ -171,7 +171,7 @@ void assertTimeoutPreemptivelyWithMessageForSupplierThatCompletesAfterTheTimeout
 		assertMessageEquals(error,
 			"Tempus Fugit ==> execution timed out after " + PREEMPTIVE_TIMEOUT.toMillis() + " ms");
 		assertMessageStartsWith(error.getCause(), "Execution timed out in ");
-		assertStackTraceContains(error.getCause().getStackTrace(), "CountDownLatch", "await");
+		assertStackTraceContains(error.getCause(), "CountDownLatch", "await");
 	}
 
 	@Test
@@ -186,15 +186,16 @@ void assertTimeoutPreemptivelyWithMessageSupplierForSupplierThatCompletesAfterTh
 		assertMessageEquals(error,
 			"Tempus Fugit ==> execution timed out after " + PREEMPTIVE_TIMEOUT.toMillis() + " ms");
 		assertMessageStartsWith(error.getCause(), "Execution timed out in ");
-		assertStackTraceContains(error.getCause().getStackTrace(), "CountDownLatch", "await");
+		assertStackTraceContains(error.getCause(), "CountDownLatch", "await");
 	}
 
 	@Test
 	void assertTimeoutPreemptivelyUsesThreadsWithSpecificNamePrefix() {
 		AtomicReference<String> threadName = new AtomicReference<>("");
 		assertTimeoutPreemptively(ofMillis(1000), () -> threadName.set(Thread.currentThread().getName()));
-		assertTrue(threadName.get().startsWith("junit-timeout-thread-"),
-			"Thread name does not match the expected prefix");
+		assertThat(threadName.get()) //
+				.withFailMessage("Thread name does not match the expected prefix") //
+				.startsWith("junit-timeout-thread-");
 	}
 
 	@Test
@@ -244,8 +245,9 @@ private void waitForInterrupt() {
 	/**
 	 * Assert the given stack trace elements contain an element with the given class name and method name.
 	 */
-	private static void assertStackTraceContains(StackTraceElement[] stackTrace, String className, String methodName) {
-		assertThat(stackTrace).anySatisfy(element -> {
+	private static void assertStackTraceContains(@Nullable Throwable throwable, String className, String methodName) {
+		assertThat(throwable).isNotNull();
+		assertThat(throwable.getStackTrace()).anySatisfy(element -> {
 			assertThat(element.getClassName()).endsWith(className);
 			assertThat(element.getMethodName()).isEqualTo(methodName);
 		});

jupiter-tests/src/test/java/org/junit/jupiter/api/AssertionTestUtils.java

@@ -13,6 +13,8 @@
 import java.io.Serializable;
 import java.util.Objects;
 
+import org.apache.groovy.parser.antlr4.util.StringUtils;
+import org.jspecify.annotations.Nullable;
 import org.opentest4j.AssertionFailedError;
 import org.opentest4j.ValueWrapper;
 
@@ -27,7 +29,7 @@ static void expectAssertionFailedError() {
 	}
 
 	static void assertEmptyMessage(Throwable ex) throws AssertionError {
-		if (!ex.getMessage().isEmpty()) {
+		if (!StringUtils.isEmpty(ex.getMessage())) {
 			throw new AssertionError("Exception message should be empty, but was [" + ex.getMessage() + "].");
 		}
 	}
@@ -39,35 +41,41 @@ static void assertMessageEquals(Throwable ex, String msg) throws AssertionError
 	}
 
 	static void assertMessageMatches(Throwable ex, String regex) throws AssertionError {
-		if (!ex.getMessage().matches(regex)) {
+		if (ex.getMessage() == null || !ex.getMessage().matches(regex)) {
 			throw new AssertionError("Exception message should match regular expression [" + regex + "], but was ["
 					+ ex.getMessage() + "].");
 		}
 	}
 
-	static void assertMessageStartsWith(Throwable ex, String msg) throws AssertionError {
-		if (!ex.getMessage().startsWith(msg)) {
+	static void assertMessageStartsWith(@Nullable Throwable ex, String msg) throws AssertionError {
+		if (ex == null) {
+			throw new AssertionError("Cause should not have been null");
+		}
+		if (ex.getMessage() == null || !ex.getMessage().startsWith(msg)) {
 			throw new AssertionError(
 				"Exception message should start with [" + msg + "], but was [" + ex.getMessage() + "].");
 		}
 	}
 
 	static void assertMessageEndsWith(Throwable ex, String msg) throws AssertionError {
-		if (!ex.getMessage().endsWith(msg)) {
+		if (ex.getMessage() == null || !ex.getMessage().endsWith(msg)) {
 			throw new AssertionError(
 				"Exception message should end with [" + msg + "], but was [" + ex.getMessage() + "].");
 		}
 	}
 
-	static void assertMessageContains(Throwable ex, String msg) throws AssertionError {
-		if (!ex.getMessage().contains(msg)) {
+	static void assertMessageContains(@Nullable Throwable ex, String msg) throws AssertionError {
+		if (ex == null) {
+			throw new AssertionError("Cause should not have been null");
+		}
+		if (ex.getMessage() == null || !ex.getMessage().contains(msg)) {
 			throw new AssertionError(
 				"Exception message should contain [" + msg + "], but was [" + ex.getMessage() + "].");
 		}
 	}
 
-	static void assertExpectedAndActualValues(AssertionFailedError ex, Object expected, Object actual)
-			throws AssertionError {
+	static void assertExpectedAndActualValues(AssertionFailedError ex, @Nullable Object expected,
+			@Nullable Object actual) throws AssertionError {
 		if (!wrapsEqualValue(ex.getExpected(), expected)) {
 			throw new AssertionError("Expected value in AssertionFailedError should equal ["
 					+ ValueWrapper.create(expected) + "], but was [" + ex.getExpected() + "].");
@@ -78,7 +86,7 @@ static void assertExpectedAndActualValues(AssertionFailedError ex, Object expect
 		}
 	}
 
-	static boolean wrapsEqualValue(ValueWrapper wrapper, Object value) {
+	static boolean wrapsEqualValue(ValueWrapper wrapper, @Nullable Object value) {
 		if (value == null || value instanceof Serializable) {
 			return Objects.equals(value, wrapper.getValue());
 		}