Report discovery issues for blank display names

marcphilipp · marcphilipp · commit dce60de5a939 · 2025-04-08T10:50:42.000+02:00
diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java
@@ -137,6 +137,14 @@ public final void validate(DiscoveryIssueReporter reporter) {
 		validateCoreLifecycleMethods(reporter);
 		validateClassTemplateInvocationLifecycleMethods(reporter);
 		validateTags(reporter);
+		validateDisplayNameAnnotation(reporter);
+	}
+
+	private void validateDisplayNameAnnotation(DiscoveryIssueReporter reporter) {
+		DisplayNameUtils.validateAnnotation(getTestClass(), //
+			() -> String.format("class '%s'", getTestClass().getName()), //
+			() -> getSource().orElse(null), //
+			reporter);
 	}
 
 	protected void validateCoreLifecycleMethods(DiscoveryIssueReporter reporter) {
diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/DisplayNameUtils.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/DisplayNameUtils.java
@@ -30,11 +30,13 @@
 import org.junit.jupiter.api.DisplayNameGenerator.Simple;
 import org.junit.jupiter.api.DisplayNameGenerator.Standard;
 import org.junit.jupiter.engine.config.JupiterConfiguration;
-import org.junit.platform.commons.logging.Logger;
-import org.junit.platform.commons.logging.LoggerFactory;
 import org.junit.platform.commons.support.ReflectionSupport;
 import org.junit.platform.commons.util.Preconditions;
 import org.junit.platform.commons.util.StringUtils;
+import org.junit.platform.engine.DiscoveryIssue;
+import org.junit.platform.engine.DiscoveryIssue.Severity;
+import org.junit.platform.engine.TestSource;
+import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter;
 
 /**
  * Collection of utilities for working with display names.
@@ -46,8 +48,6 @@
  */
 final class DisplayNameUtils {
 
-	private static final Logger logger = LoggerFactory.getLogger(DisplayNameUtils.class);
-
 	/**
 	 * Pre-defined standard display name generator instance.
 	 */
@@ -74,22 +74,23 @@ final class DisplayNameUtils {
 
 	static String determineDisplayName(AnnotatedElement element, Supplier<String> displayNameSupplier) {
 		Preconditions.notNull(element, "Annotated element must not be null");
-		Optional<DisplayName> displayNameAnnotation = findAnnotation(element, DisplayName.class);
-		if (displayNameAnnotation.isPresent()) {
-			String displayName = displayNameAnnotation.get().value().trim();
-
-			// TODO [#242] Replace logging with precondition check once we have a proper mechanism for
-			// handling validation exceptions during the TestEngine discovery phase.
-			if (StringUtils.isBlank(displayName)) {
-				logger.warn(() -> String.format(
-					"Configuration error: @DisplayName on [%s] must be declared with a non-blank value.", element));
-			}
-			else {
-				return displayName;
-			}
-		}
-		// else let a 'DisplayNameGenerator' generate a display name
-		return displayNameSupplier.get();
+		return findAnnotation(element, DisplayName.class) //
+				.map(DisplayName::value) //
+				.filter(StringUtils::isNotBlank) //
+				.orElseGet(displayNameSupplier);
+	}
+
+	static void validateAnnotation(AnnotatedElement element, Supplier<String> elementDescription,
+			Supplier<TestSource> sourceProvider, DiscoveryIssueReporter reporter) {
+		findAnnotation(element, DisplayName.class) //
+				.map(DisplayName::value) //
+				.filter(StringUtils::isBlank) //
+				.ifPresent(__ -> {
+					String message = String.format("@DisplayName on %s must be declared with a non-blank value.",
+						elementDescription.get());
+					reporter.reportIssue(
+						DiscoveryIssue.builder(Severity.WARNING, message).source(sourceProvider.get()).build());
+				});
 	}
 
 	static String determineDisplayNameForMethod(Supplier<List<Class<?>>> enclosingInstanceTypes, Class<?> testClass,
@@ -157,5 +158,4 @@ private static Optional<DisplayNameGenerator> findDisplayNameGenerator(List<Clas
 					return ReflectionSupport.newInstance(displayNameGeneratorClass);
 				});
 	}
-
 }
diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/MethodBasedTestDescriptor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/MethodBasedTestDescriptor.java
@@ -113,6 +113,11 @@ public List<Class<?>> getEnclosingTestClasses() {
 	@Override
 	public void validate(DiscoveryIssueReporter reporter) {
 		Validatable.reportAndClear(this.methodInfo.discoveryIssues, reporter);
+		DisplayNameUtils.validateAnnotation(getTestMethod(), //
+			() -> String.format("method '%s'", getTestMethod().toGenericString()), //
+			// Use _declaring_ class here because that's where the `@DisplayName` annotation is declared
+			() -> MethodSource.from(getTestMethod()), //
+			reporter);
 	}
 
 	// --- Node ----------------------------------------------------------------
diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/descriptor/DisplayNameUtilsTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/descriptor/DisplayNameUtilsTests.java
@@ -25,7 +25,6 @@
 import org.junit.jupiter.api.DisplayNameGenerator;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.fixtures.TrackLogRecords;
 import org.junit.jupiter.engine.config.JupiterConfiguration;
 import org.junit.platform.commons.logging.LogRecordListener;
 
@@ -48,15 +47,12 @@ void shouldGetDisplayNameFromDisplayNameAnnotation() {
 		}
 
 		@Test
-		void shouldGetDisplayNameFromSupplierIfNoDisplayNameAnnotationWithBlankStringPresent(
-				@TrackLogRecords LogRecordListener listener) {
+		void shouldGetDisplayNameFromSupplierIfDisplayNameAnnotationProvidesBlankString() {
 
 			String displayName = DisplayNameUtils.determineDisplayName(BlankDisplayNameTestCase.class,
 				() -> "default-name");
 
 			assertThat(displayName).isEqualTo("default-name");
-			assertThat(firstWarningLogRecord(listener).getMessage()).isEqualTo(
-				"Configuration error: @DisplayName on [class org.junit.jupiter.engine.descriptor.DisplayNameUtilsTests$BlankDisplayNameTestCase] must be declared with a non-blank value.");
 		}
 
 		@Test
diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/DiscoveryTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/DiscoveryTests.java
@@ -33,6 +33,7 @@
 import java.util.List;
 import java.util.regex.Pattern;
 
+import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Named;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Tag;
@@ -308,6 +309,28 @@ void reportsWarningsForInvalidTags() throws NoSuchMethodException {
 				.contains(org.junit.platform.engine.support.descriptor.MethodSource.from(method));
 	}
 
+	@Test
+	void reportsWarningsForBlankDisplayNames() throws NoSuchMethodException {
+
+		var results = discoverTestsForClass(BlankDisplayNamesTestCase.class);
+
+		var discoveryIssues = results.getDiscoveryIssues().stream().sorted(comparing(DiscoveryIssue::message)).toList();
+		assertThat(discoveryIssues).hasSize(2);
+
+		assertThat(discoveryIssues.getFirst().message()) //
+				.isEqualTo("@DisplayName on class '%s' must be declared with a non-blank value.",
+					BlankDisplayNamesTestCase.class.getName());
+		assertThat(discoveryIssues.getFirst().source()) //
+				.contains(ClassSource.from(BlankDisplayNamesTestCase.class));
+
+		var method = BlankDisplayNamesTestCase.class.getDeclaredMethod("test");
+		assertThat(discoveryIssues.getLast().message()) //
+				.isEqualTo("@DisplayName on method '%s' must be declared with a non-blank value.",
+					method.toGenericString());
+		assertThat(discoveryIssues.getLast().source()) //
+				.contains(org.junit.platform.engine.support.descriptor.MethodSource.from(method));
+	}
+
 	// -------------------------------------------------------------------
 
 	@SuppressWarnings("unused")
@@ -426,4 +449,13 @@ void test() {
 		}
 	}
 
+	@SuppressWarnings("JUnitMalformedDeclaration")
+	@DisplayName("")
+	static class BlankDisplayNamesTestCase {
+		@Test
+		@DisplayName("\t")
+		void test() {
+		}
+	}
+
 }
diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/RepeatedTestTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/RepeatedTestTests.java
@@ -18,7 +18,9 @@
 import static org.junit.jupiter.engine.Constants.PARALLEL_CONFIG_FIXED_PARALLELISM_PROPERTY_NAME;
 import static org.junit.jupiter.engine.Constants.PARALLEL_CONFIG_STRATEGY_PROPERTY_NAME;
 import static org.junit.jupiter.engine.Constants.PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME;
+import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
 import static org.junit.platform.engine.discovery.DiscoverySelectors.selectMethod;
+import static org.junit.platform.launcher.LauncherConstants.CRITICAL_DISCOVERY_ISSUE_SEVERITY_PROPERTY_NAME;
 import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.request;
 import static org.junit.platform.testkit.engine.EventConditions.container;
 import static org.junit.platform.testkit.engine.EventConditions.displayName;
@@ -45,6 +47,7 @@
 import org.junit.jupiter.api.TestInfo;
 import org.junit.jupiter.engine.AbstractJupiterTestEngineTests;
 import org.junit.platform.commons.support.ReflectionSupport;
+import org.junit.platform.engine.DiscoveryIssue.Severity;
 import org.junit.platform.launcher.LauncherDiscoveryRequest;
 import org.junit.platform.testkit.engine.Events;
 
@@ -62,10 +65,13 @@ void customDisplayName(TestInfo testInfo) {
 		assertThat(testInfo.getDisplayName()).isEqualTo("repetition 1 of 1");
 	}
 
-	@RepeatedTest(1)
-	@DisplayName("   \t ")
-	void customDisplayNameWithBlankName(TestInfo testInfo) {
-		assertThat(testInfo.getDisplayName()).isEqualTo("repetition 1 of 1");
+	@Test
+	void customDisplayNameWithBlankName() {
+		executeTests(request -> request //
+				.selectors(selectClass(BlankDisplayNameTestCase.class)) //
+				.configurationParameter(CRITICAL_DISCOVERY_ISSUE_SEVERITY_PROPERTY_NAME, Severity.ERROR.name())) //
+						.testEvents() //
+						.assertStatistics(stats -> stats.started(1).succeeded(1));
 	}
 
 	@RepeatedTest(value = 1, name = "{displayName}")
@@ -406,4 +412,13 @@ void failureThresholdWithConcurrentExecution() {
 
 	}
 
+	static class BlankDisplayNameTestCase {
+
+		@RepeatedTest(1)
+		@DisplayName("   \t ")
+		void test(TestInfo testInfo) {
+			assertThat(testInfo.getDisplayName()).isEqualTo("repetition 1 of 1");
+		}
+	}
+
 }
diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TestInfoParameterResolverTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TestInfoParameterResolverTests.java
@@ -13,6 +13,8 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
+import static org.junit.platform.launcher.LauncherConstants.CRITICAL_DISCOVERY_ISSUE_SEVERITY_PROPERTY_NAME;
 
 import java.util.Arrays;
 import java.util.List;
@@ -26,17 +28,19 @@
 import org.junit.jupiter.api.Tag;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.engine.AbstractJupiterTestEngineTests;
+import org.junit.platform.engine.DiscoveryIssue.Severity;
 
 /**
  * Integration tests for {@link TestInfoParameterResolver}.
  *
  * @since 5.0
  */
 @Tag("class-tag")
-class TestInfoParameterResolverTests {
+class TestInfoParameterResolverTests extends AbstractJupiterTestEngineTests {
 
 	private static final List<String> allDisplayNames = Arrays.asList("defaultDisplayName(TestInfo)",
-		"custom display name", "getTags(TestInfo)", "customDisplayNameThatIsEmpty(TestInfo)");
+		"custom display name", "getTags(TestInfo)", "customDisplayNameThatIsEmpty()");
 
 	public TestInfoParameterResolverTests(TestInfo testInfo) {
 		assertThat(testInfo.getTestClass()).contains(TestInfoParameterResolverTests.class);
@@ -54,11 +58,13 @@ void providedDisplayName(TestInfo testInfo) {
 		assertEquals("custom display name", testInfo.getDisplayName());
 	}
 
-	// TODO Update test to expect an exception once #743 is fixed.
 	@Test
-	@DisplayName("")
-	void customDisplayNameThatIsEmpty(TestInfo testInfo) {
-		assertEquals("customDisplayNameThatIsEmpty(TestInfo)", testInfo.getDisplayName());
+	void customDisplayNameThatIsEmpty() {
+		executeTests(request -> request //
+				.selectors(selectClass(BlankDisplayNameTestCase.class)) //
+				.configurationParameter(CRITICAL_DISCOVERY_ISSUE_SEVERITY_PROPERTY_NAME, Severity.ERROR.name())) //
+						.testEvents() //
+						.assertStatistics(stats -> stats.started(1).succeeded(1));
 	}
 
 	@Test
@@ -88,4 +94,14 @@ static void beforeAndAfterAll(TestInfo testInfo) {
 		assertEquals(TestInfoParameterResolverTests.class.getSimpleName(), testInfo.getDisplayName());
 	}
 
+	@SuppressWarnings("JUnitMalformedDeclaration")
+	static class BlankDisplayNameTestCase {
+
+		@Test
+		@DisplayName("")
+		void test(TestInfo testInfo) {
+			assertEquals("test(TestInfo)", testInfo.getDisplayName());
+		}
+	}
+
 }
