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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduced @ResourceLock(target = SELF | CHILDREN) #3220

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -40,7 +40,9 @@ repository on GitHub.
details.
* The new `testfeed` details mode for `ConsoleLauncher` prints test execution events as
they occur in a concise format.

* Introduced `@ResourceLock(target = SELF | CHILDREN)` where the target defaults to SELF to
preserve existing behavior. Using CHILDREN has the same effect as declaring @ResourceLock
on every @Test method of a test class. Please refer to the <<../user-guide/writing-tests.adoc#specifying-target-for-resource-lock, User Guide. Writing Tests>> for details.

[[release-notes-5.10.0-M1-junit-jupiter]]
=== JUnit Jupiter
Expand Down
10 changes: 10 additions & 0 deletions documentation/src/docs/asciidoc/user-guide/writing-tests.adoc
Expand Up @@ -2491,6 +2491,16 @@ to the same shared resource is running.
include::{testDir}/example/SharedResourcesDemo.java[tags=user_guide]
----

[[specifying-target-for-resource-lock]]
You can declare a resource lock on the class level which will apply only to its children,
and will not be applied to the class itself. This is useful if you want to keep `CONCURRENT`
execution mode for each method.

[source,java]
----
include::{testDir}/example/SharedResourcesChildrenTargetDemo.java[tags=user_guide]
----


[[writing-tests-built-in-extensions]]
=== Built-in Extensions
Expand Down
@@ -0,0 +1,70 @@
/*
* Copyright 2015-2023 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;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
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.Resources.SYSTEM_PROPERTIES;
import static org.junit.jupiter.api.parallel.Resources.TIME_ZONE;

import java.util.Properties;
import java.util.TimeZone;

import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.parallel.Execution;
import org.junit.jupiter.api.parallel.ResourceLock;
import org.junit.jupiter.api.parallel.ResourceLockTarget;

// tag::user_guide[]
@Execution(CONCURRENT)
@ResourceLock(value = TIME_ZONE, mode = READ, target = ResourceLockTarget.CHILDREN)
class SharedResourcesChildrenTargetDemo {

private Properties backup;

@BeforeEach
void backup() {
backup = new Properties();
backup.putAll(System.getProperties());
}

@AfterEach
void restore() {
System.setProperties(backup);
}

@Test
@ResourceLock(value = SYSTEM_PROPERTIES, mode = READ)
void usePropertiesAndTimeZoneWithoutModification() {
assertNull(System.getProperty("my.prop"));
}

@Test
@ResourceLock(value = SYSTEM_PROPERTIES, mode = READ)
void usePropertiesAndTimeZoneWithoutModificationAgain() {
assertNull(System.getProperty("my.prop"));
}

@Test
@ResourceLock(value = SYSTEM_PROPERTIES, mode = READ_WRITE)
void canSetCustomPropertyToTimeZone() {
String timezone = TimeZone.getDefault().getDisplayName();
System.setProperty("my.timezone", timezone);
assertEquals(timezone, System.getProperty("my.timezone"));
}

}
// end::user_guide[]
Expand Up @@ -40,6 +40,12 @@
* <p>Since JUnit Jupiter 5.4, this annotation is {@linkplain Inherited inherited}
* within class hierarchies.
*
* <p>Since JUnit Jupiter 5.10, this annotation can be used to specify the target
* of the lock with {@link ResourceLockTarget}. In case the same resource is announced
* for the parent with a target {@link ResourceLockTarget#CHILDREN} and for the child
* with {@link ResourceLockTarget#SELF} they will be merged as if they were declared
* on the child altogether with {@link ResourceLockTarget#SELF} target.
*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if a class has a @ResourceLock annotation with target CHILDREN and a method has one for the same resource? Does the one from the method override the one from the class? This should be documented here.

* @see Isolated
* @see Resources
* @see ResourceAccessMode
Expand Down Expand Up @@ -69,4 +75,13 @@
*/
ResourceAccessMode mode() default ResourceAccessMode.READ_WRITE;

/**
* Resource lock target.
*
* <p>Defaults to {@link ResourceLockTarget#SELF SELF}.
*
* @see ResourceLockTarget
*/
ResourceLockTarget target() default ResourceLockTarget.SELF;

}
@@ -0,0 +1,36 @@
/*
* Copyright 2015-2023 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;

/**
* Indicates the target of a {@link ResourceLock}.
*
* @since 5.10
* @see ResourceLock
*/
@API(status = EXPERIMENTAL, since = "5.10")
public enum ResourceLockTarget {

/**
* Point to the test descriptor itself
*/
SELF,

/**
* Skip the test descriptor itself and apply annotation {@link ResourceLock} to all its children
*/
CHILDREN

}
Expand Up @@ -22,6 +22,7 @@
import static org.junit.jupiter.engine.descriptor.LifecycleMethodUtils.findBeforeEachMethods;
import static org.junit.jupiter.engine.descriptor.TestInstanceLifecycleUtils.getTestInstanceLifecycle;
import static org.junit.jupiter.engine.support.JupiterThrowableCollectorFactory.createThrowableCollector;
import static org.junit.platform.commons.util.AnnotationUtils.findRepeatableAnnotations;
import static org.junit.platform.commons.util.CollectionUtils.forEachInReverseOrder;

import java.lang.reflect.Constructor;
Expand Down Expand Up @@ -52,6 +53,7 @@
import org.junit.jupiter.api.extension.TestInstances;
import org.junit.jupiter.api.extension.TestInstantiationException;
import org.junit.jupiter.api.function.Executable;
import org.junit.jupiter.api.parallel.ResourceLock;
import org.junit.jupiter.engine.config.JupiterConfiguration;
import org.junit.jupiter.engine.execution.AfterEachMethodAdapter;
import org.junit.jupiter.engine.execution.BeforeEachMethodAdapter;
Expand Down Expand Up @@ -142,7 +144,12 @@ public void setDefaultChildExecutionMode(ExecutionMode defaultChildExecutionMode

@Override
public Set<ExclusiveResource> getExclusiveResources() {
return getExclusiveResourcesFromAnnotation(getTestClass());
return collectExclusiveResourcesFromHierarchy();
}

@Override
protected List<ResourceLock> getResourceLocks() {
return findRepeatableAnnotations(getTestClass(), ResourceLock.class);
}

@Override
Expand Down
Expand Up @@ -12,7 +12,6 @@

import static java.util.stream.Collectors.collectingAndThen;
import static java.util.stream.Collectors.toCollection;
import static java.util.stream.Collectors.toSet;
import static org.apiguardian.api.API.Status.INTERNAL;
import static org.junit.jupiter.engine.descriptor.DisplayNameUtils.determineDisplayName;
import static org.junit.platform.commons.util.AnnotationUtils.findAnnotation;
Expand All @@ -25,6 +24,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import org.apiguardian.api.API;
import org.junit.jupiter.api.Tag;
Expand All @@ -33,6 +33,7 @@
import org.junit.jupiter.api.parallel.Execution;
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.engine.config.JupiterConfiguration;
import org.junit.jupiter.engine.execution.ConditionEvaluator;
import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext;
Expand Down Expand Up @@ -180,11 +181,31 @@ public static ExecutionMode toExecutionMode(org.junit.jupiter.api.parallel.Execu
throw new JUnitException("Unknown ExecutionMode: " + mode);
}

Set<ExclusiveResource> getExclusiveResourcesFromAnnotation(AnnotatedElement element) {
protected List<ResourceLock> getResourceLocks() {
return Collections.emptyList();
}

protected Set<ExclusiveResource> collectExclusiveResourcesFromHierarchy() {
Set<ExclusiveResource> resources = mapResourceLocksForTarget(getResourceLocks(), ResourceLockTarget.SELF);

Optional<TestDescriptor> nextParent = getParent();
while (nextParent.isPresent()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC @ResourceLock(target = CHILDREN) should only cause them to be applied to direct children.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC @ResourceLock(target = CHILDREN) should only cause them to be applied to direct children.

Is there any reason for this? Is it harmful in someway for it to not be just direct children? e.g. in our project our tests inherit from base classes scattered throughout our packages and those base classes tend to inherit from a single source parent. If I have a resource that I want read locked for everything (main server component for example) it makes more sense to me to have the lock specified in one place instead of having to remember to add it to all the base classes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Squibbles This is unrelated to inheritance in class hierarchies. The @ResourceLock annotation is inherited to subclasses already. However, the way resource locks are applied is that if a node acquires a lock, its children already hold it while being executed. Therefore it doesn't make sense to recursively declare it for all descendants.

TestDescriptor testDescriptor = nextParent.get();
if (testDescriptor instanceof JupiterTestDescriptor) {
List<ResourceLock> parentLocks = ((JupiterTestDescriptor) testDescriptor).getResourceLocks();
resources.addAll(mapResourceLocksForTarget(parentLocks, ResourceLockTarget.CHILDREN));
}
nextParent = testDescriptor.getParent();
}
return resources;
}

private Set<ExclusiveResource> mapResourceLocksForTarget(List<ResourceLock> locks, ResourceLockTarget target) {
// @formatter:off
return findRepeatableAnnotations(element, ResourceLock.class).stream()
return locks.stream()
.filter(lock -> lock.target() == target)
.map(resource -> new ExclusiveResource(resource.value(), toLockMode(resource.mode())))
.collect(toSet());
.collect(Collectors.toSet());
// @formatter:on
}

Expand Down
Expand Up @@ -12,6 +12,7 @@

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

import java.lang.reflect.Method;
Expand All @@ -20,10 +21,12 @@
import java.util.Optional;
import java.util.Set;
import java.util.function.Consumer;
import java.util.function.Predicate;

import org.apiguardian.api.API;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.TestWatcher;
import org.junit.jupiter.api.parallel.ResourceLock;
import org.junit.jupiter.engine.config.JupiterConfiguration;
import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext;
import org.junit.platform.commons.logging.Logger;
Expand All @@ -47,6 +50,7 @@
public abstract class MethodBasedTestDescriptor extends JupiterTestDescriptor {

private static final Logger logger = LoggerFactory.getLogger(MethodBasedTestDescriptor.class);
public static final Predicate<ResourceLock> ACCEPT_ALL = lock -> true;

private final Class<?> testClass;
private final Method testMethod;
Expand Down Expand Up @@ -81,7 +85,12 @@ public final Set<TestTag> getTags() {

@Override
public Set<ExclusiveResource> getExclusiveResources() {
return getExclusiveResourcesFromAnnotation(getTestMethod());
return collectExclusiveResourcesFromHierarchy();
}

@Override
protected List<ResourceLock> getResourceLocks() {
return findRepeatableAnnotations(getTestMethod(), ResourceLock.class);
}

@Override
Expand Down