diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-6.1.0-M1.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-6.1.0-M1.adoc index a42b54393899..bd1bce04e35d 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-6.1.0-M1.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-6.1.0-M1.adoc @@ -31,6 +31,9 @@ repository on GitHub. * `OpenTestReportGeneratingListener` now supports redirecting XML events to a socket via the new `junit.platform.reporting.open.xml.socket` configuration parameter. When set to a port number, events are sent to `127.0.0.1:` instead of being written to a file. +* Allow implementations of `HierarchicalTestEngine` to specify which nodes require the + global read lock by overriding the `Node.isGlobalReadLockRequired()` method to return + `false`. [[release-notes-6.1.0-M1-junit-jupiter]] diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/Node.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/Node.java index 1fa014c92d14..fa7405ab591b 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/Node.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/Node.java @@ -11,6 +11,7 @@ package org.junit.platform.engine.support.hierarchical; import static java.util.Collections.emptySet; +import static org.apiguardian.api.API.Status.EXPERIMENTAL; import static org.apiguardian.api.API.Status.MAINTAINED; import static org.apiguardian.api.API.Status.STABLE; @@ -182,6 +183,23 @@ default Set getExclusiveResources() { return emptySet(); } + /** + * {@return whether this node requires the global read-write lock} + * + *

Engines should return {@code true} for all nodes that have behavior, + * including tests but also containers with before/after lifecycle hooks. + * + *

The default implementation returns {@code true}. The value returned by + * engine-level nodes is ignored. Otherwise, if a container node returns + * {@code true}, the value returned by its descendants is ignored. + * + * @since 6.1 + */ + @API(status = EXPERIMENTAL, since = "6.1") + default boolean isGlobalReadLockRequired() { + return true; + } + /** * Get the preferred of {@linkplain ExecutionMode execution mode} for * parallel execution of this node. diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalker.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalker.java index 1f95c6233b5b..38dd1e0ea42b 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalker.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalker.java @@ -13,11 +13,13 @@ 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.Node.ExecutionMode.SAME_THREAD; +import static org.junit.platform.engine.support.hierarchical.NodeUtils.asNode; import java.util.HashSet; import java.util.Set; import java.util.function.Consumer; +import org.jspecify.annotations.Nullable; import org.junit.platform.commons.util.Preconditions; import org.junit.platform.engine.TestDescriptor; @@ -44,26 +46,34 @@ NodeExecutionAdvisor walk(TestDescriptor rootDescriptor) { Preconditions.condition(getExclusiveResources(rootDescriptor).isEmpty(), "Engine descriptor must not declare exclusive resources"); NodeExecutionAdvisor advisor = new NodeExecutionAdvisor(); - rootDescriptor.getChildren().forEach(child -> walk(child, child, advisor)); + rootDescriptor.getChildren().forEach(child -> walk(nullUnlessRequiresGlobalReadLock(child), child, advisor)); return advisor; } - private void walk(TestDescriptor globalLockDescriptor, TestDescriptor testDescriptor, + private void walk(@Nullable TestDescriptor globalLockDescriptor, TestDescriptor testDescriptor, NodeExecutionAdvisor advisor) { - if (advisor.getResourceLock(globalLockDescriptor) == globalReadWriteLock) { + if (globalLockDescriptor != null && advisor.getResourceLock(globalLockDescriptor) == globalReadWriteLock) { // Global read-write lock is already being enforced, so no additional locks are needed return; } Set exclusiveResources = getExclusiveResources(testDescriptor); if (exclusiveResources.isEmpty()) { - if (globalLockDescriptor.equals(testDescriptor)) { + if (globalLockDescriptor != null && globalLockDescriptor.equals(testDescriptor)) { advisor.useResourceLock(globalLockDescriptor, globalReadLock); } - testDescriptor.getChildren().forEach(child -> walk(globalLockDescriptor, child, advisor)); + testDescriptor.getChildren().forEach(child -> { + var newGlobalLockDescriptor = globalLockDescriptor == null // + ? nullUnlessRequiresGlobalReadLock(child) // + : globalLockDescriptor; + walk(newGlobalLockDescriptor, child, advisor); + }); } else { + Preconditions.notNull(globalLockDescriptor, + () -> "Node requiring exclusive resources must also require global read lock: " + testDescriptor); + Set allResources = new HashSet<>(exclusiveResources); if (isReadOnly(allResources)) { doForChildrenRecursively(testDescriptor, child -> allResources.addAll(getExclusiveResources(child))); @@ -109,7 +119,11 @@ private boolean isReadOnly(Set exclusiveResources) { } private Set getExclusiveResources(TestDescriptor testDescriptor) { - return NodeUtils.asNode(testDescriptor).getExclusiveResources(); + return asNode(testDescriptor).getExclusiveResources(); + } + + private static @Nullable TestDescriptor nullUnlessRequiresGlobalReadLock(TestDescriptor testDescriptor) { + return asNode(testDescriptor).isGlobalReadLockRequired() ? testDescriptor : null; } private void doForChildrenRecursively(TestDescriptor parent, Consumer consumer) { diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalkerIntegrationTests.java b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalkerIntegrationTests.java index 4857212814cb..a4418ecf95be 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalkerIntegrationTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalkerIntegrationTests.java @@ -11,6 +11,8 @@ package org.junit.platform.engine.support.hierarchical; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.InstanceOfAssertFactories.LIST; +import static org.junit.platform.commons.test.PreconditionAssertions.assertPreconditionViolationFor; import static org.junit.platform.commons.util.CollectionUtils.getOnlyElement; import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass; import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_READ; @@ -20,10 +22,13 @@ import static org.junit.platform.engine.support.hierarchical.Node.ExecutionMode.SAME_THREAD; import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.request; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.locks.Lock; import java.util.function.Function; +import org.jspecify.annotations.NullMarked; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.parallel.ResourceAccessMode; @@ -32,6 +37,8 @@ import org.junit.jupiter.engine.JupiterTestEngine; import org.junit.platform.engine.TestDescriptor; import org.junit.platform.engine.UniqueId; +import org.junit.platform.engine.support.descriptor.AbstractTestDescriptor; +import org.junit.platform.engine.support.descriptor.EngineDescriptor; /** * @since 1.3 @@ -171,6 +178,55 @@ void coarsensGlobalLockToEngineDescriptorChild() { assertThat(advisor.getForcedExecutionMode(testMethodDescriptor)).contains(SAME_THREAD); } + @Test + void putsGlobalReadLockOnFirstNodeThatRequiresIt() { + var engineDescriptor = new EngineDescriptor(UniqueId.forEngine("dummy"), "Dummy"); + + var containerWithoutBehavior = new NodeStub(engineDescriptor.getUniqueId().append("container", "1"), + "Container 1") // + .withGlobalReadLockRequired(false); + var test1 = new NodeStub(containerWithoutBehavior.getUniqueId().append("test", "1"), "Test 1") // + .withExclusiveResource(new ExclusiveResource("key1", READ_WRITE)); + containerWithoutBehavior.addChild(test1); + + var containerWithBehavior = new NodeStub(engineDescriptor.getUniqueId().append("container", "2"), "Container 2") // + .withGlobalReadLockRequired(true); + var test2 = new NodeStub(containerWithBehavior.getUniqueId().append("test", "2"), "Test 2") // + .withExclusiveResource(new ExclusiveResource("key2", READ_WRITE)); + containerWithBehavior.addChild(test2); + + engineDescriptor.addChild(containerWithoutBehavior); + engineDescriptor.addChild(containerWithBehavior); + + var advisor = nodeTreeWalker.walk(engineDescriptor); + + assertThat(advisor.getResourceLock(containerWithoutBehavior)) // + .extracting(allLocks(), LIST) // + .isEmpty(); + assertThat(advisor.getResourceLock(test1)) // + .extracting(allLocks(), LIST) // + .containsExactly(getLock(GLOBAL_READ), getReadWriteLock("key1")); + + assertThat(advisor.getResourceLock(containerWithBehavior)) // + .extracting(allLocks(), LIST) // + .containsExactly(getLock(GLOBAL_READ)); + assertThat(advisor.getResourceLock(test2)) // + .extracting(allLocks(), LIST) // + .containsExactly(getReadWriteLock("key2")); + } + + @Test + void doesNotAllowExclusiveResourcesWithoutRequiringGlobalReadLock() { + var engineDescriptor = new EngineDescriptor(UniqueId.forEngine("dummy"), "Dummy"); + var invalidNode = new NodeStub(engineDescriptor.getUniqueId().append("container", "1"), "Container") // + .withGlobalReadLockRequired(false) // + .withExclusiveResource(new ExclusiveResource("key", READ_WRITE)); + engineDescriptor.addChild(invalidNode); + + assertPreconditionViolationFor(() -> nodeTreeWalker.walk(engineDescriptor)) // + .withMessage("Node requiring exclusive resources must also require global read lock: " + invalidNode); + } + private static Function> allLocks() { return ResourceLockSupport::getLocks; } @@ -262,4 +318,41 @@ static class TestCaseWithResourceReadLockOnClassAndReadClockOnTestCase { void test() { } } + + @NullMarked + static class NodeStub extends AbstractTestDescriptor implements Node { + + private final Set exclusiveResources = new LinkedHashSet<>(); + private boolean globalReadLockRequired = true; + + NodeStub(UniqueId uniqueId, String displayName) { + super(uniqueId, displayName); + } + + NodeStub withExclusiveResource(ExclusiveResource exclusiveResource) { + exclusiveResources.add(exclusiveResource); + return this; + } + + NodeStub withGlobalReadLockRequired(boolean globalReadLockRequired) { + this.globalReadLockRequired = globalReadLockRequired; + return this; + } + + @Override + public boolean isGlobalReadLockRequired() { + return globalReadLockRequired; + } + + @Override + public Type getType() { + throw new UnsupportedOperationException("should not be called"); + } + + @Override + public Set getExclusiveResources() { + return exclusiveResources; + } + } + }