Collect locks from the providers of the enclosing classes.

Issue: #2677
junit-team · Oct 4, 2024 · ef2f38f · ef2f38f
1 parent 792b43a
commit ef2f38f
Show file tree

Hide file tree

Showing 8 changed files with 167 additions and 97 deletions.
diff --git a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/ResourceLocksProvider.java b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/ResourceLocksProvider.java
@@ -49,8 +49,8 @@ public interface ResourceLocksProvider {
 	/**
 	 * Add shared resources to a test class.
 	 *
-	 * <p>Invoked for a test class annotated with
-	 * {@code @ResourceLock(providers)} and for its child classes.
+	 * <p>Invoked in case a test class or its parent class
+	 * is annotated with {@code @ResourceLock(providers)}.
 	 *
 	 * @apiNote Adding {@linkplain Lock a shared resource} with this method
 	 * has the same semantics as annotating a test class
@@ -66,6 +66,14 @@ default Set<Lock> provideForClass(Class<?> testClass) {
 	/**
 	 * Add shared resources to a {@linkplain Nested nested} test class.
 	 *
+	 * <p>Invoked in case:
+	 * <ul>
+	 *     <li>an enclosing test class of any level or its parent class
+	 *     is annotated with {@code @ResourceLock(providers)}.</li>
+	 *     <li>a nested test class or its parent class
+	 * 	   is annotated with {@code @ResourceLock(providers)}.</li>
+	 * </ul>
+	 *
 	 * <p>Invoked for a nested test class
 	 * annotated with {@code @ResourceLock(providers)}
 	 * and for its child classes.
@@ -76,7 +84,7 @@ default Set<Lock> provideForClass(Class<?> testClass) {
 	 *
 	 * @param testClass a nested test class to add shared resources
 	 * @return a set of {@link Lock}; may be empty
-	 * @see    Nested
+	 * @see Nested
 	 */
 	default Set<Lock> provideForNestedClass(Class<?> testClass) {
 		return emptySet();
@@ -87,7 +95,7 @@ default Set<Lock> provideForNestedClass(Class<?> testClass) {
 	 *
 	 * <p>Invoked in case:
 	 * <ul>
-	 *     <li>an enclosing test class or its parent class
+	 *     <li>an enclosing test class of any level or its parent class
 	 *     is annotated with {@code @ResourceLock(providers)}.</li>
 	 *     <li>a test method
 	 *     is annotated with {@code @ResourceLock(providers)}.</li>

diff --git a/...er-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java b/...er-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java
@@ -112,8 +112,6 @@ public final Class<?> getTestClass() {
 		return this.testClass;
 	}
 
-	public abstract List<Class<?>> getEnclosingTestClasses();
-
 	@Override
 	public Type getType() {
 		return Type.CONTAINER;
@@ -143,7 +141,7 @@ public void setDefaultChildExecutionMode(ExecutionMode defaultChildExecutionMode
 	@Override
 	public Set<ExclusiveResource> getExclusiveResources() {
 		// @formatter:off
-		return getExclusiveResourcesFromAnnotation(
+		return getExclusiveResourcesFromAnnotations(
 				getTestClass(),
 				provider -> provider.provideForClass(getTestClass())
 		);

diff --git a/...jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassTestDescriptor.java b/...jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassTestDescriptor.java
@@ -10,12 +10,10 @@
 
 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;
 
@@ -59,11 +57,6 @@ public Set<TestTag> getTags() {
 		return new LinkedHashSet<>(this.tags);
 	}
 
-	@Override
-	public List<Class<?>> getEnclosingTestClasses() {
-		return emptyList();
-	}
-
 	// --- Node ----------------------------------------------------------------
 
 	@Override

diff --git a/...piter-engine/src/main/java/org/junit/jupiter/engine/descriptor/JupiterTestDescriptor.java b/...piter-engine/src/main/java/org/junit/jupiter/engine/descriptor/JupiterTestDescriptor.java
@@ -10,15 +10,18 @@
 
 package org.junit.jupiter.engine.descriptor;
 
+import static java.util.Collections.emptyList;
 import static java.util.stream.Collectors.collectingAndThen;
 import static java.util.stream.Collectors.toCollection;
+import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
 import static org.apiguardian.api.API.Status.INTERNAL;
 import static org.junit.jupiter.engine.descriptor.DisplayNameUtils.determineDisplayName;
 import static org.junit.platform.commons.support.AnnotationSupport.findAnnotation;
 import static org.junit.platform.commons.support.AnnotationSupport.findRepeatableAnnotations;
 
 import java.lang.reflect.AnnotatedElement;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.LinkedHashSet;
@@ -106,6 +109,17 @@ static Set<TestTag> getTags(AnnotatedElement element) {
 		// @formatter:on
 	}
 
+	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();
+	}
+
 	/**
 	 * Invoke exception handlers for the supplied {@code Throwable} one-by-one
 	 * until none are left or the throwable to handle has been swallowed.
@@ -186,34 +200,41 @@ public static ExecutionMode toExecutionMode(org.junit.jupiter.api.parallel.Execu
 		throw new JUnitException("Unknown ExecutionMode: " + mode);
 	}
 
-	Set<ExclusiveResource> getExclusiveResourcesFromAnnotation(AnnotatedElement element,
+	Set<ExclusiveResource> getExclusiveResourcesFromAnnotations(AnnotatedElement element,
 			Function<ResourceLocksProvider, Set<ResourceLocksProvider.Lock>> providerToLocks) {
-		List<ResourceLock> resourceLocks = findRepeatableAnnotations(element, ResourceLock.class);
 		// @formatter:off
-		return Stream.concat(
-				getExclusiveResourcesFromValues(resourceLocks),
-				getExclusiveResourcesFromProviders(resourceLocks, providerToLocks)
-		).collect(collectingAndThen(toSet(), Collections::unmodifiableSet));
+		List<ResourceLock> ownAnnotations = findRepeatableAnnotations(element, ResourceLock.class);
+		List<ResourceLock> enclosingClassesAnnotations = getEnclosingTestClasses().stream()
+				.map(clazz -> findRepeatableAnnotations(clazz, ResourceLock.class))
+				.flatMap(Collection::stream)
+				.collect(toList());
+
+		return Stream.of(
+				getExclusiveResourcesFromValues(ownAnnotations),
+				getExclusiveResourcesFromProviders(ownAnnotations, providerToLocks),
+				getExclusiveResourcesFromProviders(enclosingClassesAnnotations, providerToLocks)
+		).flatMap(s -> s)
+		.collect(toSet());
 		// @formatter:on
 	}
 
-	Stream<ExclusiveResource> getExclusiveResourcesFromValues(List<ResourceLock> resourceLocks) {
+	private Stream<ExclusiveResource> getExclusiveResourcesFromValues(List<ResourceLock> annotations) {
 		// @formatter:off
-		return resourceLocks.stream()
-				.flatMap(resourceLock -> {
-					if (StringUtils.isBlank(resourceLock.value())) {
+		return annotations.stream()
+				.flatMap(annotation -> {
+					if (StringUtils.isBlank(annotation.value())) {
 						return Stream.empty();
 					}
-					return Stream.of(new ExclusiveResource(resourceLock.value(), toLockMode(resourceLock.mode())));
+					return Stream.of(new ExclusiveResource(annotation.value(), toLockMode(annotation.mode())));
 				});
 		// @formatter:on
 	}
 
-	Stream<ExclusiveResource> getExclusiveResourcesFromProviders(List<ResourceLock> resourceLocks,
+	private Stream<ExclusiveResource> getExclusiveResourcesFromProviders(List<ResourceLock> annotations,
 			Function<ResourceLocksProvider, Set<ResourceLocksProvider.Lock>> providerToLocks) {
 		// @formatter:off
-		return resourceLocks.stream()
-				.flatMap(resourceLock -> Stream.of(resourceLock.providers())
+		return annotations.stream()
+				.flatMap(annotation -> Stream.of(annotation.providers())
 						.map(ReflectionUtils::newInstance)
 						.map(providerToLocks)
 						.flatMap(Collection::stream)

diff --git a/...r-engine/src/main/java/org/junit/jupiter/engine/descriptor/MethodBasedTestDescriptor.java b/...r-engine/src/main/java/org/junit/jupiter/engine/descriptor/MethodBasedTestDescriptor.java
@@ -10,28 +10,20 @@
 
 package org.junit.jupiter.engine.descriptor;
 
-import static java.util.stream.Collectors.collectingAndThen;
-import static java.util.stream.Collectors.toSet;
 import static org.apiguardian.api.API.Status.INTERNAL;
 import static org.junit.jupiter.engine.descriptor.DisplayNameUtils.determineDisplayNameForMethod;
-import static org.junit.platform.commons.support.AnnotationSupport.findRepeatableAnnotations;
 import static org.junit.platform.commons.util.CollectionUtils.forEachInReverseOrder;
 
 import java.lang.reflect.Method;
-import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Consumer;
-import java.util.function.Function;
-import java.util.stream.Stream;
 
 import org.apiguardian.api.API;
 import org.junit.jupiter.api.extension.ExtensionContext;
 import org.junit.jupiter.api.extension.TestWatcher;
-import org.junit.jupiter.api.parallel.ResourceLock;
-import org.junit.jupiter.api.parallel.ResourceLocksProvider;
 import org.junit.jupiter.engine.config.JupiterConfiguration;
 import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext;
 import org.junit.platform.commons.logging.Logger;
@@ -90,22 +82,10 @@ public final Set<TestTag> getTags() {
 	@Override
 	public Set<ExclusiveResource> getExclusiveResources() {
 		// @formatter:off
-		Function<ResourceLocksProvider, Set<ResourceLocksProvider.Lock>> providerToLocks =
-				p -> p.provideForMethod(getTestClass(), getTestMethod());
-
-		Set<ExclusiveResource> fromMethodAnnotation = getExclusiveResourcesFromAnnotation(
+		return getExclusiveResourcesFromAnnotations(
 				getTestMethod(),
-				providerToLocks
-		);
-		Stream<ExclusiveResource> fromProvidersInClassAnnotation = getExclusiveResourcesFromProviders(
-				findRepeatableAnnotations(getTestClass(), ResourceLock.class),
-				providerToLocks
+				provider -> provider.provideForMethod(getTestClass(), getTestMethod())
 		);
-
-		return Stream.concat(
-				fromMethodAnnotation.stream(),
-				fromProvidersInClassAnnotation
-		).collect(collectingAndThen(toSet(), Collections::unmodifiableSet));
 		// @formatter:on
 	}
 

diff --git a/...r-engine/src/main/java/org/junit/jupiter/engine/descriptor/NestedClassTestDescriptor.java b/...r-engine/src/main/java/org/junit/jupiter/engine/descriptor/NestedClassTestDescriptor.java
@@ -10,13 +10,10 @@
 
 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;
 
@@ -62,18 +59,6 @@ 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
@@ -91,7 +76,7 @@ protected TestInstances instantiateTestClass(JupiterEngineExecutionContext paren
 	@Override
 	public Set<ExclusiveResource> getExclusiveResources() {
 		// @formatter:off
-		return getExclusiveResourcesFromAnnotation(
+		return getExclusiveResourcesFromAnnotations(
 				getTestClass(),
 				provider -> provider.provideForNestedClass(getTestClass())
 		);

diff --git a/...c/test/java/org/junit/platform/engine/support/descriptor/ResourceLockAnnotationTests.java b/...c/test/java/org/junit/platform/engine/support/descriptor/ResourceLockAnnotationTests.java
@@ -183,9 +183,11 @@ private Set<ExclusiveResource> getClassResources(Class<?> testClass) {
 
 	private Set<ExclusiveResource> getMethodResources(Class<?> testClass) {
 		try {
-			return new TestMethodTestDescriptor( //
-				uniqueId, testClass, testClass.getDeclaredMethod("test"), configuration //
-			).getExclusiveResources();
+			var descriptor = new TestMethodTestDescriptor( //
+					uniqueId, testClass, testClass.getDeclaredMethod("test"), configuration //
+			);
+			descriptor.setParent(new ClassTestDescriptor(uniqueId, testClass, configuration));
+			return descriptor.getExclusiveResources();
 		}
 		catch (NoSuchMethodException e) {
 			throw new RuntimeException(e);