Inner classes can be handled by resolver but still fail in engine tests

junit5-engine/src/main/java/org/junit/gen5/engine/junit5/descriptor/ClassTestDescriptor.java

@@ -11,8 +11,6 @@
 package org.junit.gen5.engine.junit5.descriptor;
 
 import lombok.Data;
-import lombok.EqualsAndHashCode;
-
 import org.junit.gen5.commons.util.Preconditions;
 import org.junit.gen5.engine.TestDescriptor;
 
@@ -40,6 +38,9 @@ public ClassTestDescriptor(Class<?> testClass, TestDescriptor parent) {
 
 	@Override
 	public final String getUniqueId() {
+		if (testClass.isMemberClass()) {
+			return this.parent.getUniqueId() + "$" + testClass.getSimpleName();
+		}
 		return this.parent.getUniqueId() + ":" + testClass.getName();
 	}
 

junit5-engine/src/main/java/org/junit/gen5/engine/junit5/descriptor/SpecificationResolver.java

@@ -17,7 +17,6 @@
 import java.lang.reflect.Method;
 import java.util.List;
 import java.util.Set;
-
 import org.junit.gen5.commons.util.AnnotationUtils;
 import org.junit.gen5.engine.ClassNameSpecification;
 import org.junit.gen5.engine.TestDescriptor;
@@ -67,22 +66,26 @@ private void resolveUniqueIdSpecification(UniqueIdSpecification uniqueIdSpecific
 	}
 
 	private void resolveUniqueId(UniqueIdSpecification uniqueIdSpecification, TestDescriptor parent,
-			UniqueIdParts uniqueIdRest) {
-		String part = uniqueIdRest.pop();
-		if (part.isEmpty()) {
+		UniqueIdParts uniqueIdRest) {
+		String idPart = uniqueIdRest.pop();
+		if (idPart.isEmpty()) {
 			return;
 		}
-		switch (part.charAt(0)) {
+		switch (idPart.charAt(0)) {
 			case ':':
-				TestDescriptor classDescriptor = resolveClassByUniqueId(parent, part, uniqueIdRest);
+				TestDescriptor classDescriptor = resolveClassByUniqueId(parent, idPart, uniqueIdRest);
 				resolveUniqueId(uniqueIdSpecification, classDescriptor, uniqueIdRest);
 				break;
+			case '$':
+				TestDescriptor classDescriptor2 = resolveClassByUniqueId(parent, idPart, uniqueIdRest);
+				resolveUniqueId(uniqueIdSpecification, classDescriptor2, uniqueIdRest);
+				break;
 			case '#':
-				TestDescriptor methodDescriptor = resolveMethodByUniqueId(parent, part, uniqueIdRest);
+				TestDescriptor methodDescriptor = resolveMethodByUniqueId(parent, idPart, uniqueIdRest);
 				resolveUniqueId(uniqueIdSpecification, methodDescriptor, uniqueIdRest);
 				break;
 			default:
-				throwCannotResolveException(uniqueIdSpecification.getUniqueId(), part);
+				throwCannotResolveException(uniqueIdSpecification.getUniqueId(), idPart);
 		}
 	}
 
@@ -92,15 +95,18 @@ private TestDescriptor resolveMethodByUniqueId(TestDescriptor parent, String par
 		return methodDescriptor;
 	}
 
-	private MethodTestDescriptor getMethodTestDescriptor(TestDescriptor parent, String part) {
-		String methodName = part.substring(1, part.length() - 2);
+	private MethodTestDescriptor getMethodTestDescriptor(TestDescriptor parent, String idPart) {
+		String methodName = idPart.substring(1, idPart.length() - 2);
 		ClassTestDescriptor classDescriptor = (ClassTestDescriptor) parent;
 		Method method = null;
 		try {
 			method = classDescriptor.getTestClass().getDeclaredMethod(methodName, new Class[0]);
+			if (!methodTester.accept(method)) {
+				throwCannotResolveException(parent.getUniqueId() + idPart, idPart);
+			}
 		}
 		catch (NoSuchMethodException nsme) {
-			throwCannotResolveException(parent.getUniqueId() + part, part);
+			throwCannotResolveException(parent.getUniqueId() + idPart, idPart);
 		}
 		return new MethodTestDescriptor(method, classDescriptor);
 
@@ -138,12 +144,15 @@ private void resolveClassChildren(ClassTestDescriptor parent) {
 
 	private ClassTestDescriptor getClassTestDescriptor(TestDescriptor parent, String uniqueIdPart) {
 		String className = uniqueIdPart.substring(1);
+		if (parent instanceof ClassTestDescriptor) {
+			className = ((ClassTestDescriptor) parent).getTestClass().getName() + uniqueIdPart;
+		}
 		Class<?> clazz = null;
 		try {
 			clazz = loadClass(className);
 		}
 		catch (ClassNotFoundException e) {
-			throwCannotResolveException(parent + uniqueIdPart, uniqueIdPart);
+			throwCannotResolveException(parent.getUniqueId() + uniqueIdPart, uniqueIdPart);
 		}
 		ClassTestDescriptor newDescriptor = new ClassTestDescriptor(clazz, parent);
 		ClassTestDescriptor existingDescriptor = (ClassTestDescriptor) descriptorByUniqueId(
@@ -169,9 +178,9 @@ private TestDescriptor descriptorByUniqueId(String uniqueId) {
 		return null;
 	}
 
-	private void throwCannotResolveException(String fullUniqueId, String part) {
+	private void throwCannotResolveException(String fullUniqueId, String uniqueIdPart) {
 		throw new IllegalArgumentException(
-			String.format("Cannot resolve part '%s' of unique id '%s'", part, fullUniqueId));
+			String.format("Cannot resolve part '%s' of unique id '%s'", uniqueIdPart, fullUniqueId));
 	}
 
 }

junit5-engine/src/test/java/org/junit/gen5/engine/junit5/SpecificationResolverTest.java

@@ -80,13 +80,48 @@ public void testClassResolutionByUniqueId() {
 		Assert.assertTrue(uniqueIds.contains("junit5:org.junit.gen5.engine.junit5.MyTestClass#test2()"));
 	}
 
+	@org.junit.Test
+	public void testInnerClassResolutionByUniqueId() {
+		UniqueIdSpecification specification = new UniqueIdSpecification(
+			"junit5:org.junit.gen5.engine.junit5.OtherTestClass$InnerTestClass");
+
+		resolver.resolveElement(specification);
+
+		Assert.assertEquals(4, descriptors.size());
+		Set uniqueIds = descriptors.stream().map(d -> d.getUniqueId()).collect(Collectors.toSet());
+		Assert.assertTrue(uniqueIds.contains("junit5:org.junit.gen5.engine.junit5.OtherTestClass"));
+		Assert.assertTrue(uniqueIds.contains("junit5:org.junit.gen5.engine.junit5.OtherTestClass$InnerTestClass"));
+		Assert.assertTrue(uniqueIds.contains("junit5:org.junit.gen5.engine.junit5.OtherTestClass$InnerTestClass#test5()"));
+		Assert.assertTrue(uniqueIds.contains("junit5:org.junit.gen5.engine.junit5.OtherTestClass$InnerTestClass#test6()"));
+	}
+
+	@org.junit.Test
+	public void testMethodOfInnerClassByUniqueId() {
+		UniqueIdSpecification specification = new UniqueIdSpecification(
+			"junit5:org.junit.gen5.engine.junit5.OtherTestClass$InnerTestClass#test5()");
+
+		resolver.resolveElement(specification);
+
+		Assert.assertEquals(3, descriptors.size());
+		Set uniqueIds = descriptors.stream().map(d -> d.getUniqueId()).collect(Collectors.toSet());
+		Assert.assertTrue(uniqueIds.contains("junit5:org.junit.gen5.engine.junit5.OtherTestClass"));
+		Assert.assertTrue(uniqueIds.contains("junit5:org.junit.gen5.engine.junit5.OtherTestClass$InnerTestClass"));
+		Assert.assertTrue(uniqueIds.contains("junit5:org.junit.gen5.engine.junit5.OtherTestClass$InnerTestClass#test5()"));
+	}
+
 	@org.junit.Test(expected = IllegalArgumentException.class)
 	public void testNonResolvableUniqueId() {
 		UniqueIdSpecification specification1 = new UniqueIdSpecification("junit5:poops-machine");
 
 		resolver.resolveElement(specification1);
 	}
 
+	@org.junit.Test(expected = IllegalArgumentException.class)
+	public void testUniqueIdOfNotTestMethod() {
+		UniqueIdSpecification specification1 = new UniqueIdSpecification("junit5:org.junit.gen5.engine.junit5.MyTestClass#notATest()");
+		resolver.resolveElement(specification1);
+	}
+
 	@org.junit.Test
 	public void testMethodResolutionByUniqueId() {
 		UniqueIdSpecification specification = new UniqueIdSpecification(
@@ -145,6 +180,9 @@ void test2() {
 
 	}
 
+	void notATest() {
+
+	}
 }
 
 class YourTestClass {
@@ -159,4 +197,21 @@ void test4() {
 
 	}
 
+}
+
+class OtherTestClass {
+
+	static class InnerTestClass {
+
+		@Test
+		void test5() {
+
+		}
+
+		@Test
+		void test6() {
+
+		}
+
+	}
 }