diff --git a/build-logic/src/main/kotlin/nessie-testing.gradle.kts b/build-logic/src/main/kotlin/nessie-testing.gradle.kts index 89167e1108d..4f435de4b3f 100644 --- a/build-logic/src/main/kotlin/nessie-testing.gradle.kts +++ b/build-logic/src/main/kotlin/nessie-testing.gradle.kts @@ -108,7 +108,7 @@ tasks.withType().configureEach { ) minHeapSize = if (testHeapSize != null) testHeapSize as String else "768m" - maxHeapSize = if (testHeapSize != null) testHeapSize as String else "3g" + maxHeapSize = if (testHeapSize != null) testHeapSize as String else "2g" } else if (testHeapSize != null) { minHeapSize = testHeapSize!! maxHeapSize = testHeapSize!! diff --git a/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/MultiEnvExtensionRegistry.java b/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/MultiEnvExtensionRegistry.java index b886d03f2f5..617db374e89 100644 --- a/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/MultiEnvExtensionRegistry.java +++ b/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/MultiEnvExtensionRegistry.java @@ -17,10 +17,14 @@ import java.util.Arrays; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Stream; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.extension.Extension; import org.junit.jupiter.engine.config.DefaultJupiterConfiguration; +import org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor; import org.junit.jupiter.engine.extension.MutableExtensionRegistry; import org.junit.platform.commons.util.AnnotationUtils; @@ -33,17 +37,40 @@ public class MultiEnvExtensionRegistry { private final MutableExtensionRegistry registry; + private final Set probablyNotMultiEnv = new LinkedHashSet<>(); + public MultiEnvExtensionRegistry() { this.registry = MutableExtensionRegistry.createRegistryWithDefaultExtensions( new DefaultJupiterConfiguration(new EmptyConfigurationParameters())); } - public void registerExtensions(Class testClass) { - AnnotationUtils.findRepeatableAnnotations(testClass, ExtendWith.class).stream() - .flatMap(e -> Arrays.stream(e.value())) - .filter(MultiEnvTestExtension.class::isAssignableFrom) + public void registerExtensions(ClassBasedTestDescriptor descriptor) { + AtomicBoolean multiEnv = new AtomicBoolean(false); + + findMultiEnvExtensions(descriptor) + .peek(x -> multiEnv.set(true)) .forEach(registry::registerExtension); + + if (!multiEnv.get()) { + probablyNotMultiEnv.add(descriptor); + } + } + + public boolean isMultiEnvClass(ClassBasedTestDescriptor descriptor) { + return findMultiEnvExtensions(descriptor).findFirst().isPresent(); + } + + private Stream> findMultiEnvExtensions( + ClassBasedTestDescriptor descriptor) { + Class testClass = descriptor.getTestClass(); + return AnnotationUtils.findRepeatableAnnotations(testClass, ExtendWith.class).stream() + .flatMap(e -> Arrays.stream(e.value())) + .filter(MultiEnvTestExtension.class::isAssignableFrom); + } + + public Stream probablyNotMultiEnv() { + return probablyNotMultiEnv.stream(); } public Stream stream() { diff --git a/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/MultiEnvTestEngine.java b/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/MultiEnvTestEngine.java index db360fc8c6e..ceb1c40b9b6 100644 --- a/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/MultiEnvTestEngine.java +++ b/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/MultiEnvTestEngine.java @@ -26,6 +26,7 @@ import org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor; import org.junit.jupiter.engine.descriptor.JupiterEngineDescriptor; import org.junit.jupiter.engine.discovery.DiscoverySelectorResolver; +import org.junit.platform.engine.ConfigurationParameters; import org.junit.platform.engine.EngineDiscoveryRequest; import org.junit.platform.engine.ExecutionRequest; import org.junit.platform.engine.TestDescriptor; @@ -51,7 +52,8 @@ public class MultiEnvTestEngine implements TestEngine { private static final MultiEnvExtensionRegistry registry = new MultiEnvExtensionRegistry(); - private final JupiterTestEngine delegate = new JupiterTestEngine(); + private final ThreadPerTestClassExecutionTestEngine delegate = + new ThreadPerTestClassExecutionTestEngine(); static MultiEnvExtensionRegistry registry() { return registry; @@ -76,36 +78,36 @@ public TestDescriptor discover(EngineDiscoveryRequest discoveryRequest, UniqueId preliminaryResult.accept( descriptor -> { if (descriptor instanceof ClassBasedTestDescriptor) { - Class testClass = ((ClassBasedTestDescriptor) descriptor).getTestClass(); - registry.registerExtensions(testClass); + registry.registerExtensions(((ClassBasedTestDescriptor) descriptor)); } }); + ConfigurationParameters configurationParameters = + discoveryRequest.getConfigurationParameters(); + // JupiterEngineDescriptor must be the root, that's what the JUnit Jupiter engine // implementation expects. - JupiterEngineDescriptor multiEnvDescriptor = + JupiterEngineDescriptor multiEnvRootDescriptor = new JupiterEngineDescriptor( - uniqueId, - new DefaultJupiterConfiguration(discoveryRequest.getConfigurationParameters())); + uniqueId, new DefaultJupiterConfiguration(configurationParameters)); + // Handle the "multi-env" tests. List extensions = new ArrayList<>(); - AtomicBoolean envDiscovered = new AtomicBoolean(); + AtomicBoolean multiEnvDiscovered = new AtomicBoolean(); registry.stream() .forEach( ext -> { extensions.add(ext.getClass().getSimpleName()); - for (String envId : - ext.allEnvironmentIds(discoveryRequest.getConfigurationParameters())) { - envDiscovered.set(true); + for (String envId : ext.allEnvironmentIds(configurationParameters)) { + multiEnvDiscovered.set(true); UniqueId segment = uniqueId.append(ext.segmentType(), envId); MultiEnvTestDescriptor envRoot = new MultiEnvTestDescriptor(segment, envId); - multiEnvDescriptor.addChild(envRoot); + multiEnvRootDescriptor.addChild(envRoot); JupiterConfiguration envRootConfiguration = new CachingJupiterConfiguration( - new MultiEnvJupiterConfiguration( - discoveryRequest.getConfigurationParameters(), envId)); + new MultiEnvJupiterConfiguration(configurationParameters, envId)); JupiterEngineDescriptor discoverResult = new JupiterEngineDescriptor(segment, envRootConfiguration); new DiscoverySelectorResolver() @@ -116,18 +118,53 @@ public TestDescriptor discover(EngineDiscoveryRequest discoveryRequest, UniqueId for (TestDescriptor child : children) { // Note: this also removes the reference to parent from the child discoverResult.removeChild(child); - envRoot.addChild(child); + + // Must check whether the test class is a multi-env test, because discovery + // returns all test classes. + ClassBasedTestDescriptor classBased = (ClassBasedTestDescriptor) child; + boolean multi = registry().isMultiEnvClass(classBased); + if (multi) { + envRoot.addChild(child); + } + } + } + }); + + // Also execute all other tests via the MultiEnv test engine to get the "thread per + // test-class" behavior. + registry() + .probablyNotMultiEnv() + .forEach( + clazz -> { + JupiterConfiguration jupiterConfiguration = + new CachingJupiterConfiguration( + new DefaultJupiterConfiguration(configurationParameters)); + + JupiterEngineDescriptor discoverResult = + new JupiterEngineDescriptor(uniqueId, jupiterConfiguration); + new DiscoverySelectorResolver().resolveSelectors(discoveryRequest, discoverResult); + + List children = + new ArrayList<>(discoverResult.getChildren()); + for (TestDescriptor child : children) { + // Must check whether the test class is not a multi-env test here, as the + // `multiEnvNotDetected` contains some actual multi-env tests. + ClassBasedTestDescriptor classBased = (ClassBasedTestDescriptor) child; + boolean multi = registry().isMultiEnvClass(classBased); + if (!multi) { + discoverResult.removeChild(child); + multiEnvRootDescriptor.addChild(child); } } }); - if (!extensions.isEmpty() && !envDiscovered.get() && FAIL_ON_MISSING_ENVIRONMENTS) { + if (!extensions.isEmpty() && !multiEnvDiscovered.get() && FAIL_ON_MISSING_ENVIRONMENTS) { throw new IllegalStateException( String.format( "%s was enabled, but test extensions did not discover any environment IDs: %s", getClass().getSimpleName(), extensions)); } - return multiEnvDescriptor; + return multiEnvRootDescriptor; } catch (Exception e) { LOGGER.error("Failed to discover tests", e); throw new RuntimeException(e); diff --git a/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/MultiEnvTestFilter.java b/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/MultiEnvTestFilter.java index eb24474886b..57108dd5711 100644 --- a/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/MultiEnvTestFilter.java +++ b/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/MultiEnvTestFilter.java @@ -15,8 +15,6 @@ */ package org.projectnessie.junit.engine; -import static org.projectnessie.junit.engine.MultiEnvTestEngine.registry; - import java.util.Optional; import org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor; import org.junit.platform.engine.FilterResult; @@ -36,7 +34,7 @@ */ public class MultiEnvTestFilter implements PostDiscoveryFilter { - private Optional> classFor(TestDescriptor object) { + static Optional> classFor(TestDescriptor object) { for (TestDescriptor d = object; d != null; d = d.getParent().orElse(null)) { if (d instanceof ClassBasedTestDescriptor) { return Optional.of(((ClassBasedTestDescriptor) d).getTestClass()); @@ -46,39 +44,27 @@ private Optional> classFor(TestDescriptor object) { return Optional.empty(); } - private FilterResult filter(Class testClass, UniqueId id) { - // Use the static extension data collected during the discovery phase. - // It is possible to reload extensions based of class objects from test descriptors, - // however that would add unnecessary overhead. - MultiEnvExtensionRegistry registry = registry(); - + /** + * This filter effectively routes all tests via the {@link MultiEnvTestEngine}, both actual + * multi-env tests but also non-multi-env tests to achieve the needed thread-per-test-class + * behavior. + * + *

"Thread-per-test-class behavior" is needed to prevent the class/class-loader leak via {@link + * ThreadLocal}s as described in #9441. + */ + private FilterResult filter(UniqueId id) { if (id.getEngineId().map("junit-jupiter"::equals).orElse(false)) { - if (registry.stream(testClass).findAny().isPresent()) { - return FilterResult.excluded("Excluding multi-env test from Jupiter Engine: " + id); - } else { - return FilterResult.included(null); - } + return FilterResult.excluded("Excluding multi-env test from Jupiter Engine: " + id); } else { - // check whether any of the extensions declared by the test recognize the version segment - boolean matched = - registry.stream(testClass) - .anyMatch( - ext -> - id.getSegments().stream() - .anyMatch(s -> ext.segmentType().equals(s.getType()))); - - if (matched) { - return FilterResult.included(null); - } else { - return FilterResult.excluded("Excluding unmatched multi-env test: " + id); - } + return FilterResult.included(null); } } @Override public FilterResult apply(TestDescriptor test) { return classFor(test) - .map(testClass -> filter(testClass, test.getUniqueId())) + .map(testClass -> filter(test.getUniqueId())) .orElseGet(() -> FilterResult.included(null)); // fallback for non-class descriptors } } diff --git a/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/ThreadPerTestClassExecutionExecutorService.java b/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/ThreadPerTestClassExecutionExecutorService.java new file mode 100644 index 00000000000..810e490d53e --- /dev/null +++ b/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/ThreadPerTestClassExecutionExecutorService.java @@ -0,0 +1,117 @@ +/* + * Copyright (C) 2024 Dremio + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.projectnessie.junit.engine; + +import static java.util.concurrent.CompletableFuture.completedFuture; + +import java.lang.reflect.Field; +import java.util.List; +import java.util.concurrent.Future; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.platform.engine.TestDescriptor; +import org.junit.platform.engine.UniqueId; +import org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutorService; + +/** + * Implements a JUnit test executor that provides thread-per-test-class behavior. + * + *

"Thread-per-test-class behavior" is needed to prevent the class/class-loader leak via {@link + * ThreadLocal}s as described in #9441. + */ +public class ThreadPerTestClassExecutionExecutorService implements HierarchicalTestExecutorService { + + private static final Class CLASS_NODE_TEST_TASK; + private static final Field FIELD_TEST_DESCRIPTOR; + + static { + try { + CLASS_NODE_TEST_TASK = + Class.forName("org.junit.platform.engine.support.hierarchical.NodeTestTask"); + FIELD_TEST_DESCRIPTOR = CLASS_NODE_TEST_TASK.getDeclaredField("testDescriptor"); + FIELD_TEST_DESCRIPTOR.setAccessible(true); + } catch (Exception e) { + throw new RuntimeException( + "ThreadPerExecutionExecutorService is probably not compatible with the current JUnit version", + e); + } + } + + protected TestDescriptor getTestDescriptor(TestTask testTask) { + if (!CLASS_NODE_TEST_TASK.isAssignableFrom(testTask.getClass())) { + throw new IllegalArgumentException( + testTask.getClass().getName() + " is not of type " + CLASS_NODE_TEST_TASK.getName()); + } + try { + return (TestDescriptor) FIELD_TEST_DESCRIPTOR.get(testTask); + } catch (IllegalAccessException e) { + throw new RuntimeException(e); + } + } + + public ThreadPerTestClassExecutionExecutorService() {} + + @Override + public Future submit(TestTask testTask) { + executeTask(testTask); + return completedFuture(null); + } + + @Override + public void invokeAll(List tasks) { + tasks.forEach(this::executeTask); + } + + protected void executeTask(TestTask testTask) { + TestDescriptor testDescriptor = getTestDescriptor(testTask); + UniqueId.Segment lastSegment = testDescriptor.getUniqueId().getLastSegment(); + String type = lastSegment.getType(); + if ("class".equals(type)) { + AtomicReference failure = new AtomicReference<>(); + Thread threadPerClass = + new Thread( + () -> { + try { + testTask.execute(); + } catch (Exception e) { + failure.set(e); + } + }, + "TEST THREAD FOR " + lastSegment.getValue()); + threadPerClass.setDaemon(true); + threadPerClass.start(); + try { + threadPerClass.join(); + } catch (InterruptedException e) { + // delegate a thread-interrupt + threadPerClass.interrupt(); + } + Exception ex = failure.get(); + if (ex instanceof RuntimeException) { + throw (RuntimeException) ex; + } else if (ex != null) { + throw new RuntimeException(ex); + } + } else { + testTask.execute(); + } + } + + @Override + public void close() { + // nothing to do + } +} diff --git a/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/ThreadPerTestClassExecutionTestEngine.java b/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/ThreadPerTestClassExecutionTestEngine.java new file mode 100644 index 00000000000..db68bf09328 --- /dev/null +++ b/testing/multi-env-test-engine/src/main/java/org/projectnessie/junit/engine/ThreadPerTestClassExecutionTestEngine.java @@ -0,0 +1,85 @@ +/* + * Copyright (C) 2024 Dremio + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.projectnessie.junit.engine; + +import java.util.Optional; +import org.junit.jupiter.engine.config.CachingJupiterConfiguration; +import org.junit.jupiter.engine.config.DefaultJupiterConfiguration; +import org.junit.jupiter.engine.config.JupiterConfiguration; +import org.junit.jupiter.engine.descriptor.JupiterEngineDescriptor; +import org.junit.jupiter.engine.discovery.DiscoverySelectorResolver; +import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext; +import org.junit.jupiter.engine.support.JupiterThrowableCollectorFactory; +import org.junit.platform.engine.EngineDiscoveryRequest; +import org.junit.platform.engine.ExecutionRequest; +import org.junit.platform.engine.TestDescriptor; +import org.junit.platform.engine.UniqueId; +import org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine; +import org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutorService; +import org.junit.platform.engine.support.hierarchical.ThrowableCollector; + +public class ThreadPerTestClassExecutionTestEngine + extends HierarchicalTestEngine { + + @Override + public String getId() { + return JupiterEngineDescriptor.ENGINE_ID; + } + + /** Returns {@code org.junit.jupiter} as the group ID. */ + @Override + public Optional getGroupId() { + return Optional.of("org.junit.jupiter"); + } + + /** Returns {@code junit-jupiter-engine} as the artifact ID. */ + @Override + public Optional getArtifactId() { + return Optional.of("junit-jupiter-engine"); + } + + @Override + public TestDescriptor discover(EngineDiscoveryRequest discoveryRequest, UniqueId uniqueId) { + JupiterConfiguration configuration = + new CachingJupiterConfiguration( + new DefaultJupiterConfiguration(discoveryRequest.getConfigurationParameters())); + JupiterEngineDescriptor engineDescriptor = new JupiterEngineDescriptor(uniqueId, configuration); + new DiscoverySelectorResolver().resolveSelectors(discoveryRequest, engineDescriptor); + return engineDescriptor; + } + + @Override + protected HierarchicalTestExecutorService createExecutorService(ExecutionRequest request) { + return new ThreadPerTestClassExecutionExecutorService(); + } + + @Override + protected JupiterEngineExecutionContext createExecutionContext(ExecutionRequest request) { + return new JupiterEngineExecutionContext( + request.getEngineExecutionListener(), getJupiterConfiguration(request)); + } + + @Override + protected ThrowableCollector.Factory createThrowableCollectorFactory(ExecutionRequest request) { + return JupiterThrowableCollectorFactory::createThrowableCollector; + } + + private JupiterConfiguration getJupiterConfiguration(ExecutionRequest request) { + JupiterEngineDescriptor engineDescriptor = + (JupiterEngineDescriptor) request.getRootTestDescriptor(); + return engineDescriptor.getConfiguration(); + } +}