Skip to content

Commit 9e673a6

Browse files
daniel-mohedanodaniel-mohedano
authored
Avoid final field modifications in Cucumber and Spock instrumentations (#11851)
feat: avoid final field modifications in junit instrumentation fix: cucumber retries running through `Suite` feat: add retry descriptor factory for junit5-based frameworks Merge branch 'master' into daniel.mohedano/jep-500-cucumber-spock Co-authored-by: daniel.mohedano <daniel.mohedano@datadoghq.com>
1 parent 8685703 commit 9e673a6

23 files changed

Lines changed: 347 additions & 9 deletions

File tree

‎dd-java-agent/instrumentation/junit/junit-5/junit-5-cucumber-5.4/build.gradle‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ muzzle {
1313
}
1414
}
1515

16+
addTestSuiteForDir('cucumber723Test', 'test')
17+
addTestSuiteForDir('cucumber76Test', 'test')
1618
addTestSuiteForDir('latestDepTest', 'test')
1719

1820
dependencies {
@@ -33,9 +35,15 @@ dependencies {
3335

3436
latestDepTestImplementation group: 'io.cucumber', name: 'cucumber-java', version: '+'
3537
latestDepTestImplementation group: 'io.cucumber', name: 'cucumber-junit-platform-engine', version: '+'
38+
39+
cucumber76TestImplementation group: 'io.cucumber', name: 'cucumber-java', version: '7.6.0'
40+
cucumber76TestImplementation group: 'io.cucumber', name: 'cucumber-junit-platform-engine', version: '7.6.0'
41+
42+
cucumber723TestImplementation group: 'io.cucumber', name: 'cucumber-java', version: '7.23.0'
43+
cucumber723TestImplementation group: 'io.cucumber', name: 'cucumber-junit-platform-engine', version: '7.23.0'
3644
}
3745

38-
configurations.matching({ it.name.startsWith('test') }).configureEach({
46+
configurations.matching({ it.name.startsWith('test') || it.name.startsWith('cucumber') }).configureEach({
3947
it.resolutionStrategy {
4048
force group: 'org.junit.platform', name: 'junit-platform-launcher', version: libs.versions.junit.platform.get()
4149
force group: 'org.junit.platform', name: 'junit-platform-suite', version: libs.versions.junit.platform.get()

‎dd-java-agent/instrumentation/junit/junit-5/junit-5-cucumber-5.4/src/main/java/datadog/trace/instrumentation/junit5/CucumberRetryDescriptorFactory.java‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
package datadog.trace.instrumentation.junit5;
2+
3+
import datadog.trace.instrumentation.junit5.execution.RetryDescriptorFactory;
4+
import datadog.trace.util.MethodHandles;
5+
import io.cucumber.core.gherkin.Pickle;
6+
import java.lang.invoke.MethodHandle;
7+
import java.util.function.UnaryOperator;
8+
import org.junit.platform.commons.util.ClassLoaderUtils;
9+
import org.junit.platform.engine.ConfigurationParameters;
10+
import org.junit.platform.engine.TestDescriptor;
11+
import org.junit.platform.engine.TestSource;
12+
import org.junit.platform.engine.UniqueId;
13+
14+
/**
15+
* Reconstructs the Cucumber retry descriptor ({@code PickleDescriptor}) through its own constructor
16+
* with a transformed unique id to avoid final-field mutations (JEP 500).
17+
*/
18+
public final class CucumberRetryDescriptorFactory implements RetryDescriptorFactory {
19+
20+
private static final MethodHandles METHOD_HANDLES =
21+
new MethodHandles(ClassLoaderUtils.getDefaultClassLoader());
22+
23+
private static final String PACKAGE = "io.cucumber.junit.platform.engine.";
24+
25+
private static final MethodHandle CONSTRUCTOR_7_24 =
26+
METHOD_HANDLES.constructor(
27+
PACKAGE + "CucumberTestDescriptor$PickleDescriptor",
28+
JUnitPlatformUtils.loadClass(PACKAGE + "CucumberConfiguration"),
29+
UniqueId.class,
30+
String.class,
31+
TestSource.class,
32+
Pickle.class);
33+
private static final MethodHandle CONSTRUCTOR_7_7 =
34+
METHOD_HANDLES.constructor(
35+
PACKAGE + "NodeDescriptor$PickleDescriptor",
36+
ConfigurationParameters.class,
37+
UniqueId.class,
38+
String.class,
39+
TestSource.class,
40+
Pickle.class);
41+
private static final MethodHandle CONSTRUCTOR_6_0 =
42+
METHOD_HANDLES.constructor(
43+
PACKAGE + "PickleDescriptor",
44+
ConfigurationParameters.class,
45+
UniqueId.class,
46+
String.class,
47+
TestSource.class,
48+
Pickle.class);
49+
private static final MethodHandle CONSTRUCTOR_5_4 =
50+
METHOD_HANDLES.constructor(
51+
PACKAGE + "PickleDescriptor",
52+
UniqueId.class,
53+
String.class,
54+
TestSource.class,
55+
Pickle.class);
56+
57+
// 7.24+ stores the configuration on the descriptor, read it back for the reconstruction.
58+
private static final MethodHandle CONFIGURATION_GETTER =
59+
METHOD_HANDLES.privateFieldGetter(
60+
PACKAGE + "CucumberTestDescriptor$PickleDescriptor", "configuration");
61+
62+
// The Pickle field was renamed pickleEvent -> pickle; resolved lazily off the descriptor's class.
63+
private volatile MethodHandle pickleGetter;
64+
65+
@Override
66+
public TestDescriptor copy(TestDescriptor original, UnaryOperator<UniqueId> idTransform) {
67+
if (!"PickleDescriptor".equals(original.getClass().getSimpleName())) {
68+
return null; // only the leaf scenario descriptor is retried; containers are filtered earlier
69+
}
70+
Object pickle = readPickle(original);
71+
if (pickle == null) {
72+
return null;
73+
}
74+
UniqueId newId = idTransform.apply(original.getUniqueId());
75+
String name = original.getDisplayName();
76+
TestSource source = original.getSource().orElse(null);
77+
78+
if (CONSTRUCTOR_7_24 != null) {
79+
Object configuration = METHOD_HANDLES.invoke(CONFIGURATION_GETTER, original);
80+
return configuration == null
81+
? null
82+
: METHOD_HANDLES.invoke(CONSTRUCTOR_7_24, configuration, newId, name, source, pickle);
83+
}
84+
if (CONSTRUCTOR_7_7 != null) {
85+
return METHOD_HANDLES.invoke(
86+
CONSTRUCTOR_7_7, new EmptyConfigurationParameters(), newId, name, source, pickle);
87+
}
88+
if (CONSTRUCTOR_6_0 != null) {
89+
return METHOD_HANDLES.invoke(
90+
CONSTRUCTOR_6_0, new EmptyConfigurationParameters(), newId, name, source, pickle);
91+
}
92+
if (CONSTRUCTOR_5_4 != null) {
93+
return METHOD_HANDLES.invoke(CONSTRUCTOR_5_4, newId, name, source, pickle);
94+
}
95+
return null; // unknown cucumber version -> fall back to the generic clone
96+
}
97+
98+
private Object readPickle(TestDescriptor descriptor) {
99+
MethodHandle getter = pickleGetter;
100+
if (getter == null) {
101+
getter = METHOD_HANDLES.privateFieldGetter(descriptor.getClass(), "pickle");
102+
if (getter == null) {
103+
getter = METHOD_HANDLES.privateFieldGetter(descriptor.getClass(), "pickleEvent");
104+
}
105+
pickleGetter = getter;
106+
}
107+
return getter != null ? METHOD_HANDLES.invoke(getter, descriptor) : null;
108+
}
109+
}

‎dd-java-agent/instrumentation/junit/junit-5/junit-5-cucumber-5.4/src/main/java/datadog/trace/instrumentation/junit5/CucumberUtils.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import datadog.trace.api.Pair;
44
import datadog.trace.api.civisibility.config.TestIdentifier;
55
import datadog.trace.api.civisibility.config.TestSourceData;
6+
import datadog.trace.instrumentation.junit5.execution.RetryDescriptorFactories;
67
import java.io.InputStream;
78
import java.util.ArrayDeque;
89
import java.util.Deque;
@@ -25,6 +26,8 @@ public abstract class CucumberUtils {
2526
CucumberUtils::toTestIdentifier,
2627
d -> TestSourceData.UNKNOWN,
2728
null);
29+
RetryDescriptorFactories.register(
30+
JUnitPlatformUtils.ENGINE_ID_CUCUMBER, new CucumberRetryDescriptorFactory());
2831
}
2932

3033
public static @Nullable String getCucumberVersion(TestEngine cucumberEngine) {

‎dd-java-agent/instrumentation/junit/junit-5/junit-5-cucumber-5.4/src/main/java/datadog/trace/instrumentation/junit5/EmptyConfigurationParameters.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package datadog.trace.instrumentation.junit5;
2+
3+
import java.util.Collections;
4+
import java.util.Optional;
5+
import java.util.Set;
6+
import org.junit.platform.engine.ConfigurationParameters;
7+
8+
/**
9+
* NO-OP {@link ConfigurationParameters}, used when reconstructing a Cucumber retry descriptor for
10+
* engine versions (6.0–7.23) whose {@code PickleDescriptor} constructor consumes the configuration
11+
* to compute exclusive resources but does not store it (so it cannot be read back).
12+
*/
13+
@SuppressWarnings("deprecation") // ConfigurationParameters#size() is deprecated in newer platforms
14+
public final class EmptyConfigurationParameters implements ConfigurationParameters {
15+
16+
@Override
17+
public Optional<String> get(String key) {
18+
return Optional.empty();
19+
}
20+
21+
@Override
22+
public Optional<Boolean> getBoolean(String key) {
23+
return Optional.empty();
24+
}
25+
26+
@Override
27+
public int size() {
28+
return 0;
29+
}
30+
31+
public Set<String> keySet() {
32+
return Collections.emptySet();
33+
}
34+
}

‎dd-java-agent/instrumentation/junit/junit-5/junit-5-cucumber-5.4/src/main/java/datadog/trace/instrumentation/junit5/JUnit5CucumberInstrumentation.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ public String[] helperClassNames() {
3939
return new String[] {
4040
packageName + ".TestDataFactory",
4141
packageName + ".JUnitPlatformUtils",
42+
packageName + ".execution.RetryDescriptorFactory",
43+
packageName + ".execution.RetryDescriptorFactories",
44+
packageName + ".EmptyConfigurationParameters",
45+
packageName + ".CucumberRetryDescriptorFactory",
4246
packageName + ".CucumberUtils",
4347
packageName + ".TestEventsHandlerHolder",
4448
packageName + ".CucumberTracingListener",

‎dd-java-agent/instrumentation/junit/junit-5/junit-5-cucumber-5.4/src/main/java/datadog/trace/instrumentation/junit5/JUnit5CucumberSkipInstrumentation.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ public String[] helperClassNames() {
6565
return new String[] {
6666
packageName + ".TestDataFactory",
6767
packageName + ".JUnitPlatformUtils",
68+
packageName + ".execution.RetryDescriptorFactory",
69+
packageName + ".execution.RetryDescriptorFactories",
70+
packageName + ".EmptyConfigurationParameters",
71+
packageName + ".CucumberRetryDescriptorFactory",
6872
packageName + ".CucumberUtils",
6973
packageName + ".TestEventsHandlerHolder",
7074
};

‎dd-java-agent/instrumentation/junit/junit-5/junit-5-cucumber-5.4/src/test/groovy/CucumberTest.groovy‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import datadog.trace.api.civisibility.config.TestIdentifier
44
import datadog.trace.civisibility.CiVisibilityInstrumentationTest
55
import datadog.trace.instrumentation.junit5.JUnitPlatformUtils
66
import datadog.trace.instrumentation.junit5.TestEventsHandlerHolder
7+
import datadog.trace.util.ComparableVersion
78
import io.cucumber.core.api.TypeRegistry
89
import io.cucumber.core.options.Constants
910
import org.junit.platform.engine.DiscoverySelector
@@ -31,10 +32,10 @@ class CucumberTest extends CiVisibilityInstrumentationTest {
3132
where:
3233
testcaseName | features | parallel
3334
"test-succeed" | ["org/example/cucumber/calculator/basic_arithmetic.feature"] | false
34-
"test-scenario-outline-${version()}" | ["org/example/cucumber/calculator/basic_arithmetic_with_examples.feature"] | false
35+
"test-scenario-outline-${fixtureVersion()}" | ["org/example/cucumber/calculator/basic_arithmetic_with_examples.feature"] | false
3536
"test-skipped" | ["org/example/cucumber/calculator/basic_arithmetic_skipped.feature"] | false
3637
"test-skipped-feature" | ["org/example/cucumber/calculator/basic_arithmetic_skipped_feature.feature"] | false
37-
"test-skipped-scenario-outline-${version()}" | ["org/example/cucumber/calculator/basic_arithmetic_with_examples_skipped.feature"] | false
38+
"test-skipped-scenario-outline-${fixtureVersion()}" | ["org/example/cucumber/calculator/basic_arithmetic_with_examples_skipped.feature"] | false
3839
"test-parallel" | [
3940
"org/example/cucumber/calculator/basic_arithmetic.feature",
4041
"org/example/cucumber/calculator/basic_arithmetic_skipped.feature"
@@ -78,7 +79,7 @@ class CucumberTest extends CiVisibilityInstrumentationTest {
7879
"test-failed-then-succeed" | true | ["org/example/cucumber/calculator/basic_arithmetic_failed_then_succeed.feature"] | [
7980
new TestFQN("classpath:org/example/cucumber/calculator/basic_arithmetic_failed_then_succeed.feature:Basic Arithmetic", "Addition")
8081
]
81-
"test-retry-failed-scenario-outline-${version()}" | false | ["org/example/cucumber/calculator/basic_arithmetic_with_failed_examples.feature"] | [
82+
"test-retry-failed-scenario-outline-${fixtureVersion()}" | false | ["org/example/cucumber/calculator/basic_arithmetic_with_failed_examples.feature"] | [
8283
new TestFQN("classpath:org/example/cucumber/calculator/basic_arithmetic_with_failed_examples.feature:Basic Arithmetic With Examples", "Many additions.Single digits.${parameterizedTestNameSuffix()}")
8384
]
8485
}
@@ -97,7 +98,7 @@ class CucumberTest extends CiVisibilityInstrumentationTest {
9798
new TestFQN("classpath:org/example/cucumber/calculator/basic_arithmetic.feature:Basic Arithmetic", "Addition")
9899
]
99100
"test-efd-new-test" | ["org/example/cucumber/calculator/basic_arithmetic.feature"] | []
100-
"test-efd-new-scenario-outline-${version()}" | ["org/example/cucumber/calculator/basic_arithmetic_with_examples.feature"] | []
101+
"test-efd-new-scenario-outline-${fixtureVersion()}" | ["org/example/cucumber/calculator/basic_arithmetic_with_examples.feature"] | []
101102
"test-efd-new-slow-test" | ["org/example/cucumber/calculator/basic_arithmetic_slow.feature"] | []
102103
"test-efd-skip-new-test" | ["org/example/cucumber/calculator/basic_arithmetic_skip_efd.feature"] | []
103104
}
@@ -221,13 +222,22 @@ class CucumberTest extends CiVisibilityInstrumentationTest {
221222
}
222223

223224
private String parameterizedTestNameSuffix() {
224-
// older releases report different example names
225-
version() == "5.4.0" ? "Example #1" : "Example #1.1"
225+
// Cucumber 7.11.0 changed scenario-outline example naming from the flat "Example #<row>"
226+
// to "Example #<examplesBlock>.<row>".
227+
usesFlatExampleNaming() ? "Example #1" : "Example #1.1"
226228
}
227229

228-
private String version() {
230+
private String fixtureVersion() {
231+
// Scenario-outline fixtures only differ by the example naming scheme, so every release that
232+
// still uses the flat naming shares the "legacy" fixture bucket; the rest use "latest".
233+
usesFlatExampleNaming() ? "legacy" : "latest"
234+
}
235+
236+
private boolean usesFlatExampleNaming() {
229237
def version = TypeRegistry.package.getImplementationVersion()
230-
return version != null ? "latest" : "5.4.0" // older releases do not have package version populated
238+
// 5.4.0 does not populate the package version; cucumber 7.11.0 switched from the flat
239+
// "Example #<row>" naming to "Example #<examplesBlock>.<row>".
240+
return version == null || new ComparableVersion(version) < new ComparableVersion("7.11.0")
231241
}
232242

233243
protected void runFeatures(List<String> classpathFeatures, boolean parallel, boolean expectSuccess = true) {

dd-java-agent/instrumentation/junit/junit-5/junit-5-cucumber-5.4/src/test/resources/test-efd-new-scenario-outline-5.4.0/coverages.ftl renamed to dd-java-agent/instrumentation/junit/junit-5/junit-5-cucumber-5.4/src/test/resources/test-efd-new-scenario-outline-legacy/coverages.ftl

File renamed without changes.

dd-java-agent/instrumentation/junit/junit-5/junit-5-cucumber-5.4/src/test/resources/test-efd-new-scenario-outline-5.4.0/events.ftl renamed to dd-java-agent/instrumentation/junit/junit-5/junit-5-cucumber-5.4/src/test/resources/test-efd-new-scenario-outline-legacy/events.ftl

File renamed without changes.

dd-java-agent/instrumentation/junit/junit-5/junit-5-cucumber-5.4/src/test/resources/test-retry-failed-scenario-outline-5.4.0/coverages.ftl renamed to dd-java-agent/instrumentation/junit/junit-5/junit-5-cucumber-5.4/src/test/resources/test-retry-failed-scenario-outline-legacy/coverages.ftl

File renamed without changes.

0 commit comments

Comments
 (0)