Skip to content

Commit

Permalink
Allow declaring exclusive resources for child nodes (#4151)
Browse files Browse the repository at this point in the history
Allow declaring "shared resources" for direct child nodes via the new 
`@ResourceLock(target = CHILDREN)` attribute.

Using the `@ResourceLock(target = CHILDREN)` in a class-level annotation
has the same semantics as adding an annotation with the same value and 
mode to each test method and nested test class declared in this class.

This may improve parallelization when a test class declares a `READ` 
lock, but only a few methods hold a `READ_WRITE` lock.

Resolves #3102.

---------

Co-authored-by: Marc Philipp <[email protected]>
  • Loading branch information
vdmitrienko and marcphilipp authored Nov 26, 2024
1 parent b8b5dc4 commit af6b502
Show file tree
Hide file tree
Showing 10 changed files with 385 additions and 61 deletions.
1 change: 1 addition & 0 deletions documentation/src/docs/asciidoc/link-attributes.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ endif::[]
:Execution: {javadoc-root}/org.junit.jupiter.api/org/junit/jupiter/api/parallel/Execution.html[@Execution]
:Isolated: {javadoc-root}/org.junit.jupiter.api/org/junit/jupiter/api/parallel/Isolated.html[@Isolated]
:ResourceLock: {javadoc-root}/org.junit.jupiter.api/org/junit/jupiter/api/parallel/ResourceLock.html[@ResourceLock]
:ResourceLockTarget: {javadoc-root}/org.junit.jupiter.api/org/junit/jupiter/api/parallel/ResourceLockTarget.html[ResourceLockTarget]
:ResourceLocksProvider: {javadoc-root}/org.junit.jupiter.api/org/junit/jupiter/api/parallel/ResourceLocksProvider.html[ResourceLocksProvider]
:Resources: {javadoc-root}/org.junit.jupiter.api/org/junit/jupiter/api/parallel/Resources.html[Resources]
// Jupiter Extension APIs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ JUnit repository on GitHub.
the absence of invocations is expected in some cases and should not cause a test failure.
* Allow determining "shared resources" at runtime via the new `@ResourceLock#providers`
attribute that accepts implementations of `ResourceLocksProvider`.
* Allow declaring "shared resources" for _direct_ child nodes via the new
`@ResourceLock(target = CHILDREN)` attribute. This may improve parallelization when
a test class declares a `READ` lock, but only a few methods hold a `READ_WRITE` lock.
* Extensions that implement `TestInstancePreConstructCallback`, `TestInstanceFactory`,
`TestInstancePostProcessor`, `ParameterResolver`, or `InvocationInterceptor` may
override the `getTestInstantiationExtensionContextScope()` method to enable receiving
Expand Down
24 changes: 23 additions & 1 deletion documentation/src/docs/asciidoc/user-guide/writing-tests.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -2992,7 +2992,7 @@ Note that resources declared statically with `{ResourceLock}` annotation are com
resources added dynamically by `{ResourceLocksProvider}` implementations.

If the tests in the following example were run in parallel _without_ the use of
{ResourceLock}, they would be _flaky_. Sometimes they would pass, and at other times they
`{ResourceLock}`, they would be _flaky_. Sometimes they would pass, and at other times they
would fail due to the inherent race condition of writing and then reading the same JVM
System Property.

Expand Down Expand Up @@ -3029,6 +3029,28 @@ include::{testDir}/example/sharedresources/StaticSharedResourcesDemo.java[tags=u
include::{testDir}/example/sharedresources/DynamicSharedResourcesDemo.java[tags=user_guide]
----

Also, "static" shared resources can be declared for _direct_ child nodes via the `target`
attribute in the `{ResourceLock}` annotation, the attribute accepts a value from
the `{ResourceLockTarget}` enum.

Specifying `target = CHILDREN` in a class-level `{ResourceLock}` annotation
has the same semantics as adding an annotation with the same `value` and `mode`
to each test method and nested test class declared in this class.

This may improve parallelization when a test class declares a `READ` lock,
but only a few methods hold a `READ_WRITE` lock.

Tests in the following example would run in the `SAME_THREAD` if the `{ResourceLock}`
didn't have `target = CHILDREN`. This is because the test class declares a `READ`
shared resource, but one test method holds a `READ_WRITE` lock,
which would force the `SAME_THREAD` execution mode for all the test methods.

[source,java]
.Declaring shared resources for child nodes with `target` attribute
----
include::{testDir}/example/sharedresources/ChildrenSharedResourcesDemo.java[tags=user_guide]
----


[[writing-tests-built-in-extensions]]
=== Built-in Extensions
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* Copyright 2015-2024 the original author or authors.
*
* All rights reserved. This program and the accompanying materials are
* made available under the terms of the Eclipse Public License v2.0 which
* accompanies this distribution and is available at
*
* https://www.eclipse.org/legal/epl-v20.html
*/

package example.sharedresources;

import static org.junit.jupiter.api.parallel.ExecutionMode.CONCURRENT;
import static org.junit.jupiter.api.parallel.ResourceAccessMode.READ;
import static org.junit.jupiter.api.parallel.ResourceAccessMode.READ_WRITE;
import static org.junit.jupiter.api.parallel.ResourceLockTarget.CHILDREN;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.parallel.Execution;
import org.junit.jupiter.api.parallel.ResourceLock;

// tag::user_guide[]
@Execution(CONCURRENT)
@ResourceLock(value = "a", mode = READ, target = CHILDREN)
public class ChildrenSharedResourcesDemo {

@ResourceLock(value = "a", mode = READ_WRITE)
@Test
void test1() throws InterruptedException {
Thread.sleep(2000L);
}

@Test
void test2() throws InterruptedException {
Thread.sleep(2000L);
}

@Test
void test3() throws InterruptedException {
Thread.sleep(2000L);
}

@Test
void test4() throws InterruptedException {
Thread.sleep(2000L);
}

@Test
void test5() throws InterruptedException {
Thread.sleep(2000L);
}

}
// end::user_guide[]
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,44 @@
*
* <p>This annotation can be repeated to declare the use of multiple shared resources.
*
* <p>Uniqueness of a shared resource is identified by both {@link #value()} and
* {@link #mode()}. Duplicated shared resources do not cause errors.
*
* <p>Since JUnit Jupiter 5.4, this annotation is {@linkplain Inherited inherited}
* within class hierarchies.
*
* <p>Since JUnit Jupiter 5.12, this annotation supports adding shared resources
* dynamically at runtime via {@link ResourceLock#providers}.
* dynamically at runtime via {@link #providers}.
*
* <p>Resources declared "statically" using {@link #value()} and {@link #mode()}
* are combined with "dynamic" resources added via {@link #providers()}.
* For example, declaring resource "A" via {@code @ResourceLock("A")}
* and resource "B" via a provider returning {@code new Lock("B")} will result
* in two shared resources "A" and "B".
*
* <p>Since JUnit Jupiter 5.12, this annotation supports declaring "static"
* shared resources for <em>direct</em> child nodes via the {@link #target()}
* attribute.
*
* <p>Using the {@link ResourceLockTarget#CHILDREN} in a class-level
* annotation has the same semantics as adding an annotation with the same
* {@link #value()} and {@link #mode()} to each test method and nested test
* class declared in this class.
*
* <p>This may improve parallelization when a test class declares a
* {@link ResourceAccessMode#READ READ} lock, but only a few methods hold
* {@link ResourceAccessMode#READ_WRITE READ_WRITE} lock.
*
* <p>Note that the {@code target = CHILDREN} means that
* {@link #value()} and {@link #mode()} no longer apply to a node
* declaring the annotation. However, the {@link #providers()} attribute
* remains applicable, and the target of "dynamic" shared resources
* added via implementations of {@link ResourceLocksProvider} is not changed.
*
* @see Isolated
* @see Resources
* @see ResourceAccessMode
* @see ResourceLockTarget
* @see ResourceLocks
* @see ResourceLocksProvider
* @since 5.3
Expand Down Expand Up @@ -92,6 +115,20 @@
*/
ResourceAccessMode mode() default ResourceAccessMode.READ_WRITE;

/**
* The target of a resource created from {@link #value()} and {@link #mode()}.
*
* <p>Defaults to {@link ResourceLockTarget#SELF SELF}.
*
* <p>Note that using {@link ResourceLockTarget#CHILDREN} in
* a method-level annotation results in an exception.
*
* @see ResourceLockTarget
* @since 5.12
*/
@API(status = EXPERIMENTAL, since = "5.12")
ResourceLockTarget target() default ResourceLockTarget.SELF;

/**
* An array of one or more classes implementing {@link ResourceLocksProvider}.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright 2015-2024 the original author or authors.
*
* All rights reserved. This program and the accompanying materials are
* made available under the terms of the Eclipse Public License v2.0 which
* accompanies this distribution and is available at
*
* https://www.eclipse.org/legal/epl-v20.html
*/

package org.junit.jupiter.api.parallel;

import static org.apiguardian.api.API.Status.EXPERIMENTAL;

import org.apiguardian.api.API;

/**
* {@code ResourceLockTarget} is used to define the target of a shared resource.
*
* @since 5.12
* @see ResourceLock#target()
*/
@API(status = EXPERIMENTAL, since = "5.12")
public enum ResourceLockTarget {

/**
* Add a shared resource to the current node.
*/
SELF,

/**
* Add a shared resource to the <em>direct</em> children of the current node.
*
* <p>Examples of "parent - child" relationship in the context of
* {@link ResourceLockTarget}:
* <ul>
* <li>a test class
* - test methods and nested test classes declared in the class.</li>
* <li>a nested test class
* - test methods and nested test classes declared in the nested class.
* </li>
* <li>a test method
* - considered to have no children. Using {@code CHILDREN} for
* a test method results in an exception.</li>
* </ul>
*/
CHILDREN

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

package org.junit.jupiter.engine.descriptor;

import static org.junit.jupiter.api.parallel.ResourceLockTarget.SELF;
import static org.junit.platform.commons.support.AnnotationSupport.findRepeatableAnnotations;
import static org.junit.platform.commons.util.CollectionUtils.toUnmodifiableList;

Expand All @@ -22,6 +23,7 @@

import org.junit.jupiter.api.parallel.ResourceAccessMode;
import org.junit.jupiter.api.parallel.ResourceLock;
import org.junit.jupiter.api.parallel.ResourceLockTarget;
import org.junit.jupiter.api.parallel.ResourceLocksProvider;
import org.junit.platform.commons.JUnitException;
import org.junit.platform.commons.util.ReflectionUtils;
Expand All @@ -42,7 +44,7 @@ Stream<ExclusiveResource> getAllExclusiveResources(
}

@Override
public Stream<ExclusiveResource> getStaticResources() {
Stream<ExclusiveResource> getStaticResourcesFor(ResourceLockTarget target) {
return Stream.empty();
}

Expand All @@ -55,10 +57,10 @@ Stream<ExclusiveResource> getDynamicResources(

Stream<ExclusiveResource> getAllExclusiveResources(
Function<ResourceLocksProvider, Set<ResourceLocksProvider.Lock>> providerToLocks) {
return Stream.concat(getStaticResources(), getDynamicResources(providerToLocks));
return Stream.concat(getStaticResourcesFor(SELF), getDynamicResources(providerToLocks));
}

abstract Stream<ExclusiveResource> getStaticResources();
abstract Stream<ExclusiveResource> getStaticResourcesFor(ResourceLockTarget target);

abstract Stream<ExclusiveResource> getDynamicResources(
Function<ResourceLocksProvider, Set<ResourceLocksProvider.Lock>> providerToLocks);
Expand All @@ -78,9 +80,10 @@ private static class DefaultExclusiveResourceCollector extends ExclusiveResource
}

@Override
public Stream<ExclusiveResource> getStaticResources() {
Stream<ExclusiveResource> getStaticResourcesFor(ResourceLockTarget target) {
return annotations.stream() //
.filter(annotation -> StringUtils.isNotBlank(annotation.value())) //
.filter(annotation -> annotation.target() == target) //
.map(annotation -> new ExclusiveResource(annotation.value(), toLockMode(annotation.mode())));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
package org.junit.jupiter.engine.descriptor;

import static org.apiguardian.api.API.Status.INTERNAL;
import static org.junit.jupiter.api.parallel.ResourceLockTarget.CHILDREN;
import static org.junit.jupiter.engine.descriptor.DisplayNameUtils.determineDisplayNameForMethod;
import static org.junit.platform.commons.util.CollectionUtils.forEachInReverseOrder;

Expand All @@ -27,6 +28,7 @@
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.JUnitException;
import org.junit.platform.commons.logging.Logger;
import org.junit.platform.commons.logging.LoggerFactory;
import org.junit.platform.commons.util.ClassUtils;
Expand Down Expand Up @@ -82,7 +84,15 @@ public final Set<TestTag> getTags() {
@Override
public ExclusiveResourceCollector getExclusiveResourceCollector() {
// There's no need to cache this as this method should only be called once
return ExclusiveResourceCollector.from(getTestMethod());
ExclusiveResourceCollector collector = ExclusiveResourceCollector.from(getTestMethod());

if (collector.getStaticResourcesFor(CHILDREN).findAny().isPresent()) {
String message = "'ResourceLockTarget.CHILDREN' is not supported for methods." + //
" Invalid method: " + getTestMethod();
throw new JUnitException(message);
}

return collector;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

package org.junit.jupiter.engine.descriptor;

import static org.junit.jupiter.api.parallel.ResourceLockTarget.CHILDREN;

import java.util.ArrayDeque;
import java.util.Deque;
import java.util.Set;
Expand Down Expand Up @@ -37,11 +39,15 @@ default Stream<ExclusiveResource> determineExclusiveResources() {
return determineOwnExclusiveResources();
}

Stream<ExclusiveResource> parentStaticResourcesForChildren = ancestors.getLast() //
.getExclusiveResourceCollector().getStaticResourcesFor(CHILDREN);

Stream<ExclusiveResource> ancestorDynamicResources = ancestors.stream() //
.map(ResourceLockAware::getExclusiveResourceCollector) //
.flatMap(collector -> collector.getDynamicResources(this::evaluateResourceLocksProvider));

return Stream.concat(ancestorDynamicResources, determineOwnExclusiveResources());
return Stream.of(ancestorDynamicResources, parentStaticResourcesForChildren, determineOwnExclusiveResources())//
.flatMap(s -> s);
}

default Stream<ExclusiveResource> determineOwnExclusiveResources() {
Expand Down
Loading

0 comments on commit af6b502

Please sign in to comment.