From e1b5473296ede1c8f8cc8d543c508dbcf196a3a2 Mon Sep 17 00:00:00 2001 From: Vladimir Dmitrienko Date: Mon, 12 Aug 2024 21:24:30 +0200 Subject: [PATCH] Move '@ResourceLocksFrom' functionality to '@ResourceLock'. Issue: #2677 --- .../src/docs/asciidoc/link-attributes.adoc | 2 +- .../asciidoc/user-guide/writing-tests.adoc | 47 +++++--- .../DynamicSharedResourcesDemo.java} | 8 +- .../StaticSharedResourcesDemo.java} | 4 +- .../junit/jupiter/api/parallel/Execution.java | 1 - .../junit/jupiter/api/parallel/Isolated.java | 1 - .../jupiter/api/parallel/ResourceLock.java | 25 +++- .../api/parallel/ResourceLocksFrom.java | 65 ---------- .../api/parallel/ResourceLocksProvider.java | 72 ++++++++--- .../descriptor/ClassBasedTestDescriptor.java | 13 +- .../descriptor/JupiterTestDescriptor.java | 39 ++++-- .../descriptor/MethodBasedTestDescriptor.java | 25 +++- .../descriptor/NestedClassTestDescriptor.java | 13 +- .../engine/support/descriptor/LockTests.java | 21 +++- ...ResourceLocksProviderIntegrationTests.java | 114 +++++++++++++++--- 15 files changed, 275 insertions(+), 175 deletions(-) rename documentation/src/test/java/example/{SharedResourcesWithResourceLocksFromAnnotationDemo.java => sharedresources/DynamicSharedResourcesDemo.java} (90%) rename documentation/src/test/java/example/{SharedResourcesWithResourceLockAnnotationDemo.java => sharedresources/StaticSharedResourcesDemo.java} (96%) delete mode 100644 junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/ResourceLocksFrom.java diff --git a/documentation/src/docs/asciidoc/link-attributes.adoc b/documentation/src/docs/asciidoc/link-attributes.adoc index 2e500d569dbc..5c55bec09cf7 100644 --- a/documentation/src/docs/asciidoc/link-attributes.adoc +++ b/documentation/src/docs/asciidoc/link-attributes.adoc @@ -128,7 +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] -:ResourceLocksFrom: {javadoc-root}/org.junit.jupiter.api/org/junit/jupiter/api/parallel/ResourceLocksFrom.html[@ResourceLocksFrom] +: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 :extension-api-package: {javadoc-root}/org.junit.jupiter.api/org/junit/jupiter/api/extension/package-summary.html[org.junit.jupiter.api.extension] diff --git a/documentation/src/docs/asciidoc/user-guide/writing-tests.adoc b/documentation/src/docs/asciidoc/user-guide/writing-tests.adoc index 36606551f7c2..32be6411a770 100644 --- a/documentation/src/docs/asciidoc/user-guide/writing-tests.adoc +++ b/documentation/src/docs/asciidoc/user-guide/writing-tests.adoc @@ -2897,23 +2897,32 @@ strategy In addition to controlling the execution mode using the `{Execution}` annotation, JUnit Jupiter provides another annotation-based declarative synchronization mechanism. The -`{ResourceLock}` and `{ResourceLocksFrom}` annotations allows you to declare that -a test class or method uses a specific shared resource that requires synchronized access -to ensure reliable test execution. The shared resource is identified by a unique name -which is a `String`. The name can be user-defined or one of the predefined constants -in `{Resources}`: `SYSTEM_PROPERTIES`, `SYSTEM_OUT`, `SYSTEM_ERR`, `LOCALE`, or `TIME_ZONE`. +`{ResourceLock}` annotation allows you to declare that a test class or method uses a +specific shared resource that requires synchronized access to ensure reliable test +execution. The shared resource is identified by a unique name which is a `String`. +The name can be user-defined or one of the predefined constants in `{Resources}`: +`SYSTEM_PROPERTIES`, `SYSTEM_OUT`, `SYSTEM_ERR`, `LOCALE`, or `TIME_ZONE`. -If the tests in the following examples were run in parallel _without_ the use of -`{ResourceLock}` or `{ResourceLocksFrom}`, 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. +Since Junit Jupiter 5.12 `{ResourceLock}` annotation has 'providers' attribute +which accepts an array of one or more classes implementing `{ResourceLocksProvider}` +interface. This interface allows to add shared resources in runtime. + +Note that resources declared "statically" with `{ResourceLock}` annotation +are combined with resources added "dynamically" with `{ResourceLocksProvider}` +interface. -When access to shared resources is declared using the `{ResourceLock}` or `{ResourceLocksFrom}` -annotation, the JUnit Jupiter engine uses this information to ensure that no conflicting -tests are run in parallel. This guarantee extends to lifecycle methods of a test class or method. -For example, if a test method is annotated with a `{ResourceLock}` annotation, the "lock" will -be acquired before any `@BeforeEach` methods are executed and released after all -`@AfterEach` methods have been executed. +If the tests in the following examples were run in parallel _without_ the use of +`{ResourceLock}` or `{ResourceLocksProvider}`, 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. + +When access to shared resources is declared using the `{ResourceLock}` annotation +or added with `{ResourceLocksProvider}` interface, the JUnit Jupiter engine uses +this information to ensure that no conflicting tests are run in parallel. +This guarantee extends to lifecycle methods of a test class or method. +For example, if a test method is annotated with a `{ResourceLock}` annotation, +the "lock" will be acquired before any `@BeforeEach` methods are executed +and released after all `@AfterEach` methods have been executed. [NOTE] .Running tests in isolation @@ -2930,15 +2939,15 @@ parallel with each other but not while any other test that requires `READ_WRITE` to the same shared resource is running. [source,java] -.Declaring shared resources with `{ResourceLock}` annotation +.Declaring shared resources "statically" with `{ResourceLock}` annotation ---- -include::{testDir}/example/SharedResourcesWithResourceLockAnnotationDemo.java[tags=user_guide] +include::{testDir}/example/sharedresources/StaticSharedResourcesDemo.java[tags=user_guide] ---- [source,java] -.Declaring shared resources provider with `{ResourceLocksFrom}` annotation +.Adding shared resources "dynamically" with `{ResourceLocksProvider}` interface ---- -include::{testDir}/example/SharedResourcesWithResourceLocksFromAnnotationDemo.java[tags=user_guide] +include::{testDir}/example/sharedresources/DynamicSharedResourcesDemo.java[tags=user_guide] ---- diff --git a/documentation/src/test/java/example/SharedResourcesWithResourceLocksFromAnnotationDemo.java b/documentation/src/test/java/example/sharedresources/DynamicSharedResourcesDemo.java similarity index 90% rename from documentation/src/test/java/example/SharedResourcesWithResourceLocksFromAnnotationDemo.java rename to documentation/src/test/java/example/sharedresources/DynamicSharedResourcesDemo.java index 6b30b1e90f21..54e2dc25ccb7 100644 --- a/documentation/src/test/java/example/SharedResourcesWithResourceLocksFromAnnotationDemo.java +++ b/documentation/src/test/java/example/sharedresources/DynamicSharedResourcesDemo.java @@ -8,7 +8,7 @@ * https://www.eclipse.org/legal/epl-v20.html */ -package example; +package example.sharedresources; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; @@ -27,13 +27,13 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.parallel.Execution; import org.junit.jupiter.api.parallel.ResourceAccessMode; -import org.junit.jupiter.api.parallel.ResourceLocksFrom; +import org.junit.jupiter.api.parallel.ResourceLock; import org.junit.jupiter.api.parallel.ResourceLocksProvider; // tag::user_guide[] @Execution(CONCURRENT) -@ResourceLocksFrom(SharedResourcesWithResourceLocksFromAnnotationDemo.Provider.class) -class SharedResourcesWithResourceLocksFromAnnotationDemo { +@ResourceLock(providers = DynamicSharedResourcesDemo.Provider.class) +class DynamicSharedResourcesDemo { private Properties backup; diff --git a/documentation/src/test/java/example/SharedResourcesWithResourceLockAnnotationDemo.java b/documentation/src/test/java/example/sharedresources/StaticSharedResourcesDemo.java similarity index 96% rename from documentation/src/test/java/example/SharedResourcesWithResourceLockAnnotationDemo.java rename to documentation/src/test/java/example/sharedresources/StaticSharedResourcesDemo.java index ec8d5d730ae4..869f9eb7925d 100644 --- a/documentation/src/test/java/example/SharedResourcesWithResourceLockAnnotationDemo.java +++ b/documentation/src/test/java/example/sharedresources/StaticSharedResourcesDemo.java @@ -8,7 +8,7 @@ * https://www.eclipse.org/legal/epl-v20.html */ -package example; +package example.sharedresources; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; @@ -27,7 +27,7 @@ // tag::user_guide[] @Execution(CONCURRENT) -class SharedResourcesWithResourceLockAnnotationDemo { +class StaticSharedResourcesDemo { private Properties backup; diff --git a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/Execution.java b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/Execution.java index 6ce5f3b2d360..adb45cd2fcf4 100644 --- a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/Execution.java +++ b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/Execution.java @@ -46,7 +46,6 @@ * * @see Isolated * @see ResourceLock - * @see ResourceLocksFrom * @since 5.3 */ @API(status = STABLE, since = "5.10") diff --git a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/Isolated.java b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/Isolated.java index c8fc7247e092..8683cd470969 100644 --- a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/Isolated.java +++ b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/Isolated.java @@ -32,7 +32,6 @@ * @since 5.7 * @see ExecutionMode * @see ResourceLock - * @see ResourceLocksFrom */ @API(status = STABLE, since = "5.10") @Retention(RetentionPolicy.RUNTIME) diff --git a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/ResourceLock.java b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/ResourceLock.java index 93bbd214babc..c6bee1af95d2 100644 --- a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/ResourceLock.java +++ b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/ResourceLock.java @@ -10,6 +10,7 @@ package org.junit.jupiter.api.parallel; +import static org.apiguardian.api.API.Status.EXPERIMENTAL; import static org.apiguardian.api.API.Status.STABLE; import java.lang.annotation.ElementType; @@ -48,11 +49,16 @@ *

Since JUnit Jupiter 5.4, this annotation is {@linkplain Inherited inherited} * within class hierarchies. * + *

Since JUnit Jupiter 5.12, this annotation supports adding shared resources + * in runtime via {@link ResourceLock#providers}. + * + *

Resources declared "statically" using {@code @ResourceLock(value, mode)} + * are combined with "dynamic" resources added via {@link ResourceLocksProvider.Lock}. + * * @see Isolated * @see Resources * @see ResourceAccessMode * @see ResourceLocks - * @see ResourceLocksFrom * @see ResourceLocksProvider * @see ResourceLocksProvider.Lock * @since 5.3 @@ -67,9 +73,12 @@ /** * The resource key. * + *

Defaults to an empty string. + * * @see Resources + * @see ResourceLocksProvider.Lock#getKey() */ - String value(); + String value() default ""; /** * The resource access mode. @@ -77,7 +86,19 @@ *

Defaults to {@link ResourceAccessMode#READ_WRITE READ_WRITE}. * * @see ResourceAccessMode + * @see ResourceLocksProvider.Lock#getAccessMode() */ ResourceAccessMode mode() default ResourceAccessMode.READ_WRITE; + /** + * An array of one or more classes implementing {@link ResourceLocksProvider}. + * + *

Defaults to an empty array. + * + * @see ResourceLocksProvider.Lock + * @since 5.12 + */ + @API(status = EXPERIMENTAL, since = "5.12") + Class[] providers() default {}; + } diff --git a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/ResourceLocksFrom.java b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/ResourceLocksFrom.java deleted file mode 100644 index 3d2ea44978a1..000000000000 --- a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/ResourceLocksFrom.java +++ /dev/null @@ -1,65 +0,0 @@ -/* - * 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 java.lang.annotation.ElementType; -import java.lang.annotation.Inherited; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -import org.apiguardian.api.API; - -/** - * {@code @ResourceLocksFrom} is used to add shared resources - * to an annotated test class and / or its test methods in runtime. - * - *

The {@link #value} should represent - * one or more classes implementing {@link ResourceLocksProvider}. - * - *

This annotation (in conjunction with {@link ResourceLocksProvider}) - * serves the same purpose as {@link ResourceLock} annotation - * but for certain cases may be a more flexible and less verbose alternative - * since it allows to add shared resources programmatically. - * - *

Given this annotation is {@linkplain Inherited inherited}, - * if it's applied to a parent test class, - * then {@link ResourceLocksProvider} methods will be called also - * for child test classes and their test methods. - * - * @apiNote If both {@code @ResourceLock} and {@code @ResourceLocksFrom} - * are used then shared resources from them are summed up. - * It means that if resource 'A' is declared via {@code @ResourceLock} - * and resource 'B' added via {@code @ResourceLocksFrom} - * then a test class or test method will have both resources: 'A' and 'B'. - * - * @since 5.12 - * @see Isolated - * @see Resources - * @see ResourceAccessMode - * @see ResourceLock - * @see ResourceLocksProvider - * @see ResourceLocksProvider.Lock - */ -@API(status = EXPERIMENTAL, since = "5.12") -@Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.TYPE) -@Inherited -public @interface ResourceLocksFrom { - - /** - * An array of one or more classes implementing {@link ResourceLocksProvider}. - */ - Class[] value(); - -} diff --git a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/ResourceLocksProvider.java b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/ResourceLocksProvider.java index 28e109bde02e..3a3130309865 100644 --- a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/ResourceLocksProvider.java +++ b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/parallel/ResourceLocksProvider.java @@ -20,6 +20,7 @@ import org.apiguardian.api.API; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; import org.junit.platform.commons.util.Preconditions; import org.junit.platform.commons.util.ToStringBuilder; @@ -29,23 +30,23 @@ * *

Each shared resource is represented by an instance of {@link Lock}. * - *

This interface (in conjunction with {@link ResourceLocksFrom}) - * serves the same purpose as {@link ResourceLock} annotation - * but for certain cases may be a more flexible and less verbose alternative - * since it allows to add shared resources programmatically. + *

Adding shared resources using this interface has the same semantics + * as declaring them via {@code @ResourceLock(value, mode)} annotation + * but for some cases may be a more flexible and less verbose alternative + * since it allows to add resources programmatically. * - * @apiNote If both {@code @ResourceLock} and {@code ResourceLocksProvider} - * are used then shared resources from them are summed up. - * It means that if resource 'A' is declared via {@code @ResourceLock} - * and resource 'B' added via {@code ResourceLocksProvider} - * then a test class or test method will have both resources: 'A' and 'B'. + * @implNote If both {@code @ResourceLock(value, mode)} + * and {@code ResourceLocksProvider} are used + * then shared resources from them are summed up. + * It means that if resource "A" is declared via {@code @ResourceLock("A")} + * and resource "B" added via {@code new Lock("B")} + * then a test class or test method will have both resources: "A" and "B". * * @since 5.12 * @see Isolated * @see Resources * @see ResourceAccessMode * @see ResourceLock - * @see ResourceLocksFrom * @see Lock */ @API(status = EXPERIMENTAL, since = "5.12") @@ -54,22 +55,34 @@ public interface ResourceLocksProvider { /** * Add shared resources to a test class. * + *

Invoked for a test class annotated with + * {@code @ResourceLock(providers)} and for its child classes. + * + * @apiNote Adding {@linkplain Lock a shared resource} with this method + * has the same semantics as annotating a test class + * with analogous {@code @ResourceLock(value, mode)}. + * * @param testClass a test class to add shared resources * @return a set of {@link Lock}; may be empty - * - * @apiNote has the same semantics as adding {@code @ResourceLock} to a test class. */ default Set provideForClass(Class testClass) { return emptySet(); } /** - * Add shared resources to a nested test class. + * Add shared resources to a {@linkplain Nested nested} test class. + * + *

Invoked for a nested test class + * annotated with {@code @ResourceLock(providers)} + * and for its child classes. + * + * @apiNote Adding {@linkplain Lock a shared resource} with this method + * has the same semantics as annotating a nested test class + * with analogous {@code @ResourceLock(value, mode)}. * * @param testClass a nested test class to add shared resources * @return a set of {@link Lock}; may be empty - * - * @apiNote has the same semantics as adding {@code @ResourceLock} to a nested test class. + * @see Nested */ default Set provideForNestedClass(Class testClass) { return emptySet(); @@ -78,11 +91,22 @@ default Set provideForNestedClass(Class testClass) { /** * Add shared resources to a test method. * - * @param testClass a test class containing the {@code testMethod} + *

Invoked in case: + *

+ * + * @apiNote Adding {@linkplain Lock a shared resource} with this method + * has the same semantics as annotating a test method + * with analogous {@code @ResourceLock(value, mode)}. + * + * @param testClass a test class + * or {@linkplain Nested nested} test class containing the {@code testMethod} * @param testMethod a test method to add shared resources * @return a set of {@link Lock}; may be empty - * - * @apiNote has the same semantics as adding {@code @ResourceLock} to a test method. */ default Set provideForMethod(Class testClass, Method testMethod) { return emptySet(); @@ -110,12 +134,15 @@ default Set provideForMethod(Class testClass, Method testMethod) { * {@link BeforeEach @BeforeEach} methods are executed and released after all * {@link AfterEach @AfterEach} methods have been executed. * + * @apiNote {@link Lock#key} and {@link Lock#accessMode} have the same + * semantics as {@link ResourceLock#value()} and {@link ResourceLock#mode()} + * respectively. + * * @since 5.12 * @see Isolated * @see Resources * @see ResourceAccessMode * @see ResourceLock - * @see ResourceLocksFrom * @see ResourceLocksProvider */ final class Lock { @@ -128,6 +155,7 @@ final class Lock { * Create a new {@code Lock} with {@code accessMode = READ_WRITE}. * * @param key the identifier of the resource; never {@code null} or blank + * @see ResourceLock#value() */ public Lock(String key) { this(key, ResourceAccessMode.READ_WRITE); @@ -138,6 +166,8 @@ public Lock(String key) { * * @param key the identifier of the resource; never {@code null} or blank * @param accessMode the lock mode to use to synchronize access to the resource; never {@code null} + * @see ResourceLock#value() + * @see ResourceLock#mode() */ public Lock(String key, ResourceAccessMode accessMode) { this.key = Preconditions.notBlank(key, "key must not be null or blank"); @@ -146,6 +176,8 @@ public Lock(String key, ResourceAccessMode accessMode) { /** * Get the key of this lock. + * + * @see ResourceLock#value() */ public String getKey() { return key; @@ -153,6 +185,8 @@ public String getKey() { /** * Get the access mode of this lock. + * + * @see ResourceLock#mode() */ public ResourceAccessMode getAccessMode() { return accessMode; diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java index 7223780c4221..ea61465ca395 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java @@ -11,7 +11,6 @@ package org.junit.jupiter.engine.descriptor; import static java.util.stream.Collectors.joining; -import static java.util.stream.Collectors.toSet; import static org.apiguardian.api.API.Status.INTERNAL; import static org.junit.jupiter.engine.descriptor.ExtensionUtils.populateNewExtensionRegistryFromExtendWithAnnotation; import static org.junit.jupiter.engine.descriptor.ExtensionUtils.registerExtensionsFromConstructorParameters; @@ -35,7 +34,6 @@ import java.util.Set; import java.util.function.Function; import java.util.function.Supplier; -import java.util.stream.Stream; import org.apiguardian.api.API; import org.junit.jupiter.api.TestInstance.Lifecycle; @@ -145,13 +143,10 @@ public void setDefaultChildExecutionMode(ExecutionMode defaultChildExecutionMode @Override public Set getExclusiveResources() { // @formatter:off - return Stream.concat( - getExclusiveResourcesFromAnnotation(getTestClass()), - getExclusiveResourcesFromProvider( - getTestClass(), - provider -> provider.provideForClass(getTestClass()) - ) - ).collect(toSet()); + return getExclusiveResourcesFromAnnotation( + getTestClass(), + provider -> provider.provideForClass(getTestClass()) + ); // @formatter:on } diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/JupiterTestDescriptor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/JupiterTestDescriptor.java index c719a365b377..78f19b91562d 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/JupiterTestDescriptor.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/JupiterTestDescriptor.java @@ -12,6 +12,7 @@ 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; @@ -35,7 +36,6 @@ 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.ResourceLocksFrom; import org.junit.jupiter.api.parallel.ResourceLocksProvider; import org.junit.jupiter.engine.config.JupiterConfiguration; import org.junit.jupiter.engine.execution.ConditionEvaluator; @@ -46,6 +46,7 @@ import org.junit.platform.commons.logging.LoggerFactory; import org.junit.platform.commons.util.ExceptionUtils; import org.junit.platform.commons.util.ReflectionUtils; +import org.junit.platform.commons.util.StringUtils; import org.junit.platform.commons.util.UnrecoverableExceptions; import org.junit.platform.engine.TestDescriptor; import org.junit.platform.engine.TestSource; @@ -185,24 +186,38 @@ public static ExecutionMode toExecutionMode(org.junit.jupiter.api.parallel.Execu throw new JUnitException("Unknown ExecutionMode: " + mode); } - Stream getExclusiveResourcesFromAnnotation(AnnotatedElement element) { + Set getExclusiveResourcesFromAnnotation(AnnotatedElement element, + Function> providerToLocks) { + // @formatter:off + return Stream.concat( + getExclusiveResourcesFromAnnotationValue(element), + getExclusiveResourcesFromAnnotationProviders(element, providerToLocks) + ).collect(collectingAndThen(toSet(), Collections::unmodifiableSet)); + // @formatter:on + } + + Stream getExclusiveResourcesFromAnnotationValue(AnnotatedElement element) { // @formatter:off return findRepeatableAnnotations(element, ResourceLock.class).stream() - .map(resource -> new ExclusiveResource(resource.value(), toLockMode(resource.mode()))); + .flatMap(resourceLock -> { + if (StringUtils.isBlank(resourceLock.value())) { + return Stream.empty(); + } + return Stream.of(new ExclusiveResource(resourceLock.value(), toLockMode(resourceLock.mode()))); + }); // @formatter:on } - @SuppressWarnings("Convert2MethodRef") - Stream getExclusiveResourcesFromProvider(Class testClass, + Stream getExclusiveResourcesFromAnnotationProviders(AnnotatedElement element, Function> providerToLocks) { // @formatter:off - return findAnnotation(testClass, ResourceLocksFrom.class) - .map(annotation -> Stream.of(annotation.value())) - .orElseGet(Stream::empty) - .map(providerClass -> ReflectionUtils.newInstance(providerClass)) - .map(providerToLocks) - .flatMap(Collection::stream) - .map(lock -> new ExclusiveResource(lock.getKey(), toLockMode(lock.getAccessMode()))); + return findRepeatableAnnotations(element, ResourceLock.class).stream() + .flatMap(resourceLock -> Stream.of(resourceLock.providers()) + .map(ReflectionUtils::newInstance) + .map(providerToLocks) + .flatMap(Collection::stream) + .map(lock -> new ExclusiveResource(lock.getKey(), toLockMode(lock.getAccessMode()))) + ); // @formatter:on } diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/MethodBasedTestDescriptor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/MethodBasedTestDescriptor.java index 85e3dc0f7c8c..6a8ad8ddee9c 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/MethodBasedTestDescriptor.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/MethodBasedTestDescriptor.java @@ -10,22 +10,26 @@ package org.junit.jupiter.engine.descriptor; +import static java.util.stream.Collectors.collectingAndThen; import static java.util.stream.Collectors.toSet; import static org.apiguardian.api.API.Status.INTERNAL; import static org.junit.jupiter.engine.descriptor.DisplayNameUtils.determineDisplayNameForMethod; import static org.junit.platform.commons.util.CollectionUtils.forEachInReverseOrder; import java.lang.reflect.Method; +import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; import java.util.Set; import java.util.function.Consumer; +import java.util.function.Function; import java.util.stream.Stream; 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.ResourceLocksProvider; import org.junit.jupiter.engine.config.JupiterConfiguration; import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext; import org.junit.platform.commons.logging.Logger; @@ -84,13 +88,22 @@ public final Set getTags() { @Override public Set getExclusiveResources() { // @formatter:off + Function> providerToLocks = + p -> p.provideForMethod(getTestClass(), getTestMethod()); + + Set fromMethodLevelAnnotation = getExclusiveResourcesFromAnnotation( + getTestMethod(), + providerToLocks + ); + Stream fromProvidersInClassLevelAnnotation = getExclusiveResourcesFromAnnotationProviders( + getTestClass(), + providerToLocks + ); + return Stream.concat( - getExclusiveResourcesFromAnnotation(getTestMethod()), - getExclusiveResourcesFromProvider( - getTestClass(), - provider -> provider.provideForMethod(getTestClass(), getTestMethod()) - ) - ).collect(toSet()); + fromMethodLevelAnnotation.stream(), + fromProvidersInClassLevelAnnotation + ).collect(collectingAndThen(toSet(), Collections::unmodifiableSet)); // @formatter:on } diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/NestedClassTestDescriptor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/NestedClassTestDescriptor.java index 9878793d7fb8..ec3e9b12629d 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/NestedClassTestDescriptor.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/NestedClassTestDescriptor.java @@ -11,7 +11,6 @@ package org.junit.jupiter.engine.descriptor; import static java.util.Collections.emptyList; -import static java.util.stream.Collectors.toSet; import static org.apiguardian.api.API.Status.INTERNAL; import static org.junit.jupiter.engine.descriptor.DisplayNameUtils.createDisplayNameSupplierForNestedClass; @@ -20,7 +19,6 @@ import java.util.List; import java.util.Optional; import java.util.Set; -import java.util.stream.Stream; import org.apiguardian.api.API; import org.junit.jupiter.api.extension.ExtensionContext; @@ -93,13 +91,10 @@ protected TestInstances instantiateTestClass(JupiterEngineExecutionContext paren @Override public Set getExclusiveResources() { // @formatter:off - return Stream.concat( - getExclusiveResourcesFromAnnotation(getTestClass()), - getExclusiveResourcesFromProvider( - getTestClass(), - provider -> provider.provideForNestedClass(getTestClass()) - ) - ).collect(toSet()); + return getExclusiveResourcesFromAnnotation( + getTestClass(), + provider -> provider.provideForNestedClass(getTestClass()) + ); // @formatter:on } diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/LockTests.java b/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/LockTests.java index fd85f1950ea7..c836e2122eae 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/LockTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/LockTests.java @@ -10,10 +10,12 @@ package org.junit.platform.engine.support.descriptor; +import static org.junit.jupiter.api.Assertions.assertEquals; +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.ResourceLocksProvider.Lock; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.parallel.ResourceAccessMode; import org.junit.platform.AbstractEqualsAndHashCodeTests; /** @@ -21,14 +23,23 @@ */ class LockTests extends AbstractEqualsAndHashCodeTests { + @Test + void readWriteModeSetByDefault() { + assertEquals(READ_WRITE, new Lock("a").getAccessMode()); + } + @Test void equalsAndHashCode() { - assertEqualsAndHashCode(new Lock("a"), new Lock("a"), new Lock("b")); // @formatter:off assertEqualsAndHashCode( - new Lock("a", ResourceAccessMode.READ_WRITE), - new Lock("a", ResourceAccessMode.READ_WRITE), - new Lock("a", ResourceAccessMode.READ) + new Lock("a", READ_WRITE), + new Lock("a", READ_WRITE), + new Lock("b", READ_WRITE) + ); + assertEqualsAndHashCode( + new Lock("a", READ_WRITE), + new Lock("a", READ_WRITE), + new Lock("a", READ) ); // @formatter:on } diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/ResourceLocksProviderIntegrationTests.java b/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/ResourceLocksProviderIntegrationTests.java index e1609661615d..f6875327abad 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/ResourceLocksProviderIntegrationTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/ResourceLocksProviderIntegrationTests.java @@ -13,6 +13,7 @@ import static java.util.Collections.emptySet; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.request; import static org.junit.platform.testkit.engine.EventConditions.event; @@ -25,7 +26,7 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.parallel.ResourceLocksFrom; +import org.junit.jupiter.api.parallel.ResourceLock; import org.junit.jupiter.api.parallel.ResourceLocksProvider; import org.junit.platform.engine.discovery.DiscoverySelectors; import org.junit.platform.testkit.engine.EngineTestKit; @@ -37,14 +38,20 @@ class ResourceLocksProviderIntegrationTests { @Test - void provideLocksForClassAndMethod() { - var events = execute(ProvideForClassAndMethodTestCase.class); + void provideForClassAndProvideForMethodCalledWithCorrectArguments() { + var events = execute(ProvideForClassAndProvideForMethodTestCase.class); assertThat(events.filter(event(test(), finishedSuccessfully())::matches)).hasSize(2); } @Test - void provideLocksForNestedClassAndMethod() { - var events = execute(ProvideForNestedClassAndMethodTestCase.class); + void provideForNestedClassAndProvideForMethodCalledWithCorrectArguments() { + var events = execute(ProvideForNestedClassAndProvideForMethodTestCase.class); + assertThat(events.filter(event(test(), finishedSuccessfully())::matches)).hasSize(2); + } + + @Test + void provideForMethodCalledWithCorrectArguments() { + var events = execute(ProvideForMethodTestCase.class); assertThat(events.filter(event(test(), finishedSuccessfully())::matches)).hasSize(2); } @@ -61,25 +68,36 @@ private Stream execute(Class testCase) { // ------------------------------------------------------------------------- - @ResourceLocksFrom(ProvideForClassAndMethodTestCase.Provider.class) - static class ProvideForClassAndMethodTestCase { + @ResourceLock(providers = ProvideForClassAndProvideForMethodTestCase.Provider.class) + static class ProvideForClassAndProvideForMethodTestCase { @Test - void classMethod() { + void test() { + assertTrue(Provider.isProvideForClassCalled, "'provideForClass' was not called"); + Provider.isProvideForClassCalled = false; + + assertTrue(Provider.isProvideForMethodCalled, "'provideForMethod' was not called"); + Provider.isProvideForMethodCalled = false; } @Nested class NestedClass { + @Test - void nestedClassMethod() { + void nestedTest() { } } - static final class Provider implements ResourceLocksProvider { + static class Provider implements ResourceLocksProvider { + + private static boolean isProvideForClassCalled = false; + + private static boolean isProvideForMethodCalled = false; @Override public Set provideForClass(Class testClass) { - assertEquals(ProvideForClassAndMethodTestCase.class, testClass); + isProvideForClassCalled = true; + assertEquals(ProvideForClassAndProvideForMethodTestCase.class, testClass); return emptySet(); } @@ -91,28 +109,39 @@ public Set provideForNestedClass(Class testClass) { @Override public Set provideForMethod(Class testClass, Method testMethod) { - assertEquals(ProvideForClassAndMethodTestCase.class, testClass); - assertEquals("classMethod", testMethod.getName()); + isProvideForMethodCalled = true; + assertEquals(ProvideForClassAndProvideForMethodTestCase.class, testClass); + assertEquals("test", testMethod.getName()); return emptySet(); } } } - static class ProvideForNestedClassAndMethodTestCase { + static class ProvideForNestedClassAndProvideForMethodTestCase { @Test - void classMethod() { + void test() { } - @ResourceLocksFrom(ProvideForNestedClassAndMethodTestCase.Provider.class) @Nested + @ResourceLock(providers = ProvideForNestedClassAndProvideForMethodTestCase.Provider.class) class NestedClass { + @Test - void nestedClassMethod() { + void nestedTest() { + assertTrue(Provider.isProvideForNestedClassCalled, "'provideForNestedClass' was not called"); + Provider.isProvideForNestedClassCalled = false; + + assertTrue(Provider.isProvideForMethodCalled, "'provideForMethod' was not called"); + Provider.isProvideForMethodCalled = false; } } - static final class Provider implements ResourceLocksProvider { + static class Provider implements ResourceLocksProvider { + + private static boolean isProvideForNestedClassCalled = false; + + private static boolean isProvideForMethodCalled = false; @Override public Set provideForClass(Class testClass) { @@ -122,14 +151,59 @@ public Set provideForClass(Class testClass) { @Override public Set provideForNestedClass(Class testClass) { + isProvideForNestedClassCalled = true; assertEquals(NestedClass.class, testClass); return emptySet(); } @Override public Set provideForMethod(Class testClass, Method testMethod) { - assertEquals(NestedClass.class, testClass); - assertEquals("nestedClassMethod", testMethod.getName()); + isProvideForMethodCalled = true; + assertEquals(ProvideForNestedClassAndProvideForMethodTestCase.NestedClass.class, testClass); + assertEquals("nestedTest", testMethod.getName()); + return emptySet(); + } + } + } + + static class ProvideForMethodTestCase { + + @Test + @ResourceLock(providers = ProvideForMethodTestCase.Provider.class) + void test() { + assertTrue(Provider.isProvideForMethodCalled, "'provideForMethod' was not called"); + Provider.isProvideForMethodCalled = false; + } + + @Nested + class NestedClass { + + @Test + void nestedTest() { + } + } + + static class Provider implements ResourceLocksProvider { + + private static boolean isProvideForMethodCalled = false; + + @Override + public Set provideForClass(Class testClass) { + fail("'provideForClass' should not be called"); + return emptySet(); + } + + @Override + public Set provideForNestedClass(Class testClass) { + fail("'provideForNestedClass' should not be called"); + return emptySet(); + } + + @Override + public Set provideForMethod(Class testClass, Method testMethod) { + isProvideForMethodCalled = true; + assertEquals(ProvideForMethodTestCase.class, testClass); + assertEquals("test", testMethod.getName()); return emptySet(); } }