Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Acquire global read lock in presence of other exclusive resources #2614

Merged
merged 1 commit into from
May 15, 2021
Merged

Acquire global read lock in presence of other exclusive resources #2614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion documentation/src/docs/asciidoc/release-notes/release-notes-5.7.2.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
Expand Down
11 changes: 6 additions & 5 deletions ...m-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
10 changes: 5 additions & 5 deletions ...t/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalkerIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand Down
20 changes: 20 additions & 0 deletions ...ava/org/junit/platform/engine/support/hierarchical/ParallelExecutionIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down