Acquire global read lock in presence of other exclusive resources

Prior to this commit, the global read lock was not acquired for test
classes with `@ResourceLock` annotations causing `@Isolated` tests to
run in parallel.

Fixes #2605.

Author: marcphilipp

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

@@ -18,14 +18,17 @@ GitHub.
 * Method `getRootUrisForPackage()` in class `ClasspathScanner` now returns a valid list of
   class names when the package name is equal to the name of a module on the module path
   and when running on Java 8.
+* Direct child descriptors of the engine descriptor now also acquire the global read lock
+  when they require other exclusive resources.
 
 
 [[release-notes-5.7.2-junit-jupiter]]
 === JUnit Jupiter
 
 ==== Bug Fixes
 
-* ❓
+* Test classes annotated with `@ResourceLock` no longer run in parallel with `@Isolated`
+  ones.
 
 
 [[release-notes-5.7.2-junit-vintage]]

junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalker.java

@@ -74,15 +74,16 @@ private void walk(TestDescriptor globalLockDescriptor, TestDescriptor testDescri
 				forceDescendantExecutionModeRecursively(advisor, globalLockDescriptor);
 				advisor.useResourceLock(globalLockDescriptor, globalReadWriteLock);
 			}
+			if (globalLockDescriptor.equals(testDescriptor) && !allResources.contains(GLOBAL_READ_WRITE)) {
+				allResources.add(GLOBAL_READ);
+			}
 			advisor.useResourceLock(testDescriptor, lockManager.getLockForResources(allResources));
 		}
 	}
 
-	private void forceDescendantExecutionModeRecursively(NodeExecutionAdvisor advisor,
-			TestDescriptor globalLockDescriptor) {
-		advisor.forceDescendantExecutionMode(globalLockDescriptor, SAME_THREAD);
-		doForChildrenRecursively(globalLockDescriptor,
-			child -> advisor.forceDescendantExecutionMode(child, SAME_THREAD));
+	private void forceDescendantExecutionModeRecursively(NodeExecutionAdvisor advisor, TestDescriptor testDescriptor) {
+		advisor.forceDescendantExecutionMode(testDescriptor, SAME_THREAD);
+		doForChildrenRecursively(testDescriptor, child -> advisor.forceDescendantExecutionMode(child, SAME_THREAD));
 	}
 
 	private boolean isReadOnly(Set<ExclusiveResource> exclusiveResources) {

platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalkerIntegrationTests.java

@@ -48,7 +48,7 @@ void pullUpExclusiveChildResourcesToTestClass() {
 
 		var testClassDescriptor = getOnlyElement(engineDescriptor.getChildren());
 		assertThat(advisor.getResourceLock(testClassDescriptor)).extracting(allLocks()) //
-				.isEqualTo(List.of(getReadWriteLock("a"), getReadWriteLock("b")));
+				.isEqualTo(List.of(getLock(GLOBAL_READ), getReadWriteLock("a"), getReadWriteLock("b")));
 		assertThat(advisor.getForcedExecutionMode(testClassDescriptor)).isEmpty();
 
 		var testMethodDescriptor = getOnlyElement(testClassDescriptor.getChildren());
@@ -64,7 +64,7 @@ void setsForceExecutionModeForChildrenWithWriteLocksOnClass() {
 
 		var testClassDescriptor = getOnlyElement(engineDescriptor.getChildren());
 		assertThat(advisor.getResourceLock(testClassDescriptor)).extracting(allLocks()) //
-				.isEqualTo(List.of(getReadWriteLock("a")));
+				.isEqualTo(List.of(getLock(GLOBAL_READ), getReadWriteLock("a")));
 		assertThat(advisor.getForcedExecutionMode(testClassDescriptor)).isEmpty();
 
 		var testMethodDescriptor = getOnlyElement(testClassDescriptor.getChildren());
@@ -80,7 +80,7 @@ void doesntSetForceExecutionModeForChildrenWithReadLocksOnClass() {
 
 		var testClassDescriptor = getOnlyElement(engineDescriptor.getChildren());
 		assertThat(advisor.getResourceLock(testClassDescriptor)).extracting(allLocks()) //
-				.isEqualTo(List.of(getReadLock("a")));
+				.isEqualTo(List.of(getLock(GLOBAL_READ), getReadLock("a")));
 		assertThat(advisor.getForcedExecutionMode(testClassDescriptor)).isEmpty();
 
 		var testMethodDescriptor = getOnlyElement(testClassDescriptor.getChildren());
@@ -96,7 +96,7 @@ void setsForceExecutionModeForChildrenWithReadLocksOnClassAndWriteLockOnTest() {
 
 		var testClassDescriptor = getOnlyElement(engineDescriptor.getChildren());
 		assertThat(advisor.getResourceLock(testClassDescriptor)).extracting(allLocks()) //
-				.isEqualTo(List.of(getReadWriteLock("a")));
+				.isEqualTo(List.of(getLock(GLOBAL_READ), getReadWriteLock("a")));
 		assertThat(advisor.getForcedExecutionMode(testClassDescriptor)).isEmpty();
 
 		var testMethodDescriptor = getOnlyElement(testClassDescriptor.getChildren());
@@ -112,7 +112,7 @@ void doesntSetForceExecutionModeForChildrenWithReadLocksOnClassAndReadLockOnTest
 
 		var testClassDescriptor = getOnlyElement(engineDescriptor.getChildren());
 		assertThat(advisor.getResourceLock(testClassDescriptor)).extracting(allLocks()) //
-				.isEqualTo(List.of(getReadLock("a"), getReadLock("b")));
+				.isEqualTo(List.of(getLock(GLOBAL_READ), getReadLock("a"), getReadLock("b")));
 		assertThat(advisor.getForcedExecutionMode(testClassDescriptor)).isEmpty();
 
 		var testMethodDescriptor = getOnlyElement(testClassDescriptor.getChildren());

platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/ParallelExecutionIntegrationTests.java

@@ -243,6 +243,13 @@ void canRunTestsIsolatedFromEachOtherAcrossClasses() {
 		assertThat(events.stream().filter(event(test(), finishedWithFailure())::matches)).isEmpty();
 	}
 
+	@RepeatedTest(10)
+	void canRunTestsIsolatedFromEachOtherAcrossClassesWithOtherResourceLocks() {
+		var events = executeConcurrently(4, IndependentClasses.B.class, IndependentClasses.C.class);
+
+		assertThat(events.stream().filter(event(test(), finishedWithFailure())::matches)).isEmpty();
+	}
+
 	@Isolated("testing")
 	static class IsolatedTestCase {
 		static AtomicInteger sharedResource;
@@ -343,6 +350,19 @@ void b() throws Exception {
 				storeAndBlockAndCheck(sharedResource, countDownLatch);
 			}
 		}
+
+		@ResourceLock("other")
+		static class C {
+			@Test
+			void a() throws Exception {
+				storeAndBlockAndCheck(sharedResource, countDownLatch);
+			}
+
+			@Test
+			void b() throws Exception {
+				storeAndBlockAndCheck(sharedResource, countDownLatch);
+			}
+		}
 	}
 
 	private List<Event> getEventsOfChildren(EngineExecutionResults results, TestDescriptor container) {