diff --git a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/JUnitPlatformUtils.java b/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/JUnitPlatformUtils.java index ac7849dce81..e3bd1d58eca 100644 --- a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/JUnitPlatformUtils.java +++ b/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/JUnitPlatformUtils.java @@ -99,6 +99,18 @@ private JUnitPlatformUtils() {} private static final MethodHandles METHOD_HANDLES = new MethodHandles(ClassLoaderUtils.getDefaultClassLoader()); + /** + * Loads a class by name from the default class loader, returning {@code null} if it is absent. + */ + @Nullable + public static Class loadClass(String className) { + try { + return ClassLoaderUtils.getDefaultClassLoader().loadClass(className); + } catch (Throwable t) { + return null; + } + } + /* * We have to support older versions of JUnit 5 that do not have certain methods that we would * like to use. We try to get method handles in runtime, and if we fail to do it there's a diff --git a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/execution/JUnit5ExecutionInstrumentation.java b/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/execution/JUnit5ExecutionInstrumentation.java index 83c29a5306a..e977d70628d 100644 --- a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/execution/JUnit5ExecutionInstrumentation.java +++ b/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/execution/JUnit5ExecutionInstrumentation.java @@ -30,7 +30,6 @@ import org.junit.platform.engine.TestDescriptor; import org.junit.platform.engine.support.hierarchical.EngineExecutionContext; import org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutorService; -import org.junit.platform.engine.support.hierarchical.Node; import org.junit.platform.engine.support.hierarchical.ThrowableCollector; @AutoService(InstrumenterModule.class) @@ -161,12 +160,13 @@ public static Boolean execute(@Advice.This HierarchicalTestExecutorService.TestT EngineExecutionContext parentContext = taskHandle.getParentContext(); TestDescriptorHandle descriptorHandle = new TestDescriptorHandle(testDescriptor); + HierarchicalTestExecutorService.TestTask currentTask = testTask; int retryAttemptIdx = 0; while (true) { factory.setSuppressFailures(executionPolicy.suppressFailures()); CallDepthThreadLocalMap.incrementCallDepth(HierarchicalTestExecutorService.TestTask.class); - testTask.execute(); + currentTask.execute(); CallDepthThreadLocalMap.decrementCallDepth(HierarchicalTestExecutorService.TestTask.class); factory.setSuppressFailures(false); // restore default behavior @@ -185,13 +185,12 @@ public static Boolean execute(@Advice.This HierarchicalTestExecutorService.TestT JUnitPlatformUtils.RETRY_DESCRIPTOR_ID_SUFFIX, String.valueOf(++retryAttemptIdx)); TestDescriptor retryDescriptor = descriptorHandle.withIdSuffix(suffix); - taskHandle.setTestDescriptor(retryDescriptor); - taskHandle.setNode((Node) retryDescriptor); taskHandle.getListener().dynamicTestRegistered(retryDescriptor); TestEventsHandlerHolder.setExecutionTracker(retryDescriptor, executionPolicy); - // restore parent context, since the reference is overwritten with null after execution - taskHandle.setParentContext(parentContext); + // build a fresh task for the retry and reuse the original parent context, since execution + // overwrites it with null + currentTask = taskHandle.createRetryTask(retryDescriptor, parentContext); } return Boolean.TRUE; // skip original method execution } diff --git a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/execution/TestDescriptorHandle.java b/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/execution/TestDescriptorHandle.java index b89fc1113e4..182171776ac 100644 --- a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/execution/TestDescriptorHandle.java +++ b/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/execution/TestDescriptorHandle.java @@ -1,12 +1,14 @@ package datadog.trace.instrumentation.junit5.execution; import datadog.trace.agent.tooling.muzzle.Reference; +import datadog.trace.instrumentation.junit5.JUnitPlatformUtils; import datadog.trace.util.MethodHandles; import datadog.trace.util.UnsafeUtils; import java.lang.invoke.MethodHandle; import java.util.Collection; import java.util.Collections; import java.util.Map; +import java.util.function.UnaryOperator; import org.junit.platform.commons.util.ClassLoaderUtils; import org.junit.platform.engine.TestDescriptor; import org.junit.platform.engine.UniqueId; @@ -17,8 +19,28 @@ public class TestDescriptorHandle { private static final MethodHandles METHOD_HANDLES = new MethodHandles(ClassLoaderUtils.getDefaultClassLoader()); - private static final MethodHandle UNIQUE_ID_SETTER = - METHOD_HANDLES.privateFieldSetter(AbstractTestDescriptor.class, "uniqueId"); + private static final String JUPITER_TEST_DESCRIPTOR = + "org.junit.jupiter.engine.descriptor.JupiterTestDescriptor"; + private static final Class JUPITER_TEST_DESCRIPTOR_CLASS = + JUnitPlatformUtils.loadClass(JUPITER_TEST_DESCRIPTOR); + + /** {@code JupiterTestDescriptor#copyIncludingDescendants(UnaryOperator)} (5.13+) */ + private static final MethodHandle COPY_INCLUDING_DESCENDANTS = + METHOD_HANDLES.method( + JUPITER_TEST_DESCRIPTOR, "copyIncludingDescendants", UnaryOperator.class); + + // Legacy fallback used when copyIncludingDescendants is unavailable. + // Overwrites the final unique ID field by reflection. Lazily created to avoid JEP 500 warnings. + private static volatile MethodHandle uniqueIdSetter; + + private static MethodHandle uniqueIdSetter() { + MethodHandle handle = uniqueIdSetter; + if (handle == null) { + handle = METHOD_HANDLES.privateFieldSetter(AbstractTestDescriptor.class, "uniqueId"); + uniqueIdSetter = handle; + } + return handle; + } public static final class MuzzleHelper { public static Collection compileReferences() { @@ -33,24 +55,57 @@ public static Collection compileReferences() { public TestDescriptorHandle(TestDescriptor testDescriptor) { /* - * We're cloning the descriptor to preserve its original state: + * We're copying the descriptor to preserve its original state: * JUnit will modify some of its fields during and after test execution * (one example is parameterized test descriptor, * whose invocation context is overwritten with null). - * Cloning has to be done before each test retry to - * compensate for the state modifications. + * The snapshot is taken before the first execution so that every retry + * can be derived from the pristine state. */ - this.testDescriptor = UnsafeUtils.tryShallowClone(testDescriptor); + this.testDescriptor = copy(testDescriptor, UnaryOperator.identity()); } public TestDescriptor withIdSuffix(Map suffices) { - UniqueId updatedId = testDescriptor.getUniqueId(); - for (Map.Entry e : suffices.entrySet()) { - updatedId = updatedId.append(e.getKey(), String.valueOf(e.getValue())); + return copy( + testDescriptor, + id -> { + UniqueId updatedId = id; + for (Map.Entry e : suffices.entrySet()) { + updatedId = updatedId.append(e.getKey(), String.valueOf(e.getValue())); + } + return updatedId; + }); + } + + private static TestDescriptor copy( + TestDescriptor testDescriptor, UnaryOperator idTransform) { + if (COPY_INCLUDING_DESCENDANTS != null + && JUPITER_TEST_DESCRIPTOR_CLASS != null + && JUPITER_TEST_DESCRIPTOR_CLASS.isInstance(testDescriptor)) { + TestDescriptor copy = + METHOD_HANDLES.invoke(COPY_INCLUDING_DESCENDANTS, testDescriptor, idTransform); + if (copy != null) { + // copyIncludingDescendants returns a detached copy so we link it back to its suite + if (copy instanceof AbstractTestDescriptor) { + ((AbstractTestDescriptor) copy).setParent(testDescriptor.getParent().orElse(null)); + } + return copy; + } } + return legacyCopy(testDescriptor, idTransform); + } + /** + * Fallback for engines without {@code copyIncludingDescendants}: shallow-clone the descriptor and + * overwrite the cloned unique ID field by reflection. Not JEP 500 compliant. + */ + private static TestDescriptor legacyCopy( + TestDescriptor testDescriptor, UnaryOperator idTransform) { TestDescriptor descriptorClone = UnsafeUtils.tryShallowClone(testDescriptor); - METHOD_HANDLES.invoke(UNIQUE_ID_SETTER, descriptorClone, updatedId); + UniqueId updatedId = idTransform.apply(testDescriptor.getUniqueId()); + if (descriptorClone != testDescriptor && !updatedId.equals(testDescriptor.getUniqueId())) { + METHOD_HANDLES.invoke(uniqueIdSetter(), descriptorClone, updatedId); + } return descriptorClone; } } diff --git a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/execution/TestTaskHandle.java b/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/execution/TestTaskHandle.java index af50a2aef5b..adf7c364ea5 100644 --- a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/execution/TestTaskHandle.java +++ b/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/execution/TestTaskHandle.java @@ -2,6 +2,7 @@ import datadog.trace.agent.tooling.muzzle.Reference; import datadog.trace.agent.tooling.muzzle.ReferenceProvider; +import datadog.trace.instrumentation.junit5.JUnitPlatformUtils; import datadog.trace.util.MethodHandles; import java.lang.invoke.MethodHandle; import java.util.Arrays; @@ -13,7 +14,6 @@ import org.junit.platform.engine.TestDescriptor; import org.junit.platform.engine.support.hierarchical.EngineExecutionContext; import org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutorService; -import org.junit.platform.engine.support.hierarchical.Node; import org.junit.platform.engine.support.hierarchical.ThrowableCollector; public class TestTaskHandle { @@ -28,17 +28,44 @@ public class TestTaskHandle { private static final MethodHandle TEST_DESCRIPTOR_GETTER = METHOD_HANDLES.privateFieldGetter(TEST_TASK_CLASS, "testDescriptor"); - private static final MethodHandle TEST_DESCRIPTOR_SETTER = - METHOD_HANDLES.privateFieldSetter(TEST_TASK_CLASS, "testDescriptor"); - - private static final MethodHandle NODE_SETTER = - METHOD_HANDLES.privateFieldSetter(TEST_TASK_CLASS, "node"); private static final MethodHandle PARENT_CONTEXT_GETTER = METHOD_HANDLES.privateFieldGetter(TEST_TASK_CLASS, "parentContext"); private static final MethodHandle PARENT_CONTEXT_SETTER = METHOD_HANDLES.privateFieldSetter(TEST_TASK_CLASS, "parentContext"); + /** NodeTestTask's {@code (NodeTestTaskContext, TestDescriptor)} constructor (1.3.1+) */ + private static final Class TEST_TASK_CONTEXT_CLASS_REF = + JUnitPlatformUtils.loadClass(TEST_TASK_CONTEXT_CLASS); + + private static final MethodHandle TEST_TASK_CONSTRUCTOR = + TEST_TASK_CONTEXT_CLASS_REF != null + ? METHOD_HANDLES.constructor( + TEST_TASK_CLASS, TEST_TASK_CONTEXT_CLASS_REF, TestDescriptor.class) + : null; + + // Legacy fallback setters, lazily created to avoid JEP 500 warnings, only used on 1.3.0 + private static volatile MethodHandle testDescriptorSetter; + private static volatile MethodHandle nodeSetter; + + private static MethodHandle testDescriptorSetter() { + MethodHandle handle = testDescriptorSetter; + if (handle == null) { + handle = METHOD_HANDLES.privateFieldSetter(TEST_TASK_CLASS, "testDescriptor"); + testDescriptorSetter = handle; + } + return handle; + } + + private static MethodHandle nodeSetter() { + MethodHandle handle = nodeSetter; + if (handle == null) { + handle = METHOD_HANDLES.privateFieldSetter(TEST_TASK_CLASS, "node"); + nodeSetter = handle; + } + return handle; + } + private static final MethodHandle THROWABLE_COLLECTOR_FACTORY_GETTER = METHOD_HANDLES.privateFieldGetter(TEST_TASK_CLASS, "throwableCollectorFactory"); private static final MethodHandle TASK_CONTEXT_THROWABLE_COLLECTOR_FACTORY_GETTER = @@ -118,20 +145,29 @@ public TestDescriptor getTestDescriptor() { return METHOD_HANDLES.invoke(TEST_DESCRIPTOR_GETTER, testTask); } - public void setTestDescriptor(TestDescriptor testDescriptor) { - METHOD_HANDLES.invoke(TEST_DESCRIPTOR_SETTER, testTask, testDescriptor); - } - - public void setNode(Node node) { - METHOD_HANDLES.invoke(NODE_SETTER, testTask, node); - } - public EngineExecutionContext getParentContext() { return METHOD_HANDLES.invoke(PARENT_CONTEXT_GETTER, testTask); } - public void setParentContext(EngineExecutionContext parentContext) { + /** + * Returns a task that will execute the given retry descriptor. If possible, a brand-new + * NodeTestTask is constructed; otherwise we fall back to overwriting the current task's fields + * (non-compliant with JEP500). + */ + public HierarchicalTestExecutorService.TestTask createRetryTask( + TestDescriptor descriptor, EngineExecutionContext parentContext) { + if (TEST_TASK_CONSTRUCTOR != null && testTaskContext != null) { + Object retryTask = METHOD_HANDLES.invoke(TEST_TASK_CONSTRUCTOR, testTaskContext, descriptor); + if (retryTask != null) { + METHOD_HANDLES.invoke(PARENT_CONTEXT_SETTER, retryTask, parentContext); + return (HierarchicalTestExecutorService.TestTask) retryTask; + } + } + // fallback (< 1.3.1): reuse the current task by overwriting its final fields. + METHOD_HANDLES.invoke(testDescriptorSetter(), testTask, descriptor); + METHOD_HANDLES.invoke(nodeSetter(), testTask, descriptor); METHOD_HANDLES.invoke(PARENT_CONTEXT_SETTER, testTask, parentContext); + return testTask; } public EngineExecutionListener getListener() {