From d696878bcda5d05723dbaf6f80884635ffc843f7 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Mon, 7 Oct 2024 13:44:05 +0200 Subject: [PATCH] Avoid multiple class-level annotation lookups Instead of looking up annotations on the declaring and all enclosing classes for each test method, each `ResourceLockAware` test descriptor now delegates to the `ExclusiveResourceCollector` of its ancestors which cache the class-level annotations and `ResourceLocksProvider` instances. Resolves #2677. --- .../descriptor/ClassBasedTestDescriptor.java | 16 ++- .../descriptor/ClassTestDescriptor.java | 13 ++ .../ExclusiveResourceCollector.java | 119 ++++++++++++++++++ .../descriptor/JupiterTestDescriptor.java | 79 +----------- .../descriptor/MethodBasedTestDescriptor.java | 19 +-- .../descriptor/NestedClassTestDescriptor.java | 26 ++-- .../engine/descriptor/ResourceLockAware.java | 55 ++++++++ .../ResourceLocksProviderTests.java | 4 + 8 files changed, 232 insertions(+), 99 deletions(-) create mode 100644 junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ExclusiveResourceCollector.java create mode 100644 junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ResourceLockAware.java 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 ddfbe7ef3a38..6b3b524e6f55 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 @@ -74,7 +74,6 @@ import org.junit.platform.engine.TestTag; import org.junit.platform.engine.UniqueId; import org.junit.platform.engine.support.descriptor.ClassSource; -import org.junit.platform.engine.support.hierarchical.ExclusiveResource; import org.junit.platform.engine.support.hierarchical.ThrowableCollector; /** @@ -83,13 +82,14 @@ * @since 5.5 */ @API(status = INTERNAL, since = "5.5") -public abstract class ClassBasedTestDescriptor extends JupiterTestDescriptor { +public abstract class ClassBasedTestDescriptor extends JupiterTestDescriptor implements ResourceLockAware { private static final InterceptingExecutableInvoker executableInvoker = new InterceptingExecutableInvoker(); private final Class testClass; protected final Set tags; protected final Lifecycle lifecycle; + private final ExclusiveResourceCollector exclusiveResourceCollector; private ExecutionMode defaultChildExecutionMode; private TestInstanceFactory testInstanceFactory; @@ -104,6 +104,7 @@ public abstract class ClassBasedTestDescriptor extends JupiterTestDescriptor { this.tags = getTags(testClass); this.lifecycle = getTestInstanceLifecycle(testClass, configuration); this.defaultChildExecutionMode = (this.lifecycle == Lifecycle.PER_CLASS ? ExecutionMode.SAME_THREAD : null); + this.exclusiveResourceCollector = ExclusiveResourceCollector.from(testClass); } // --- TestDescriptor ------------------------------------------------------ @@ -112,6 +113,8 @@ public final Class getTestClass() { return this.testClass; } + public abstract List> getEnclosingTestClasses(); + @Override public Type getType() { return Type.CONTAINER; @@ -139,13 +142,8 @@ public void setDefaultChildExecutionMode(ExecutionMode defaultChildExecutionMode } @Override - public Set getExclusiveResources() { - // @formatter:off - return getExclusiveResourcesFromAnnotations( - getTestClass(), - provider -> provider.provideForClass(getTestClass()) - ); - // @formatter:on + public final ExclusiveResourceCollector getExclusiveResourceCollector() { + return exclusiveResourceCollector; } @Override diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassTestDescriptor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassTestDescriptor.java index a7c3034a8f60..e644518fb57b 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassTestDescriptor.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassTestDescriptor.java @@ -10,16 +10,19 @@ package org.junit.jupiter.engine.descriptor; +import static java.util.Collections.emptyList; import static org.apiguardian.api.API.Status.INTERNAL; import static org.junit.jupiter.engine.descriptor.DisplayNameUtils.createDisplayNameSupplierForClass; import java.util.LinkedHashSet; +import java.util.List; import java.util.Optional; import java.util.Set; import org.apiguardian.api.API; import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.api.extension.TestInstances; +import org.junit.jupiter.api.parallel.ResourceLocksProvider; import org.junit.jupiter.engine.config.JupiterConfiguration; import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext; import org.junit.jupiter.engine.extension.ExtensionRegistrar; @@ -57,6 +60,11 @@ public Set getTags() { return new LinkedHashSet<>(this.tags); } + @Override + public List> getEnclosingTestClasses() { + return emptyList(); + } + // --- Node ---------------------------------------------------------------- @Override @@ -72,4 +80,9 @@ protected TestInstances instantiateTestClass(JupiterEngineExecutionContext paren return instantiateTestClass(Optional.empty(), registry, extensionContext); } + @Override + public Set evaluateResourceLocksProvider(ResourceLocksProvider provider) { + return provider.provideForClass(getTestClass()); + } + } diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ExclusiveResourceCollector.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ExclusiveResourceCollector.java new file mode 100644 index 000000000000..42b1295b2221 --- /dev/null +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ExclusiveResourceCollector.java @@ -0,0 +1,119 @@ +/* + * 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.engine.descriptor; + +import static org.junit.platform.commons.support.AnnotationSupport.findRepeatableAnnotations; +import static org.junit.platform.commons.util.CollectionUtils.toUnmodifiableList; + +import java.lang.reflect.AnnotatedElement; +import java.util.Collection; +import java.util.List; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Stream; + +import org.junit.jupiter.api.parallel.ResourceAccessMode; +import org.junit.jupiter.api.parallel.ResourceLock; +import org.junit.jupiter.api.parallel.ResourceLocksProvider; +import org.junit.platform.commons.JUnitException; +import org.junit.platform.commons.util.ReflectionUtils; +import org.junit.platform.commons.util.StringUtils; +import org.junit.platform.engine.support.hierarchical.ExclusiveResource; + +/** + * @since 5.12 + */ +abstract class ExclusiveResourceCollector { + + private static final ExclusiveResourceCollector NO_EXCLUSIVE_RESOURCES = new ExclusiveResourceCollector() { + + @Override + Stream getAllExclusiveResources( + Function> providerToLocks) { + return Stream.empty(); + } + + @Override + public Stream getStaticResources() { + return Stream.empty(); + } + + @Override + Stream getDynamicResources( + Function> providerToLocks) { + return Stream.empty(); + } + }; + + Stream getAllExclusiveResources( + Function> providerToLocks) { + return Stream.concat(getStaticResources(), getDynamicResources(providerToLocks)); + } + + abstract Stream getStaticResources(); + + abstract Stream getDynamicResources( + Function> providerToLocks); + + static ExclusiveResourceCollector from(AnnotatedElement element) { + List annotations = findRepeatableAnnotations(element, ResourceLock.class); + return annotations.isEmpty() ? NO_EXCLUSIVE_RESOURCES : new DefaultExclusiveResourceCollector(annotations); + } + + private static class DefaultExclusiveResourceCollector extends ExclusiveResourceCollector { + + private final List annotations; + private List providers; + + DefaultExclusiveResourceCollector(List annotations) { + this.annotations = annotations; + } + + @Override + public Stream getStaticResources() { + return annotations.stream() // + .filter(annotation -> StringUtils.isNotBlank(annotation.value())) // + .map(annotation -> new ExclusiveResource(annotation.value(), toLockMode(annotation.mode()))); + } + + @Override + Stream getDynamicResources( + Function> providerToLocks) { + List providers = getProviders(); + if (providers.isEmpty()) { + return Stream.empty(); + } + return providers.stream() // + .map(providerToLocks) // + .flatMap(Collection::stream) // + .map(lock -> new ExclusiveResource(lock.getKey(), toLockMode(lock.getAccessMode()))); + } + + private List getProviders() { + if (this.providers == null) { + this.providers = annotations.stream() // + .flatMap(annotation -> Stream.of(annotation.providers()).map(ReflectionUtils::newInstance)) // + .collect(toUnmodifiableList()); + } + return providers; + } + + private static ExclusiveResource.LockMode toLockMode(ResourceAccessMode mode) { + switch (mode) { + case READ: + return ExclusiveResource.LockMode.READ; + case READ_WRITE: + return ExclusiveResource.LockMode.READ_WRITE; + } + throw new JUnitException("Unknown ResourceAccessMode: " + mode); + } + } +} 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 54673dcb91d5..86fecaf73ddd 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 @@ -10,10 +10,9 @@ package org.junit.jupiter.engine.descriptor; -import static java.util.Collections.emptyList; +import static java.util.Collections.emptySet; import static java.util.stream.Collectors.collectingAndThen; import static java.util.stream.Collectors.toCollection; -import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toSet; import static org.apiguardian.api.API.Status.INTERNAL; import static org.junit.jupiter.engine.descriptor.DisplayNameUtils.determineDisplayName; @@ -21,25 +20,18 @@ import static org.junit.platform.commons.support.AnnotationSupport.findRepeatableAnnotations; import java.lang.reflect.AnnotatedElement; -import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; 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.Tag; import org.junit.jupiter.api.extension.ConditionEvaluationResult; import org.junit.jupiter.api.extension.Extension; 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.ResourceLocksProvider; import org.junit.jupiter.engine.config.JupiterConfiguration; import org.junit.jupiter.engine.execution.ConditionEvaluator; import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext; @@ -48,8 +40,6 @@ import org.junit.platform.commons.logging.Logger; 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; @@ -57,7 +47,6 @@ import org.junit.platform.engine.UniqueId; 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.Node; /** @@ -109,17 +98,6 @@ static Set getTags(AnnotatedElement element) { // @formatter:on } - public List> getEnclosingTestClasses() { - TestDescriptor parent = getParent().orElse(null); - if (parent instanceof ClassBasedTestDescriptor) { - ClassBasedTestDescriptor parentClassDescriptor = (ClassBasedTestDescriptor) parent; - List> result = new ArrayList<>(parentClassDescriptor.getEnclosingTestClasses()); - result.add(parentClassDescriptor.getTestClass()); - return result; - } - return emptyList(); - } - /** * Invoke exception handlers for the supplied {@code Throwable} one-by-one * until none are left or the throwable to handle has been swallowed. @@ -200,57 +178,12 @@ public static ExecutionMode toExecutionMode(org.junit.jupiter.api.parallel.Execu throw new JUnitException("Unknown ExecutionMode: " + mode); } - Set getExclusiveResourcesFromAnnotations(AnnotatedElement element, - Function> providerToLocks) { - // @formatter:off - List ownAnnotations = findRepeatableAnnotations(element, ResourceLock.class); - List enclosingClassesAnnotations = getEnclosingTestClasses().stream() - .map(clazz -> findRepeatableAnnotations(clazz, ResourceLock.class)) - .flatMap(Collection::stream) - .collect(toList()); - - return Stream.of( - getExclusiveResourcesFromValues(ownAnnotations), - getExclusiveResourcesFromProviders(ownAnnotations, providerToLocks), - getExclusiveResourcesFromProviders(enclosingClassesAnnotations, providerToLocks) - ).flatMap(s -> s) - .collect(toSet()); - // @formatter:on - } - - private Stream getExclusiveResourcesFromValues(List annotations) { - // @formatter:off - return annotations.stream() - .flatMap(annotation -> { - if (StringUtils.isBlank(annotation.value())) { - return Stream.empty(); - } - return Stream.of(new ExclusiveResource(annotation.value(), toLockMode(annotation.mode()))); - }); - // @formatter:on - } - - private Stream getExclusiveResourcesFromProviders(List annotations, - Function> providerToLocks) { - // @formatter:off - return annotations.stream() - .flatMap(annotation -> Stream.of(annotation.providers()) - .map(ReflectionUtils::newInstance) - .map(providerToLocks) - .flatMap(Collection::stream) - .map(lock -> new ExclusiveResource(lock.getKey(), toLockMode(lock.getAccessMode()))) - ); - // @formatter:on - } - - private static LockMode toLockMode(ResourceAccessMode mode) { - switch (mode) { - case READ: - return LockMode.READ; - case READ_WRITE: - return LockMode.READ_WRITE; + @Override + public Set getExclusiveResources() { + if (this instanceof ResourceLockAware) { + return ((ResourceLockAware) this).determineExclusiveResources().collect(toSet()); } - throw new JUnitException("Unknown ResourceAccessMode: " + mode); + return emptySet(); } @Override 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 6a21a3ae6660..9c8e4bcb1ce3 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 @@ -24,6 +24,7 @@ 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; @@ -36,7 +37,6 @@ import org.junit.platform.engine.TestTag; import org.junit.platform.engine.UniqueId; import org.junit.platform.engine.support.descriptor.MethodSource; -import org.junit.platform.engine.support.hierarchical.ExclusiveResource; /** * Base class for {@link TestDescriptor TestDescriptors} based on Java methods. @@ -44,7 +44,7 @@ * @since 5.0 */ @API(status = INTERNAL, since = "5.0") -public abstract class MethodBasedTestDescriptor extends JupiterTestDescriptor { +public abstract class MethodBasedTestDescriptor extends JupiterTestDescriptor implements ResourceLockAware { private static final Logger logger = LoggerFactory.getLogger(MethodBasedTestDescriptor.class); @@ -80,13 +80,14 @@ public final Set getTags() { } @Override - public Set getExclusiveResources() { - // @formatter:off - return getExclusiveResourcesFromAnnotations( - getTestMethod(), - provider -> provider.provideForMethod(getTestClass(), getTestMethod()) - ); - // @formatter:on + public ExclusiveResourceCollector getExclusiveResourceCollector() { + // There's no need to cache this as this method should only be called once + return ExclusiveResourceCollector.from(getTestMethod()); + } + + @Override + public Set evaluateResourceLocksProvider(ResourceLocksProvider provider) { + return provider.provideForMethod(getTestClass(), getTestMethod()); } @Override 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 7732a85854f8..0c1f697b2686 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 @@ -10,16 +10,20 @@ package org.junit.jupiter.engine.descriptor; +import static java.util.Collections.emptyList; import static org.apiguardian.api.API.Status.INTERNAL; import static org.junit.jupiter.engine.descriptor.DisplayNameUtils.createDisplayNameSupplierForNestedClass; +import java.util.ArrayList; import java.util.LinkedHashSet; +import java.util.List; import java.util.Optional; import java.util.Set; import org.apiguardian.api.API; import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.api.extension.TestInstances; +import org.junit.jupiter.api.parallel.ResourceLocksProvider; import org.junit.jupiter.engine.config.JupiterConfiguration; import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext; import org.junit.jupiter.engine.extension.ExtensionRegistrar; @@ -27,7 +31,6 @@ import org.junit.platform.engine.TestDescriptor; import org.junit.platform.engine.TestTag; import org.junit.platform.engine.UniqueId; -import org.junit.platform.engine.support.hierarchical.ExclusiveResource; import org.junit.platform.engine.support.hierarchical.ThrowableCollector; /** @@ -59,6 +62,18 @@ public final Set getTags() { return allTags; } + @Override + public List> getEnclosingTestClasses() { + TestDescriptor parent = getParent().orElse(null); + if (parent instanceof ClassBasedTestDescriptor) { + ClassBasedTestDescriptor parentClassDescriptor = (ClassBasedTestDescriptor) parent; + List> result = new ArrayList<>(parentClassDescriptor.getEnclosingTestClasses()); + result.add(parentClassDescriptor.getTestClass()); + return result; + } + return emptyList(); + } + // --- Node ---------------------------------------------------------------- @Override @@ -74,13 +89,8 @@ protected TestInstances instantiateTestClass(JupiterEngineExecutionContext paren } @Override - public Set getExclusiveResources() { - // @formatter:off - return getExclusiveResourcesFromAnnotations( - getTestClass(), - provider -> provider.provideForNestedClass(getTestClass()) - ); - // @formatter:on + public Set evaluateResourceLocksProvider(ResourceLocksProvider provider) { + return provider.provideForNestedClass(getTestClass()); } } diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ResourceLockAware.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ResourceLockAware.java new file mode 100644 index 000000000000..858644d48520 --- /dev/null +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ResourceLockAware.java @@ -0,0 +1,55 @@ +/* + * 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.engine.descriptor; + +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.Set; +import java.util.stream.Stream; + +import org.junit.jupiter.api.parallel.ResourceLocksProvider; +import org.junit.platform.engine.TestDescriptor; +import org.junit.platform.engine.support.hierarchical.ExclusiveResource; + +/** + * @since 5.12 + */ +interface ResourceLockAware extends TestDescriptor { + + default Stream determineExclusiveResources() { + + Deque ancestors = new ArrayDeque<>(); + TestDescriptor parent = this.getParent().orElse(null); + while (parent instanceof ResourceLockAware) { + ancestors.addFirst((ResourceLockAware) parent); + parent = parent.getParent().orElse(null); + } + + if (ancestors.isEmpty()) { + return determineOwnExclusiveResources(); + } + + Stream ancestorDynamicResources = ancestors.stream() // + .map(ResourceLockAware::getExclusiveResourceCollector) // + .flatMap(collector -> collector.getDynamicResources(this::evaluateResourceLocksProvider)); + + return Stream.concat(ancestorDynamicResources, determineOwnExclusiveResources()); + } + + default Stream determineOwnExclusiveResources() { + return this.getExclusiveResourceCollector().getAllExclusiveResources(this::evaluateResourceLocksProvider); + } + + ExclusiveResourceCollector getExclusiveResourceCollector(); + + Set evaluateResourceLocksProvider(ResourceLocksProvider provider); + +} diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/ResourceLocksProviderTests.java b/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/ResourceLocksProviderTests.java index f9b350ccbc6b..3b60bc75569a 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/ResourceLocksProviderTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/ResourceLocksProviderTests.java @@ -77,6 +77,7 @@ private Stream execute(Class testCase) { // ------------------------------------------------------------------------- + @SuppressWarnings("JUnitMalformedDeclaration") @ResourceLock(providers = ClassLevelProviderTestCase.Provider.class) static class ClassLevelProviderTestCase { @@ -150,6 +151,7 @@ public Set provideForMethod(Class testClass, Method testMethod) { } } + @SuppressWarnings("JUnitMalformedDeclaration") static class NestedClassLevelProviderTestCase { @Test @@ -202,6 +204,7 @@ public Set provideForMethod(Class testClass, Method testMethod) { } } + @SuppressWarnings("JUnitMalformedDeclaration") static class MethodLevelProviderTestCase { @Test @@ -249,6 +252,7 @@ public Set provideForMethod(Class testClass, Method testMethod) { } } + @SuppressWarnings("JUnitMalformedDeclaration") static class MethodLevelProviderInNestedClassTestCase { @Test