Allow parallel execution of child nodes if only read locks are acquired

fixes junit-team#2423
leonard84 · Jan 4, 2021 · f132c89 · f132c89
1 parent 81e82d9
commit f132c89
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 5 deletions.
diff --git a/...m-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalker.java b/...m-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalker.java
@@ -57,11 +57,16 @@ private void walk(TestDescriptor globalLockDescriptor, TestDescriptor testDescri
 		}
 		else {
 			Set<ExclusiveResource> allResources = new HashSet<>(exclusiveResources);
-			advisor.forceDescendantExecutionMode(testDescriptor, SAME_THREAD);
-			doForChildrenRecursively(testDescriptor, child -> {
-				allResources.addAll(getExclusiveResources(child));
-				advisor.forceDescendantExecutionMode(child, SAME_THREAD);
-			});
+			if (isReadOnly(allResources)) {
+				doForChildrenRecursively(testDescriptor, child -> allResources.addAll(getExclusiveResources(child)));
+			}
+			if (!isReadOnly(allResources)) {
+				advisor.forceDescendantExecutionMode(testDescriptor, SAME_THREAD);
+				doForChildrenRecursively(testDescriptor, child -> {
+					allResources.addAll(getExclusiveResources(child));
+					advisor.forceDescendantExecutionMode(child, SAME_THREAD);
+				});
+			}
 			if (!globalLockDescriptor.equals(testDescriptor) && allResources.contains(GLOBAL_READ_WRITE)) {
 				advisor.forceDescendantExecutionMode(globalLockDescriptor, SAME_THREAD);
 				doForChildrenRecursively(globalLockDescriptor,
@@ -72,6 +77,11 @@ private void walk(TestDescriptor globalLockDescriptor, TestDescriptor testDescri
 		}
 	}
 
+	private boolean isReadOnly(Set<ExclusiveResource> exclusiveResources) {
+		return exclusiveResources.stream().noneMatch(
+			exclusiveResource -> exclusiveResource.getLockMode() == ExclusiveResource.LockMode.READ_WRITE);
+	}
+
 	private Set<ExclusiveResource> getExclusiveResources(TestDescriptor testDescriptor) {
 		return NodeUtils.asNode(testDescriptor).getExclusiveResources();
 	}

diff --git a/...t/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalkerIntegrationTests.java b/...t/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalkerIntegrationTests.java
@@ -15,6 +15,7 @@
 import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
 import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_READ;
 import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_READ_WRITE;
+import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.LockMode.READ;
 import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.LockMode.READ_WRITE;
 import static org.junit.platform.engine.support.hierarchical.Node.ExecutionMode.SAME_THREAD;
 import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.request;
@@ -25,6 +26,7 @@
 
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.parallel.ResourceAccessMode;
 import org.junit.jupiter.api.parallel.ResourceLock;
 import org.junit.jupiter.engine.JupiterTestEngine;
 import org.junit.platform.engine.TestDescriptor;
@@ -54,6 +56,38 @@ void pullUpExclusiveChildResourcesToTestClass() {
 		assertThat(advisor.getForcedExecutionMode(testMethodDescriptor)).contains(SAME_THREAD);
 	}
 
+	@Test
+	void setsForceExecutionModeForChildrenWithWriteLocksOnClass() {
+		var engineDescriptor = discover(TestCaseWithResourceWriteLockOnClass.class);
+
+		var advisor = nodeTreeWalker.walk(engineDescriptor);
+
+		var testClassDescriptor = getOnlyElement(engineDescriptor.getChildren());
+		assertThat(advisor.getResourceLock(testClassDescriptor)).extracting(allLocks()) //
+				.isEqualTo(List.of(getReadWriteLock("a")));
+		assertThat(advisor.getForcedExecutionMode(testClassDescriptor)).isEmpty();
+
+		var testMethodDescriptor = getOnlyElement(testClassDescriptor.getChildren());
+		assertThat(advisor.getResourceLock(testMethodDescriptor)).extracting(allLocks()).isEqualTo(List.of());
+		assertThat(advisor.getForcedExecutionMode(testMethodDescriptor)).contains(SAME_THREAD);
+	}
+
+	@Test
+	void doesntSetForceExecutionModeForChildrenWithReadLocksOnClass() {
+		var engineDescriptor = discover(TestCaseWithResourceReadLockOnClass.class);
+
+		var advisor = nodeTreeWalker.walk(engineDescriptor);
+
+		var testClassDescriptor = getOnlyElement(engineDescriptor.getChildren());
+		assertThat(advisor.getResourceLock(testClassDescriptor)).extracting(allLocks()) //
+				.isEqualTo(List.of(getReadLock("a")));
+		assertThat(advisor.getForcedExecutionMode(testClassDescriptor)).isEmpty();
+
+		var testMethodDescriptor = getOnlyElement(testClassDescriptor.getChildren());
+		assertThat(advisor.getResourceLock(testMethodDescriptor)).extracting(allLocks()).isEqualTo(List.of());
+		assertThat(advisor.getForcedExecutionMode(testMethodDescriptor)).isEmpty();
+	}
+
 	@Test
 	void leavesResourceLockOnTestMethodWhenClassDoesNotUseResource() {
 		var engineDescriptor = discover(TestCaseWithoutResourceLock.class);
@@ -112,6 +146,10 @@ private Lock getReadWriteLock(String key) {
 		return getLock(new ExclusiveResource(key, READ_WRITE));
 	}
 
+	private Lock getReadLock(String key) {
+		return getLock(new ExclusiveResource(key, READ));
+	}
+
 	private Lock getLock(ExclusiveResource exclusiveResource) {
 		return getOnlyElement(ResourceLockSupport.getLocks(lockManager.getLockForResource(exclusiveResource)));
 	}
@@ -154,4 +192,18 @@ void test() {
 			}
 		}
 	}
+
+	@ResourceLock("a")
+	static class TestCaseWithResourceWriteLockOnClass {
+		@Test
+		void test() {
+		}
+	}
+
+	@ResourceLock(value = "a", mode = ResourceAccessMode.READ)
+	static class TestCaseWithResourceReadLockOnClass {
+		@Test
+		void test() {
+		}
+	}
 }