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.

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

Allow declaring exclusive resources for child nodes #4151

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 @@ -89,6 +89,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.

Using the `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