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 4 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,9 @@
* <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}.
*
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 +72,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}.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Indicates the target of a {@link ResourceLock}.
* 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 @@ -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 All @@ -49,6 +50,7 @@
import org.junit.platform.engine.support.descriptor.AbstractTestDescriptor;
import org.junit.platform.engine.support.hierarchical.ExclusiveResource;
import org.junit.platform.engine.support.hierarchical.ExclusiveResource.LockMode;
import org.junit.platform.engine.support.hierarchical.ExclusiveResource.LockScope;
import org.junit.platform.engine.support.hierarchical.Node;

/**
Expand Down Expand Up @@ -183,7 +185,11 @@ public static ExecutionMode toExecutionMode(org.junit.jupiter.api.parallel.Execu
Set<ExclusiveResource> getExclusiveResourcesFromAnnotation(AnnotatedElement element) {
// @formatter:off
return findRepeatableAnnotations(element, ResourceLock.class).stream()
.map(resource -> new ExclusiveResource(resource.value(), toLockMode(resource.mode())))
.map(resource -> new ExclusiveResource(
resource.value(),
toLockMode(resource.mode()),
toLockScope(resource.target()))
)
.collect(toSet());
// @formatter:on
}
Expand All @@ -198,6 +204,16 @@ private static LockMode toLockMode(ResourceAccessMode mode) {
throw new JUnitException("Unknown ResourceAccessMode: " + mode);
}

private static LockScope toLockScope(ResourceLockTarget lockTarget) {
switch (lockTarget) {
case SELF:
return LockScope.SELF;
case CHILDREN:
return LockScope.CHILDREN;
}
throw new JUnitException("Unknown ResourceLockTarget: " + lockTarget);
}

@Override
public SkipResult shouldBeSkipped(JupiterEngineExecutionContext context) throws Exception {
context.getThrowableCollector().assertEmpty();
Expand Down
Expand Up @@ -52,18 +52,34 @@ public class ExclusiveResource {

private final String key;
private final LockMode lockMode;
private final LockScope lockScope;
private int hash;

/**
* Create a new {@code ExclusiveResource}.
* Create a new {@code ExclusiveResource} with a default lock scope {@link LockScope#SELF}
*
* @param key the identifier of the resource; never {@code null} or blank
* @param lockMode the lock mode to use to synchronize access to the
* resource; never {@code null}
*
*/
public ExclusiveResource(String key, LockMode lockMode) {
this(key, lockMode, LockScope.SELF);
}

/**
* Create a new {@code ExclusiveResource}.
*
* @param key the identifier of the resource; never {@code null} or blank
* @param lockMode the lock mode to use to synchronize access to the
* resource; never {@code null}
* @param lockScope the lock scope to use to synchronize access to the
* resource; never {@code null}
*/
public ExclusiveResource(String key, LockMode lockMode, LockScope lockScope) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this should be a Platform concept. Instead, Jupiter should return ExclusiveResource instances from the children TestDescriptors if their parent declares a @ResourceLock with target of CHILDREN.

Copy link
Author

Choose a reason for hiding this comment

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

sounds good, no need to bother Platform with those details :) I drafted how it may look in https://github.com/junit-team/junit5/pull/3220/files#diff-63a01db8dd78b02e05c3888eb786c656af0cd3dc8263c816530003f2d7069daeR188

this.key = Preconditions.notBlank(key, "key must not be blank");
this.lockMode = Preconditions.notNull(lockMode, "lockMode must not be null");
this.lockScope = Preconditions.notNull(lockScope, "lockScope must not be null");
}

/**
Expand All @@ -80,6 +96,13 @@ public LockMode getLockMode() {
return lockMode;
}

/**
* Get the lock scope of this resource.
*/
public LockScope getLockScope() {
return lockScope;
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand All @@ -89,21 +112,26 @@ public boolean equals(Object o) {
return false;
}
ExclusiveResource that = (ExclusiveResource) o;
return Objects.equals(key, that.key) && lockMode == that.lockMode;
return Objects.equals(key, that.key) && lockMode == that.lockMode && lockScope == that.lockScope;
}

@Override
public int hashCode() {
int h = hash;
if (h == 0) {
h = hash = Objects.hash(key, lockMode);
h = hash = Objects.hash(key, lockMode, lockScope);
}
return h;
}

@Override
public String toString() {
return new ToStringBuilder(this).append("key", key).append("lockMode", lockMode).toString();
return new ToStringBuilder(this).append("key", key).append("lockMode", lockMode).append("lockScope",
lockScope).toString();
}

public ExclusiveResource convertToSelfTarget() {
return new ExclusiveResource(key, lockMode, LockScope.SELF);
}

/**
Expand Down Expand Up @@ -131,4 +159,21 @@ public enum LockMode {

}

/**
* {@code LockTarget} defines the scope of the lock.
*/
public enum LockScope {

/**
* Lock the resource for the node itself.
*/
SELF,

/**
* Lock the resource for all children of the node. Bypass the node itself.
*/
CHILDREN

}

}