Skip to content

Commit 7b9008e

Browse files
marcphilippmarcphilipp
committed
Report discovery issues for blank display names
1 parent 4696b7f commit 7b9008e

File tree

7 files changed

+108
-35
lines changed

7 files changed

+108
-35
lines changed

‎junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java

+8
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,14 @@ public final void validate(DiscoveryIssueReporter reporter) {
137137
validateCoreLifecycleMethods(reporter);
138138
validateClassTemplateInvocationLifecycleMethods(reporter);
139139
validateTags(reporter);
140+
validateDisplayNameAnnotation(reporter);
141+
}
142+
143+
private void validateDisplayNameAnnotation(DiscoveryIssueReporter reporter) {
144+
DisplayNameUtils.validateAnnotation(getTestClass(), //
145+
() -> String.format("class '%s'", getTestClass().getName()), //
146+
() -> getSource().orElse(null), //
147+
reporter);
140148
}
141149

142150
protected void validateCoreLifecycleMethods(DiscoveryIssueReporter reporter) {

‎junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/DisplayNameUtils.java

+21-20
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,13 @@
3030
import org.junit.jupiter.api.DisplayNameGenerator.Simple;
3131
import org.junit.jupiter.api.DisplayNameGenerator.Standard;
3232
import org.junit.jupiter.engine.config.JupiterConfiguration;
33-
import org.junit.platform.commons.logging.Logger;
34-
import org.junit.platform.commons.logging.LoggerFactory;
3533
import org.junit.platform.commons.support.ReflectionSupport;
3634
import org.junit.platform.commons.util.Preconditions;
3735
import org.junit.platform.commons.util.StringUtils;
36+
import org.junit.platform.engine.DiscoveryIssue;
37+
import org.junit.platform.engine.DiscoveryIssue.Severity;
38+
import org.junit.platform.engine.TestSource;
39+
import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter;
3840

3941
/**
4042
* Collection of utilities for working with display names.
@@ -46,8 +48,6 @@
4648
*/
4749
final class DisplayNameUtils {
4850

49-
private static final Logger logger = LoggerFactory.getLogger(DisplayNameUtils.class);
50-
5151
/**
5252
* Pre-defined standard display name generator instance.
5353
*/
@@ -74,22 +74,23 @@ final class DisplayNameUtils {
7474

7575
static String determineDisplayName(AnnotatedElement element, Supplier<String> displayNameSupplier) {
7676
Preconditions.notNull(element, "Annotated element must not be null");
77-
Optional<DisplayName> displayNameAnnotation = findAnnotation(element, DisplayName.class);
78-
if (displayNameAnnotation.isPresent()) {
79-
String displayName = displayNameAnnotation.get().value().trim();
80-
81-
// TODO [#242] Replace logging with precondition check once we have a proper mechanism for
82-
// handling validation exceptions during the TestEngine discovery phase.
83-
if (StringUtils.isBlank(displayName)) {
84-
logger.warn(() -> String.format(
85-
"Configuration error: @DisplayName on [%s] must be declared with a non-blank value.", element));
86-
}
87-
else {
88-
return displayName;
89-
}
90-
}
91-
// else let a 'DisplayNameGenerator' generate a display name
92-
return displayNameSupplier.get();
77+
return findAnnotation(element, DisplayName.class) //
78+
.map(DisplayName::value) //
79+
.filter(StringUtils::isNotBlank) //
80+
.orElseGet(displayNameSupplier);
81+
}
82+
83+
static void validateAnnotation(AnnotatedElement element, Supplier<String> elementDescription,
84+
Supplier<TestSource> sourceProvider, DiscoveryIssueReporter reporter) {
85+
findAnnotation(element, DisplayName.class) //
86+
.map(DisplayName::value) //
87+
.filter(StringUtils::isBlank) //
88+
.ifPresent(__ -> {
89+
String message = String.format("@DisplayName on %s must be declared with a non-blank value.",
90+
elementDescription.get());
91+
reporter.reportIssue(
92+
DiscoveryIssue.builder(Severity.WARNING, message).source(sourceProvider.get()).build());
93+
});
9394
}
9495

9596
static String determineDisplayNameForMethod(Supplier<List<Class<?>>> enclosingInstanceTypes, Class<?> testClass,

‎junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/MethodBasedTestDescriptor.java

+5
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@ public List<Class<?>> getEnclosingTestClasses() {
113113
@Override
114114
public void validate(DiscoveryIssueReporter reporter) {
115115
Validatable.reportAndClear(this.methodInfo.discoveryIssues, reporter);
116+
DisplayNameUtils.validateAnnotation(getTestMethod(), //
117+
() -> String.format("method '%s'", getTestMethod().toGenericString()), //
118+
// Use _declaring_ class here because that's where the `@DisplayName` annotation is declared
119+
() -> MethodSource.from(getTestMethod()), //
120+
reporter);
116121
}
117122

118123
// --- Node ----------------------------------------------------------------

‎jupiter-tests/src/test/java/org/junit/jupiter/engine/descriptor/DisplayNameUtilsTests.java

+1-5
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.junit.jupiter.api.DisplayNameGenerator;
2626
import org.junit.jupiter.api.Nested;
2727
import org.junit.jupiter.api.Test;
28-
import org.junit.jupiter.api.fixtures.TrackLogRecords;
2928
import org.junit.jupiter.engine.config.JupiterConfiguration;
3029
import org.junit.platform.commons.logging.LogRecordListener;
3130

@@ -48,15 +47,12 @@ void shouldGetDisplayNameFromDisplayNameAnnotation() {
4847
}
4948

5049
@Test
51-
void shouldGetDisplayNameFromSupplierIfNoDisplayNameAnnotationWithBlankStringPresent(
52-
@TrackLogRecords LogRecordListener listener) {
50+
void shouldGetDisplayNameFromSupplierIfDisplayNameAnnotationProvidesBlankString() {
5351

5452
String displayName = DisplayNameUtils.determineDisplayName(BlankDisplayNameTestCase.class,
5553
() -> "default-name");
5654

5755
assertThat(displayName).isEqualTo("default-name");
58-
assertThat(firstWarningLogRecord(listener).getMessage()).isEqualTo(
59-
"Configuration error: @DisplayName on [class org.junit.jupiter.engine.descriptor.DisplayNameUtilsTests$BlankDisplayNameTestCase] must be declared with a non-blank value.");
6056
}
6157

6258
@Test

‎jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/DiscoveryTests.java

+32
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.List;
3434
import java.util.regex.Pattern;
3535

36+
import org.junit.jupiter.api.DisplayName;
3637
import org.junit.jupiter.api.Named;
3738
import org.junit.jupiter.api.Nested;
3839
import org.junit.jupiter.api.Tag;
@@ -308,6 +309,28 @@ void reportsWarningsForInvalidTags() throws NoSuchMethodException {
308309
.contains(org.junit.platform.engine.support.descriptor.MethodSource.from(method));
309310
}
310311

312+
@Test
313+
void reportsWarningsForBlankDisplayNames() throws NoSuchMethodException {
314+
315+
var results = discoverTestsForClass(BlankDisplayNamesTestCase.class);
316+
317+
var discoveryIssues = results.getDiscoveryIssues().stream().sorted(comparing(DiscoveryIssue::message)).toList();
318+
assertThat(discoveryIssues).hasSize(2);
319+
320+
assertThat(discoveryIssues.getFirst().message()) //
321+
.isEqualTo("@DisplayName on class '%s' must be declared with a non-blank value.",
322+
BlankDisplayNamesTestCase.class.getName());
323+
assertThat(discoveryIssues.getFirst().source()) //
324+
.contains(ClassSource.from(BlankDisplayNamesTestCase.class));
325+
326+
var method = BlankDisplayNamesTestCase.class.getDeclaredMethod("test");
327+
assertThat(discoveryIssues.getLast().message()) //
328+
.isEqualTo("@DisplayName on method '%s' must be declared with a non-blank value.",
329+
method.toGenericString());
330+
assertThat(discoveryIssues.getLast().source()) //
331+
.contains(org.junit.platform.engine.support.descriptor.MethodSource.from(method));
332+
}
333+
311334
// -------------------------------------------------------------------
312335

313336
@SuppressWarnings("unused")
@@ -426,4 +449,13 @@ void test() {
426449
}
427450
}
428451

452+
@SuppressWarnings("JUnitMalformedDeclaration")
453+
@DisplayName("")
454+
static class BlankDisplayNamesTestCase {
455+
@Test
456+
@DisplayName("\t")
457+
void test() {
458+
}
459+
}
460+
429461
}

‎jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/RepeatedTestTests.java

+19-4
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
import static org.junit.jupiter.engine.Constants.PARALLEL_CONFIG_FIXED_PARALLELISM_PROPERTY_NAME;
1919
import static org.junit.jupiter.engine.Constants.PARALLEL_CONFIG_STRATEGY_PROPERTY_NAME;
2020
import static org.junit.jupiter.engine.Constants.PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME;
21+
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
2122
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectMethod;
23+
import static org.junit.platform.launcher.LauncherConstants.CRITICAL_DISCOVERY_ISSUE_SEVERITY_PROPERTY_NAME;
2224
import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.request;
2325
import static org.junit.platform.testkit.engine.EventConditions.container;
2426
import static org.junit.platform.testkit.engine.EventConditions.displayName;
@@ -45,6 +47,7 @@
4547
import org.junit.jupiter.api.TestInfo;
4648
import org.junit.jupiter.engine.AbstractJupiterTestEngineTests;
4749
import org.junit.platform.commons.support.ReflectionSupport;
50+
import org.junit.platform.engine.DiscoveryIssue.Severity;
4851
import org.junit.platform.launcher.LauncherDiscoveryRequest;
4952
import org.junit.platform.testkit.engine.Events;
5053

@@ -62,10 +65,13 @@ void customDisplayName(TestInfo testInfo) {
6265
assertThat(testInfo.getDisplayName()).isEqualTo("repetition 1 of 1");
6366
}
6467

65-
@RepeatedTest(1)
66-
@DisplayName(" \t ")
67-
void customDisplayNameWithBlankName(TestInfo testInfo) {
68-
assertThat(testInfo.getDisplayName()).isEqualTo("repetition 1 of 1");
68+
@Test
69+
void customDisplayNameWithBlankName() {
70+
executeTests(request -> request //
71+
.selectors(selectClass(BlankDisplayNameTestCase.class)) //
72+
.configurationParameter(CRITICAL_DISCOVERY_ISSUE_SEVERITY_PROPERTY_NAME, Severity.ERROR.name())) //
73+
.testEvents() //
74+
.assertStatistics(stats -> stats.started(1).succeeded(1));
6975
}
7076

7177
@RepeatedTest(value = 1, name = "{displayName}")
@@ -406,4 +412,13 @@ void failureThresholdWithConcurrentExecution() {
406412

407413
}
408414

415+
static class BlankDisplayNameTestCase {
416+
417+
@RepeatedTest(1)
418+
@DisplayName(" \t ")
419+
void test(TestInfo testInfo) {
420+
assertThat(testInfo.getDisplayName()).isEqualTo("repetition 1 of 1");
421+
}
422+
}
423+
409424
}

‎jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TestInfoParameterResolverTests.java

+22-6
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
import static org.assertj.core.api.Assertions.assertThat;
1414
import static org.junit.jupiter.api.Assertions.assertEquals;
1515
import static org.junit.jupiter.api.Assertions.assertTrue;
16+
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
17+
import static org.junit.platform.launcher.LauncherConstants.CRITICAL_DISCOVERY_ISSUE_SEVERITY_PROPERTY_NAME;
1618

1719
import java.util.Arrays;
1820
import java.util.List;
@@ -26,17 +28,19 @@
2628
import org.junit.jupiter.api.Tag;
2729
import org.junit.jupiter.api.Test;
2830
import org.junit.jupiter.api.TestInfo;
31+
import org.junit.jupiter.engine.AbstractJupiterTestEngineTests;
32+
import org.junit.platform.engine.DiscoveryIssue.Severity;
2933

3034
/**
3135
* Integration tests for {@link TestInfoParameterResolver}.
3236
*
3337
* @since 5.0
3438
*/
3539
@Tag("class-tag")
36-
class TestInfoParameterResolverTests {
40+
class TestInfoParameterResolverTests extends AbstractJupiterTestEngineTests {
3741

3842
private static final List<String> allDisplayNames = Arrays.asList("defaultDisplayName(TestInfo)",
39-
"custom display name", "getTags(TestInfo)", "customDisplayNameThatIsEmpty(TestInfo)");
43+
"custom display name", "getTags(TestInfo)", "customDisplayNameThatIsEmpty()");
4044

4145
public TestInfoParameterResolverTests(TestInfo testInfo) {
4246
assertThat(testInfo.getTestClass()).contains(TestInfoParameterResolverTests.class);
@@ -54,11 +58,13 @@ void providedDisplayName(TestInfo testInfo) {
5458
assertEquals("custom display name", testInfo.getDisplayName());
5559
}
5660

57-
// TODO Update test to expect an exception once #743 is fixed.
5861
@Test
59-
@DisplayName("")
60-
void customDisplayNameThatIsEmpty(TestInfo testInfo) {
61-
assertEquals("customDisplayNameThatIsEmpty(TestInfo)", testInfo.getDisplayName());
62+
void customDisplayNameThatIsEmpty() {
63+
executeTests(request -> request //
64+
.selectors(selectClass(BlankDisplayNameTestCase.class)) //
65+
.configurationParameter(CRITICAL_DISCOVERY_ISSUE_SEVERITY_PROPERTY_NAME, Severity.ERROR.name())) //
66+
.testEvents() //
67+
.assertStatistics(stats -> stats.started(1).succeeded(1));
6268
}
6369

6470
@Test
@@ -88,4 +94,14 @@ static void beforeAndAfterAll(TestInfo testInfo) {
8894
assertEquals(TestInfoParameterResolverTests.class.getSimpleName(), testInfo.getDisplayName());
8995
}
9096

97+
@SuppressWarnings("JUnitMalformedDeclaration")
98+
static class BlankDisplayNameTestCase {
99+
100+
@Test
101+
@DisplayName("")
102+
void test(TestInfo testInfo) {
103+
assertEquals("test(TestInfo)", testInfo.getDisplayName());
104+
}
105+
}
106+
91107
}

0 commit comments

Comments
 (0)