Report warning when @SuiteDisplayName is blank
#4822
Conversation
Report WARNING when @SuiteDisplayName contains blank or whitespace-only values.
@SuiteDisplayName is blank
Indeed, we do not want to use Even though I did not explicitly state it in #4810, the intention was to make use of the discovery issue mechanism to report a warning like with Though, for 6.0 I personally think it should be an error instead of a warning. @marcphilipp, thoughts? |
There was a problem hiding this comment.
I took a quick glance and think this PR looks pretty good; however, I will probably let @marcphilipp review it as well.
In addition, I requested two very minor changes to Javadoc.
...sts/src/test/java/org/junit/platform/suite/engine/testsuites/BlankSuiteDisplayNameSuite.java
Outdated
Show resolved
Hide resolved
...rc/test/java/org/junit/platform/suite/engine/testsuites/WhitespaceSuiteDisplayNameSuite.java
Outdated
Show resolved
Hide resolved
|
@sbrannen |
| @@ -149,6 +150,16 @@ private static String getSuiteDisplayName(Class<?> testClass) { | |||
| // @formatter:on | |||
| } | |||
|
|
|||
| private static void inspectSuiteDisplayName(Class<?> suiteClass, DiscoveryIssueReporter issueReporter) { | |||
There was a problem hiding this comment.
I think it would be better to address this in one place by changing getSuiteDisplayName above to the following:
private static String getSuiteDisplayName(Class<?> suiteClass, DiscoveryIssueReporter issueReporter) {
var nonBlank = issueReporter.createReportingCondition(StringUtils::isNotBlank, __ -> {
String message = "@SuiteDisplayName on %s must be declared with a non-blank value.".formatted(
suiteClass.getName());
return DiscoveryIssue.builder(Severity.WARNING, message) //
.source(ClassSource.from(suiteClass)) //
.build();
}).toPredicate();
return findAnnotation(suiteClass, SuiteDisplayName.class) //
.map(SuiteDisplayName::value) //
.filter(nonBlank) //
.orElse(suiteClass.getSimpleName());
}There was a problem hiding this comment.
@marcphilipp
I think that’s a good suggestion.
I will reflect it!
|
@marcphilipp class SomethingDiscoveryIssueReporter implements DiscoveryIssueReporter {
// ...
@Override
public <T> Condition<T> createReportingCondition(Predicate<T> predicate, Function<T, DiscoveryIssue> issueCreator) {
return null;
}
// ....
} |
Purpose
This PR adds a discovery-time validation for
@SuiteDisplayNamein the Suite Engine.If the annotation value is blank or whitespace-only, the engine now reports a
DiscoveryIssuewith WARNING severity.Related issue
@SuiteDisplayName#4810Key Changes
SuiteTestDescriptor: invokeinspectSuiteDisplayName(...)during construction and report a warning when needed.(referred to DisplayNameUtils.validateAnnotation().)
Preconditions.notBlank()to throw aPreconditionViolationExceptionwhen the value contains only whitespace.However, to maintain backward compatibility, I decided that reporting a warning would be more appropriate than throwing an exception.
Definition of Done
@APIannotations