Skip to content

Commit

Permalink
Check for DisplayNameGeneration annotations on runtime enclosing types
Browse files Browse the repository at this point in the history
Prior to this commit, the `DisplayNameGeneration` annotation was
discovered on the current test, a superclass, or the enclosing class 
in which a `Nested` test class is declared. However, the runtime type of
an enclosing instance is not always identical to the class in which the
`Nested` test class was declared. This caused annotations on runtime
enclosing types to be missed when checking for them in 
`IndicativeSentences` which resulted in unexpected display names. Now, 
the runtime enclosing types are checked rather than the compile-time 
ones.

Resolves #4131.

---------

Co-authored-by: Sam Brannen <104798+sbrannen@users.noreply.github.com>
  • Loading branch information
marcphilipp and sbrannen authored Jan 27, 2025
1 parent 3fa27e9 commit ed68831
Show file tree
Hide file tree
Showing 9 changed files with 225 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ JUnit repository on GitHub.
[[release-notes-5.12.0-M1-junit-platform-deprecations-and-breaking-changes]]
==== Deprecations and Breaking Changes

* ❓
* `SearchOption` and `AnnotationSupport.findAnnotation(Class, Class, SearchOption)` from
`junit-platform-commons` have been deprecated.

[[release-notes-5.12.0-M1-junit-platform-new-features-and-improvements]]
==== New Features and Improvements
Expand Down Expand Up @@ -75,6 +76,8 @@ JUnit repository on GitHub.
to the most recent test or container that was started or has written output.
* New public interface `ClasspathScanner` allowing third parties to provide a custom
implementation for scanning the classpath for classes and resources.
* New `AnnotationSupport.findAnnotation(Class, Class, List)` method to support searching
for an annotation on an inner class and its runtime enclosing instance types.


[[release-notes-5.12.0-M1-junit-jupiter]]
Expand All @@ -87,6 +90,9 @@ JUnit repository on GitHub.
to `DisplayNameGenerator` implementations. Prior to this change, such generators were
only able to access the enclosing class in which `@Nested` was declared, but they could
not access the concrete runtime type of the enclosing instance.
* `@DisplayNameGeneration` annotations are now discovered on the _runtime_ enclosing types
of `@Nested` test classes instead of the compile-time enclosing class in which the
`@Nested` class was _declared_.

[[release-notes-5.12.0-M1-junit-jupiter-deprecations-and-breaking-changes]]
==== Deprecations and Breaking Changes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import static java.util.Collections.emptyList;
import static org.apiguardian.api.API.Status.DEPRECATED;
import static org.apiguardian.api.API.Status.EXPERIMENTAL;
import static org.apiguardian.api.API.Status.INTERNAL;
import static org.apiguardian.api.API.Status.STABLE;
import static org.junit.platform.commons.support.AnnotationSupport.findAnnotation;
import static org.junit.platform.commons.support.ModifierSupport.isStatic;
Expand All @@ -24,7 +25,6 @@

import org.apiguardian.api.API;
import org.junit.platform.commons.support.ReflectionSupport;
import org.junit.platform.commons.support.SearchOption;
import org.junit.platform.commons.util.ClassUtils;
import org.junit.platform.commons.util.Preconditions;

Expand Down Expand Up @@ -315,23 +315,24 @@ public IndicativeSentences() {

@Override
public String generateDisplayNameForClass(Class<?> testClass) {
return getGeneratorFor(testClass).generateDisplayNameForClass(testClass);
return getGeneratorFor(testClass, emptyList()).generateDisplayNameForClass(testClass);
}

@Override
public String generateDisplayNameForNestedClass(List<Class<?>> enclosingInstanceTypes, Class<?> nestedClass) {
return getSentenceBeginning(enclosingInstanceTypes, nestedClass);
return getSentenceBeginning(nestedClass, enclosingInstanceTypes);
}

@Override
public String generateDisplayNameForMethod(List<Class<?>> enclosingInstanceTypes, Class<?> testClass,
Method testMethod) {
return getSentenceBeginning(enclosingInstanceTypes, testClass) + getFragmentSeparator(testClass)
+ getGeneratorFor(testClass).generateDisplayNameForMethod(enclosingInstanceTypes, testClass,
testMethod);
return getSentenceBeginning(testClass, enclosingInstanceTypes)
+ getFragmentSeparator(testClass, enclosingInstanceTypes)
+ getGeneratorFor(testClass, enclosingInstanceTypes).generateDisplayNameForMethod(
enclosingInstanceTypes, testClass, testMethod);
}

private String getSentenceBeginning(List<Class<?>> enclosingInstanceTypes, Class<?> testClass) {
private String getSentenceBeginning(Class<?> testClass, List<Class<?>> enclosingInstanceTypes) {
Class<?> enclosingClass = enclosingInstanceTypes.isEmpty() ? null
: enclosingInstanceTypes.get(enclosingInstanceTypes.size() - 1);
boolean topLevelTestClass = (enclosingClass == null || isStatic(testClass));
Expand All @@ -342,33 +343,35 @@ private String getSentenceBeginning(List<Class<?>> enclosingInstanceTypes, Class
if (displayName.isPresent()) {
return displayName.get();
}
Class<? extends DisplayNameGenerator> generatorClass = findDisplayNameGeneration(testClass)//
.map(DisplayNameGeneration::value)//
.filter(not(IndicativeSentences.class))//
.orElse(null);
Class<? extends DisplayNameGenerator> generatorClass = findDisplayNameGeneration(testClass,
enclosingInstanceTypes)//
.map(DisplayNameGeneration::value)//
.filter(not(IndicativeSentences.class))//
.orElse(null);
if (generatorClass != null) {
return getDisplayNameGenerator(generatorClass).generateDisplayNameForClass(testClass);
}
return generateDisplayNameForClass(testClass);
}

List<Class<?>> remainingEnclosingInstanceTypes = enclosingInstanceTypes.isEmpty() ? emptyList()
: enclosingInstanceTypes.subList(0, enclosingInstanceTypes.size() - 1);

// Only build prefix based on the enclosing class if the enclosing
// class is also configured to use the IndicativeSentences generator.
boolean buildPrefix = findDisplayNameGeneration(enclosingClass)//
boolean buildPrefix = findDisplayNameGeneration(enclosingClass, remainingEnclosingInstanceTypes)//
.map(DisplayNameGeneration::value)//
.filter(IndicativeSentences.class::equals)//
.isPresent();

List<Class<?>> remainingEnclosingInstanceTypes = enclosingInstanceTypes.isEmpty() ? emptyList()
: enclosingInstanceTypes.subList(0, enclosingInstanceTypes.size() - 1);

String prefix = (buildPrefix
? getSentenceBeginning(remainingEnclosingInstanceTypes, enclosingClass)
+ getFragmentSeparator(testClass)
? getSentenceBeginning(enclosingClass, remainingEnclosingInstanceTypes)
+ getFragmentSeparator(testClass, enclosingInstanceTypes)
: "");

return prefix + displayName.orElseGet(() -> getGeneratorFor(testClass).generateDisplayNameForNestedClass(
remainingEnclosingInstanceTypes, testClass));
return prefix + displayName.orElseGet(
() -> getGeneratorFor(testClass, enclosingInstanceTypes).generateDisplayNameForNestedClass(
remainingEnclosingInstanceTypes, testClass));
}

/**
Expand All @@ -381,10 +384,12 @@ private String getSentenceBeginning(List<Class<?>> enclosingInstanceTypes, Class
* will be used.
*
* @param testClass the test class to search on for {@code @IndicativeSentencesGeneration}
* @param enclosingInstanceTypes the runtime types of the enclosing
* instances; never {@code null}
* @return the sentence fragment separator
*/
private static String getFragmentSeparator(Class<?> testClass) {
return findIndicativeSentencesGeneration(testClass)//
private static String getFragmentSeparator(Class<?> testClass, List<Class<?>> enclosingInstanceTypes) {
return findIndicativeSentencesGeneration(testClass, enclosingInstanceTypes)//
.map(IndicativeSentencesGeneration::separator)//
.orElse(IndicativeSentencesGeneration.DEFAULT_SEPARATOR);
}
Expand All @@ -399,10 +404,12 @@ private static String getFragmentSeparator(Class<?> testClass) {
* will be used.
*
* @param testClass the test class to search on for {@code @IndicativeSentencesGeneration}
* @param enclosingInstanceTypes the runtime types of the enclosing
* instances; never {@code null}
* @return the {@code DisplayNameGenerator} instance to use
*/
private static DisplayNameGenerator getGeneratorFor(Class<?> testClass) {
return findIndicativeSentencesGeneration(testClass)//
private static DisplayNameGenerator getGeneratorFor(Class<?> testClass, List<Class<?>> enclosingInstanceTypes) {
return findIndicativeSentencesGeneration(testClass, enclosingInstanceTypes)//
.map(IndicativeSentencesGeneration::generator)//
.filter(not(IndicativeSentences.class))//
.map(DisplayNameGenerator::getDisplayNameGenerator)//
Expand All @@ -412,26 +419,32 @@ private static DisplayNameGenerator getGeneratorFor(Class<?> testClass) {
/**
* Find the first {@code DisplayNameGeneration} annotation that is either
* <em>directly present</em>, <em>meta-present</em>, or <em>indirectly present</em>
* on the supplied {@code testClass} or on an enclosing class.
* on the supplied {@code testClass} or on an enclosing instance type.
*
* @param testClass the test class on which to find the annotation; never {@code null}
* @param enclosingInstanceTypes the runtime types of the enclosing
* instances; never {@code null}
* @return an {@code Optional} containing the annotation, potentially empty if not found
*/
private static Optional<DisplayNameGeneration> findDisplayNameGeneration(Class<?> testClass) {
return findAnnotation(testClass, DisplayNameGeneration.class, SearchOption.INCLUDE_ENCLOSING_CLASSES);
@API(status = INTERNAL, since = "5.12")
private static Optional<DisplayNameGeneration> findDisplayNameGeneration(Class<?> testClass,
List<Class<?>> enclosingInstanceTypes) {
return findAnnotation(testClass, DisplayNameGeneration.class, enclosingInstanceTypes);
}

/**
* Find the first {@code IndicativeSentencesGeneration} annotation that is either
* <em>directly present</em>, <em>meta-present</em>, or <em>indirectly present</em>
* on the supplied {@code testClass} or on an enclosing class.
* on the supplied {@code testClass} or on an enclosing instance type.
*
* @param testClass the test class on which to find the annotation; never {@code null}
* @param enclosingInstanceTypes the runtime types of the enclosing
* instances; never {@code null}
* @return an {@code Optional} containing the annotation, potentially empty if not found
*/
private static Optional<IndicativeSentencesGeneration> findIndicativeSentencesGeneration(Class<?> testClass) {
return findAnnotation(testClass, IndicativeSentencesGeneration.class,
SearchOption.INCLUDE_ENCLOSING_CLASSES);
private static Optional<IndicativeSentencesGeneration> findIndicativeSentencesGeneration(Class<?> testClass,
List<Class<?>> enclosingInstanceTypes) {
return findAnnotation(testClass, IndicativeSentencesGeneration.class, enclosingInstanceTypes);
}

private static Predicate<Class<?>> not(Class<?> clazz) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@

import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Method;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.function.Function;
import java.util.function.BiFunction;
import java.util.function.Supplier;

import org.junit.jupiter.api.DisplayName;
Expand All @@ -30,7 +31,6 @@
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.support.SearchOption;
import org.junit.platform.commons.util.Preconditions;
import org.junit.platform.commons.util.StringUtils;

Expand Down Expand Up @@ -97,33 +97,43 @@ static String determineDisplayNameForMethod(Supplier<List<Class<?>>> enclosingIn
}

static Supplier<String> createDisplayNameSupplierForClass(Class<?> testClass, JupiterConfiguration configuration) {
return createDisplayNameSupplier(testClass, configuration,
generator -> generator.generateDisplayNameForClass(testClass));
return createDisplayNameSupplier(Collections::emptyList, testClass, configuration,
(generator, __) -> generator.generateDisplayNameForClass(testClass));
}

static Supplier<String> createDisplayNameSupplierForNestedClass(Supplier<List<Class<?>>> enclosingInstanceTypes,
Class<?> testClass, JupiterConfiguration configuration) {
return createDisplayNameSupplier(testClass, configuration,
generator -> generator.generateDisplayNameForNestedClass(enclosingInstanceTypes.get(), testClass));
static Supplier<String> createDisplayNameSupplierForNestedClass(
Supplier<List<Class<?>>> enclosingInstanceTypesSupplier, Class<?> testClass,
JupiterConfiguration configuration) {
return createDisplayNameSupplier(enclosingInstanceTypesSupplier, testClass, configuration,
(generator, enclosingInstanceTypes) -> generator.generateDisplayNameForNestedClass(enclosingInstanceTypes,
testClass));
}

private static Supplier<String> createDisplayNameSupplierForMethod(Supplier<List<Class<?>>> enclosingInstanceTypes,
Class<?> testClass, Method testMethod, JupiterConfiguration configuration) {
return createDisplayNameSupplier(testClass, configuration,
generator -> generator.generateDisplayNameForMethod(enclosingInstanceTypes.get(), testClass, testMethod));
private static Supplier<String> createDisplayNameSupplierForMethod(
Supplier<List<Class<?>>> enclosingInstanceTypesSupplier, Class<?> testClass, Method testMethod,
JupiterConfiguration configuration) {
return createDisplayNameSupplier(enclosingInstanceTypesSupplier, testClass, configuration,
(generator, enclosingInstanceTypes) -> generator.generateDisplayNameForMethod(enclosingInstanceTypes,
testClass, testMethod));
}

private static Supplier<String> createDisplayNameSupplier(Class<?> testClass, JupiterConfiguration configuration,
Function<DisplayNameGenerator, String> generatorFunction) {
return () -> findDisplayNameGenerator(testClass) //
.map(generatorFunction) //
.orElseGet(() -> generatorFunction.apply(configuration.getDefaultDisplayNameGenerator()));
private static Supplier<String> createDisplayNameSupplier(Supplier<List<Class<?>>> enclosingInstanceTypesSupplier,
Class<?> testClass, JupiterConfiguration configuration,
BiFunction<DisplayNameGenerator, List<Class<?>>, String> generatorFunction) {
return () -> {
List<Class<?>> enclosingInstanceTypes = enclosingInstanceTypesSupplier.get();
return findDisplayNameGenerator(enclosingInstanceTypes, testClass) //
.map(it -> generatorFunction.apply(it, enclosingInstanceTypes)) //
.orElseGet(() -> generatorFunction.apply(configuration.getDefaultDisplayNameGenerator(),
enclosingInstanceTypes));
};
}

private static Optional<DisplayNameGenerator> findDisplayNameGenerator(Class<?> testClass) {
private static Optional<DisplayNameGenerator> findDisplayNameGenerator(List<Class<?>> enclosingInstanceTypes,
Class<?> testClass) {
Preconditions.notNull(testClass, "Test class must not be null");

return findAnnotation(testClass, DisplayNameGeneration.class, SearchOption.INCLUDE_ENCLOSING_CLASSES) //
return findAnnotation(testClass, DisplayNameGeneration.class, enclosingInstanceTypes) //
.map(DisplayNameGeneration::value) //
.map(displayNameGeneratorClass -> {
if (displayNameGeneratorClass == Standard.class) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@

package org.junit.platform.commons.support;

import static org.apiguardian.api.API.Status.DEPRECATED;
import static org.apiguardian.api.API.Status.EXPERIMENTAL;
import static org.apiguardian.api.API.Status.MAINTAINED;
import static org.apiguardian.api.API.Status.STABLE;

import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedElement;
Expand Down Expand Up @@ -163,8 +164,13 @@ public static <A extends Annotation> Optional<A> findAnnotation(AnnotatedElement
* @since 1.8
* @see SearchOption
* @see #findAnnotation(AnnotatedElement, Class)
* @deprecated Use {@link #findAnnotation(Class, Class, List)}
* (for {@code SearchOption.DEFAULT}) or
* {@link #findAnnotation(Class, Class, List)} (for
* {@code SearchOption.INCLUDE_ENCLOSING_CLASSES}) instead
*/
@API(status = STABLE, since = "1.10")
@Deprecated
@API(status = DEPRECATED, since = "1.12")
public static <A extends Annotation> Optional<A> findAnnotation(Class<?> clazz, Class<A> annotationType,
SearchOption searchOption) {

Expand All @@ -174,6 +180,55 @@ public static <A extends Annotation> Optional<A> findAnnotation(Class<?> clazz,
searchOption == SearchOption.INCLUDE_ENCLOSING_CLASSES);
}

/**
* Find the first annotation of the specified type that is either
* <em>directly present</em>, <em>meta-present</em>, or <em>indirectly
* present</em> on the supplied class.
*
* <p>If the annotation is neither <em>directly present</em> nor <em>meta-present</em>
* on the class, this method will additionally search on interfaces implemented
* by the class before searching for an annotation that is <em>indirectly present</em>
* on the class (i.e., within the class inheritance hierarchy).
*
* <p>If the annotation still has not been found, this method will optionally
* search recursively through the supplied enclosing instance types, starting
* at the innermost enclosing class (the last one in the supplied list of
* {@code enclosingInstanceTypes}).
*
* @implNote The classes supplied as {@code enclosingInstanceTypes} may
* differ from the classes returned from invocations of
* {@link Class#getEnclosingClass()} &mdash; for example, when a nested test
* class is inherited from a superclass.
*
* @param <A> the annotation type
* @param clazz the class on which to search for the annotation; may be {@code null}
* @param annotationType the annotation type to search for; never {@code null}
* @param enclosingInstanceTypes the runtime types of the enclosing
* instances for the class, ordered from outermost to innermost,
* excluding {@code clazz}; never {@code null}
* @return an {@code Optional} containing the annotation; never {@code null} but
* potentially empty if {@code clazz} is not an inner class
* @since 1.12
* @see #findAnnotation(AnnotatedElement, Class)
*/
@API(status = EXPERIMENTAL, since = "1.12")
public static <A extends Annotation> Optional<A> findAnnotation(Class<?> clazz, Class<A> annotationType,
List<Class<?>> enclosingInstanceTypes) {

Preconditions.notNull(enclosingInstanceTypes, "enclosingInstanceTypes must not be null");

Optional<A> annotation = findAnnotation(clazz, annotationType);
if (!annotation.isPresent()) {
for (int i = enclosingInstanceTypes.size() - 1; i >= 0; i--) {
annotation = findAnnotation(enclosingInstanceTypes.get(i), annotationType);
if (annotation.isPresent()) {
break;
}
}
}
return annotation;
}

/**
* Find all <em>repeatable</em> {@linkplain Annotation annotations} of the
* supplied {@code annotationType} that are either <em>present</em>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

package org.junit.platform.commons.support;

import static org.apiguardian.api.API.Status.STABLE;
import static org.apiguardian.api.API.Status.DEPRECATED;

import org.apiguardian.api.API;

Expand All @@ -20,8 +20,10 @@
* @since 1.8
* @see #DEFAULT
* @see #INCLUDE_ENCLOSING_CLASSES
* @deprecated because there is only a single non-deprecated search option left
*/
@API(status = STABLE, since = "1.10")
@Deprecated
@API(status = DEPRECATED, since = "1.12")
public enum SearchOption {

/**
Expand All @@ -37,7 +39,11 @@ public enum SearchOption {
* Search the inheritance hierarchy as with the {@link #DEFAULT} search option
* but also search the {@linkplain Class#getEnclosingClass() enclosing class}
* hierarchy for <em>inner classes</em> (i.e., a non-static member classes).
*
* @deprecated because it is preferable to inspect the runtime enclosing
* types of a class rather than where they are declared.
*/
@Deprecated @API(status = DEPRECATED, since = "1.12")
INCLUDE_ENCLOSING_CLASSES

}
Loading

0 comments on commit ed68831

Please sign in to comment.