Fix discovery of inherited nested classes

The discovery algorithm introduced in 5.5 remembers already the
resolution of each discovery selector. Prior to this commit, nested
classes were resolved by adding a ClassSelector for the nested class.
If that ClassSelector has been resolved before, discovery is assumed to
be finished. Similarly, when resolving MethodSelectors for methods
within a nested test class that have been resolved before, discovery
will stop.

This commit introduces dedicated selectors for nested classes and their
methods which take into account the actual enclosing classes that will
be present at runtime. These selectors are package-private for now but
could be made publicly available in a future release.

Fixes #1954.

Author: marcphilipp

documentation/src/docs/asciidoc/release-notes/index.adoc

@@ -15,6 +15,8 @@ authors as well as build tool and IDE vendors.
 
 include::../link-attributes.adoc[]
 
+include::release-notes-5.5.1.adoc[]
+
 include::release-notes-5.5.0.adoc[]
 
 include::release-notes-5.4.2.adoc[]

documentation/src/docs/asciidoc/release-notes/release-notes-5.5.1.adoc

@@ -0,0 +1,58 @@
+[[release-notes-5.5.1]]
+== 5.5.1
+
+*Date of Release:* ❓
+
+*Scope:* Bug fixes since 5.5.0
+
+For a complete list of all _closed_ issues and pull requests for this release, consult
+the link:{junit5-repo}+/milestone/42?closed=1+[5.5.1] milestone page in the JUnit repository
+on GitHub.
+
+
+[[release-notes-5.5.1-junit-platform]]
+=== JUnit Platform
+
+==== Bug Fixes
+
+* ❓
+
+==== Deprecations and Breaking Changes
+
+* ❓
+
+==== New Features and Improvements
+
+* ❓
+
+
+[[release-notes-5.5.1-junit-jupiter]]
+=== JUnit Jupiter
+
+==== Bug Fixes
+
+* Fix test discovery and execution of inherited `@Nested` classes.
+
+==== Deprecations and Breaking Changes
+
+* ❓
+
+==== New Features and Improvements
+
+* ❓
+
+
+[[release-notes-5.5.1-junit-vintage]]
+=== JUnit Vintage
+
+==== Bug Fixes
+
+* ❓
+
+==== Deprecations and Breaking Changes
+
+* ❓
+
+==== New Features and Improvements
+
+* ❓

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

@@ -104,6 +104,8 @@ public final Class<?> getTestClass() {
 		return this.testClass;
 	}
 
+	public abstract List<Class<?>> getEnclosingTestClasses();
+
 	@Override
 	public Type getType() {
 		return Type.CONTAINER;

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassTestDescriptor.java

@@ -10,10 +10,12 @@
 
 package org.junit.jupiter.engine.descriptor;
 
+import static java.util.Collections.emptyList;
 import static org.apiguardian.api.API.Status.INTERNAL;
 import static org.junit.jupiter.engine.descriptor.DisplayNameUtils.createDisplayNameSupplierForClass;
 
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 
@@ -56,6 +58,11 @@ public Set<TestTag> getTags() {
 		return new LinkedHashSet<>(this.tags);
 	}
 
+	@Override
+	public List<Class<?>> getEnclosingTestClasses() {
+		return emptyList();
+	}
+
 	// --- Node ----------------------------------------------------------------
 
 	@Override

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/NestedClassTestDescriptor.java

@@ -10,10 +10,13 @@
 
 package org.junit.jupiter.engine.descriptor;
 
+import static java.util.Collections.emptyList;
 import static org.apiguardian.api.API.Status.INTERNAL;
 import static org.junit.jupiter.engine.descriptor.DisplayNameUtils.createDisplayNameSupplierForNestedClass;
 
+import java.util.ArrayList;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 
@@ -57,6 +60,18 @@ public final Set<TestTag> getTags() {
 		return allTags;
 	}
 
+	@Override
+	public List<Class<?>> getEnclosingTestClasses() {
+		TestDescriptor parent = getParent().orElse(null);
+		if (parent instanceof ClassBasedTestDescriptor) {
+			ClassBasedTestDescriptor parentClassDescriptor = (ClassBasedTestDescriptor) parent;
+			List<Class<?>> result = new ArrayList<>(parentClassDescriptor.getEnclosingTestClasses());
+			result.add(parentClassDescriptor.getTestClass());
+			return result;
+		}
+		return emptyList();
+	}
+
 	// --- Node ----------------------------------------------------------------
 
 	@Override

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/ClassSelectorResolver.java

@@ -16,12 +16,13 @@
 import static org.junit.platform.commons.support.ReflectionSupport.findNestedClasses;
 import static org.junit.platform.commons.util.FunctionUtils.where;
 import static org.junit.platform.commons.util.ReflectionUtils.findMethods;
-import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
-import static org.junit.platform.engine.discovery.DiscoverySelectors.selectMethod;
 import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId;
 import static org.junit.platform.engine.support.discovery.SelectorResolver.Resolution.unresolved;
 
+import java.lang.reflect.Method;
+import java.util.ArrayList;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Predicate;
@@ -40,7 +41,6 @@
 import org.junit.platform.engine.UniqueId;
 import org.junit.platform.engine.discovery.ClassSelector;
 import org.junit.platform.engine.discovery.DiscoverySelectors;
-import org.junit.platform.engine.discovery.MethodSelector;
 import org.junit.platform.engine.discovery.UniqueIdSelector;
 import org.junit.platform.engine.support.discovery.SelectorResolver;
 
@@ -71,12 +71,24 @@ public Resolution resolve(ClassSelector selector, Context context) {
 			}
 		}
 		else if (isNestedTestClass.test(testClass)) {
-			return toResolution(context.addToParent(() -> selectClass(testClass.getEnclosingClass()),
+			return toResolution(context.addToParent(() -> DiscoverySelectors.selectClass(testClass.getEnclosingClass()),
 				parent -> Optional.of(newNestedClassTestDescriptor(parent, testClass))));
 		}
 		return unresolved();
 	}
 
+	@Override
+	public Resolution resolve(DiscoverySelector selector, Context context) {
+		if (selector instanceof NestedClassSelector) {
+			NestedClassSelector nestedClassSelector = (NestedClassSelector) selector;
+			if (isNestedTestClass.test(nestedClassSelector.getNestedClass())) {
+				return toResolution(context.addToParent(() -> selectClass(nestedClassSelector.getEnclosingClasses()),
+					parent -> Optional.of(newNestedClassTestDescriptor(parent, nestedClassSelector.getNestedClass()))));
+			}
+		}
+		return unresolved();
+	}
+
 	@Override
 	public Resolution resolve(UniqueIdSelector selector, Context context) {
 		UniqueId uniqueId = selector.getUniqueId();
@@ -93,7 +105,6 @@ public Resolution resolve(UniqueIdSelector selector, Context context) {
 			return toResolution(context.addToParent(() -> selectUniqueId(uniqueId.removeLastSegment()), parent -> {
 				if (parent instanceof ClassBasedTestDescriptor) {
 					Class<?> parentTestClass = ((ClassBasedTestDescriptor) parent).getTestClass();
-					// TODO add test for resolving unique id of inherited nested test class
 					return ReflectionUtils.findNestedClasses(parentTestClass,
 						isNestedTestClass.and(
 							where(Class::getSimpleName, isEqual(simpleClassName)))).stream().findFirst().flatMap(
@@ -120,16 +131,34 @@ private NestedClassTestDescriptor newNestedClassTestDescriptor(TestDescriptor pa
 	private Resolution toResolution(Optional<? extends ClassBasedTestDescriptor> testDescriptor) {
 		return testDescriptor.map(it -> {
 			Class<?> testClass = it.getTestClass();
+			List<Class<?>> testClasses = new ArrayList<>(it.getEnclosingTestClasses());
+			testClasses.add(testClass);
 			// @formatter:off
 			return Resolution.match(Match.exact(it, () -> {
-				Stream<MethodSelector> methods = findMethods(testClass, isTestOrTestFactoryOrTestTemplateMethod).stream()
-						.map(method -> selectMethod(testClass, method));
-				Stream<ClassSelector> nestedClasses = findNestedClasses(testClass, isNestedTestClass).stream()
-						.map(DiscoverySelectors::selectClass);
+				Stream<DiscoverySelector> methods = findMethods(testClass, isTestOrTestFactoryOrTestTemplateMethod).stream()
+						.map(method -> selectMethod(testClasses, method));
+				Stream<NestedClassSelector> nestedClasses = findNestedClasses(testClass, isNestedTestClass).stream()
+						.map(nestedClass -> new NestedClassSelector(testClasses, nestedClass));
 				return Stream.concat(methods, nestedClasses).collect(toCollection((Supplier<Set<DiscoverySelector>>) LinkedHashSet::new));
 			}));
 			// @formatter:on
 		}).orElse(unresolved());
 	}
 
+	private DiscoverySelector selectClass(List<Class<?>> classes) {
+		if (classes.size() == 1) {
+			return DiscoverySelectors.selectClass(classes.get(0));
+		}
+		int lastIndex = classes.size() - 1;
+		return new NestedClassSelector(classes.subList(0, lastIndex), classes.get(lastIndex));
+	}
+
+	private DiscoverySelector selectMethod(List<Class<?>> classes, Method method) {
+		if (classes.size() == 1) {
+			return DiscoverySelectors.selectMethod(classes.get(0), method);
+		}
+		int lastIndex = classes.size() - 1;
+		return new NestedMethodSelector(classes.subList(0, lastIndex), classes.get(lastIndex), method);
+	}
+
 }

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodSelectorResolver.java

@@ -10,17 +10,18 @@
 
 package org.junit.jupiter.engine.discovery;
 
+import static java.util.Collections.emptyList;
 import static java.util.Collections.emptySet;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
-import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
 import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId;
 import static org.junit.platform.engine.support.discovery.SelectorResolver.Resolution.matches;
 import static org.junit.platform.engine.support.discovery.SelectorResolver.Resolution.unresolved;
 
 import java.lang.reflect.Method;
 import java.util.Arrays;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Predicate;
@@ -43,6 +44,7 @@
 import org.junit.platform.engine.DiscoverySelector;
 import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.UniqueId;
+import org.junit.platform.engine.discovery.DiscoverySelectors;
 import org.junit.platform.engine.discovery.MethodSelector;
 import org.junit.platform.engine.discovery.UniqueIdSelector;
 import org.junit.platform.engine.support.discovery.SelectorResolver;
@@ -63,9 +65,23 @@ class MethodSelectorResolver implements SelectorResolver {
 
 	@Override
 	public Resolution resolve(MethodSelector selector, Context context) {
+		return resolve(context, emptyList(), selector.getJavaClass(), selector.getJavaMethod());
+	}
+
+	@Override
+	public Resolution resolve(DiscoverySelector selector, Context context) {
+		if (selector instanceof NestedMethodSelector) {
+			NestedMethodSelector nestedMethodSelector = (NestedMethodSelector) selector;
+			return resolve(context, nestedMethodSelector.getEnclosingClasses(), nestedMethodSelector.getNestedClass(),
+				nestedMethodSelector.getMethod());
+		}
+		return unresolved();
+	}
+
+	private Resolution resolve(Context context, List<Class<?>> enclosingClasses, Class<?> testClass, Method method) {
 		// @formatter:off
 		Set<Match> matches = Arrays.stream(MethodType.values())
-				.map(methodType -> methodType.resolveMethodSelector(selector, context, configuration))
+				.map(methodType -> methodType.resolve(enclosingClasses, testClass, method, context, configuration))
 				.filter(Optional::isPresent)
 				.map(Optional::get)
 				.map(testDescriptor -> Match.exact(testDescriptor, expansionCallback(testDescriptor)))
@@ -78,8 +94,7 @@ public Resolution resolve(MethodSelector selector, Context context) {
 					"Possible configuration error: method [%s] resulted in multiple TestDescriptors %s. "
 							+ "This is typically the result of annotating a method with multiple competing annotations "
 							+ "such as @Test, @RepeatedTest, @ParameterizedTest, @TestFactory, etc.",
-					selector.getJavaMethod().toGenericString(),
-					testDescriptors.map(d -> d.getClass().getName()).collect(toList()));
+					method.toGenericString(), testDescriptors.map(d -> d.getClass().getName()).collect(toList()));
 			});
 		}
 		return matches.isEmpty() ? unresolved() : matches(matches);
@@ -159,18 +174,23 @@ protected TestDescriptor createTestDescriptor(UniqueId uniqueId, Class<?> testCl
 			this.dynamicDescendantSegmentTypes = new LinkedHashSet<>(Arrays.asList(dynamicDescendantSegmentTypes));
 		}
 
-		private Optional<TestDescriptor> resolveMethodSelector(MethodSelector selector, Context resolver,
-				JupiterConfiguration configuration) {
-			if (!methodPredicate.test(selector.getJavaMethod())) {
+		private Optional<TestDescriptor> resolve(List<Class<?>> enclosingClasses, Class<?> testClass, Method method,
+				Context context, JupiterConfiguration configuration) {
+			if (!methodPredicate.test(method)) {
 				return Optional.empty();
 			}
-			Class<?> testClass = selector.getJavaClass();
-			Method method = selector.getJavaMethod();
-			return resolver.addToParent(() -> selectClass(testClass), //
+			return context.addToParent(() -> selectClass(enclosingClasses, testClass), //
 				parent -> Optional.of(
 					createTestDescriptor(createUniqueId(method, parent), testClass, method, configuration)));
 		}
 
+		private DiscoverySelector selectClass(List<Class<?>> enclosingClasses, Class<?> testClass) {
+			if (enclosingClasses.isEmpty()) {
+				return DiscoverySelectors.selectClass(testClass);
+			}
+			return new NestedClassSelector(enclosingClasses, testClass);
+		}
+
 		private Optional<TestDescriptor> resolveUniqueIdIntoTestDescriptor(UniqueId uniqueId, Context context,
 				JupiterConfiguration configuration) {
 			UniqueId.Segment lastSegment = uniqueId.getLastSegment();

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/NestedClassSelector.java

@@ -0,0 +1,57 @@
+/*
+ * Copyright 2015-2019 the original author or authors.
+ *
+ * All rights reserved. This program and the accompanying materials are
+ * made available under the terms of the Eclipse Public License v2.0 which
+ * accompanies this distribution and is available at
+ *
+ * https://www.eclipse.org/legal/epl-v20.html
+ */
+
+package org.junit.jupiter.engine.discovery;
+
+import java.util.List;
+import java.util.Objects;
+
+import org.junit.platform.commons.util.Preconditions;
+import org.junit.platform.engine.DiscoverySelector;
+
+/**
+ * @since 5.5.1
+ */
+class NestedClassSelector implements DiscoverySelector {
+
+	private final List<Class<?>> enclosingClasses;
+	private final Class<?> nestedClass;
+
+	NestedClassSelector(List<Class<?>> enclosingClasses, Class<?> nestedClass) {
+		this.enclosingClasses = Preconditions.notEmpty(enclosingClasses, "enclosingClasses must not be null or empty");
+		this.nestedClass = Preconditions.notNull(nestedClass, "nestedClass must not be null");
+	}
+
+	List<Class<?>> getEnclosingClasses() {
+		return enclosingClasses;
+	}
+
+	Class<?> getNestedClass() {
+		return nestedClass;
+	}
+
+	@Override
+	public boolean equals(Object o) {
+		if (this == o) {
+			return true;
+		}
+		if (o == null || getClass() != o.getClass()) {
+			return false;
+		}
+		NestedClassSelector that = (NestedClassSelector) o;
+		return enclosingClasses.equals(that.enclosingClasses) && nestedClass.equals(that.nestedClass);
+	}
+
+	@Override
+	public int hashCode() {
+		return Objects.hash(enclosingClasses, nestedClass);
+	}
+
+}