Skip to content

Commit 33f41e2

Browse files
jbachorikclaude
andcommitted
Fix Exec task arg handling and test filter heuristic
- Preserve build-script args in doFirst blocks (LIFO order) - Fix test filter to only use --select-method for '#' separator - Update javadoc to reference Exec tasks instead of Test/JavaExec - Simplify build.gradle.kts to use doFirst (plugin preserves args) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 8c19613 commit 33f41e2

2 files changed

Lines changed: 35 additions & 16 deletions

File tree

build-logic/conventions/src/main/kotlin/com/datadoghq/profiler/ProfilerTestPlugin.kt

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,8 @@ class ProfilerTestPlugin : Plugin<Project> {
141141
testTask.description = "Runs unit tests with the $configName library variant"
142142
testTask.group = "verification"
143143

144-
// Use JAVA_TEST_HOME if set, otherwise JAVA_HOME
145-
val javaHome = System.getenv("JAVA_TEST_HOME") ?: System.getenv("JAVA_HOME")
146-
testTask.executable = "$javaHome/bin/java"
144+
// Use JAVA_TEST_HOME if set, otherwise JAVA_HOME (with proper fallback and error handling)
145+
testTask.executable = PlatformUtils.testJavaExecutable()
147146

148147
// Dependencies
149148
testTask.dependsOn(project.tasks.named("compileTestJava"))
@@ -157,6 +156,8 @@ class ProfilerTestPlugin : Plugin<Project> {
157156

158157
// Configure JVM arguments and test execution
159158
testTask.doFirst {
159+
// Preserve any args added by build scripts (doFirst blocks run in LIFO order)
160+
val buildScriptArgs = testTask.args.toList()
160161
val allArgs = mutableListOf<String>()
161162

162163
// Standard JVM args
@@ -166,6 +167,9 @@ class ProfilerTestPlugin : Plugin<Project> {
166167
}
167168
allArgs.addAll(extension.extraJvmArgs.get())
168169

170+
// Add any args from build scripts (e.g., -Dddprof_test.* properties)
171+
allArgs.addAll(buildScriptArgs)
172+
169173
// System properties
170174
allArgs.add("-DDDPROF_TEST_DISABLE_RATE_LIMIT=1")
171175
allArgs.add("-DCI=${project.hasProperty("CI") || System.getenv("CI")?.toBoolean() ?: false}")
@@ -185,6 +189,18 @@ class ProfilerTestPlugin : Plugin<Project> {
185189
allArgs.add("--details=verbose")
186190
allArgs.add("--details-theme=unicode")
187191

192+
// Support Gradle's --tests filter by mapping to JUnit's selection options
193+
if (project.hasProperty("tests")) {
194+
val testFilter = project.property("tests") as String
195+
// Only use --select-method if filter contains '#' (method separator)
196+
// FQCNs contain '.' but should use --select-class
197+
if (testFilter.contains("#")) {
198+
allArgs.add("--select-method=$testFilter")
199+
} else {
200+
allArgs.add("--select-class=$testFilter")
201+
}
202+
}
203+
188204
testTask.args = allArgs
189205
}
190206

@@ -223,8 +239,8 @@ class ProfilerTestPlugin : Plugin<Project> {
223239
runTask.description = "Run the unwinding validator application ($configName config)"
224240
runTask.group = "application"
225241

226-
val javaHome = System.getenv("JAVA_TEST_HOME") ?: System.getenv("JAVA_HOME")
227-
runTask.executable = "$javaHome/bin/java"
242+
// Use JAVA_TEST_HOME if set, otherwise JAVA_HOME (with proper fallback and error handling)
243+
runTask.executable = PlatformUtils.testJavaExecutable()
228244

229245
runTask.dependsOn(project.tasks.named("compileJava"))
230246
runTask.dependsOn(mainCfg)
@@ -261,8 +277,8 @@ class ProfilerTestPlugin : Plugin<Project> {
261277
reportTask.description = "Generate unwinding report for CI ($configName config)"
262278
reportTask.group = "verification"
263279

264-
val javaHome = System.getenv("JAVA_TEST_HOME") ?: System.getenv("JAVA_HOME")
265-
reportTask.executable = "$javaHome/bin/java"
280+
// Use JAVA_TEST_HOME if set, otherwise JAVA_HOME (with proper fallback and error handling)
281+
reportTask.executable = PlatformUtils.testJavaExecutable()
266282

267283
reportTask.dependsOn(project.tasks.named("compileJava"))
268284
reportTask.dependsOn(mainCfg)
@@ -372,7 +388,7 @@ abstract class ProfilerTestExtension @Inject constructor(
372388
) {
373389

374390
/**
375-
* Standard JVM arguments applied to all Test and JavaExec tasks.
391+
* Standard JVM arguments applied to all Exec-based test and application tasks.
376392
* These are the common profiler testing requirements.
377393
*/
378394
abstract val standardJvmArgs: ListProperty<String>
@@ -384,7 +400,7 @@ abstract class ProfilerTestExtension @Inject constructor(
384400

385401
/**
386402
* Directory containing native test libraries.
387-
* When set, adds -Djava.library.path to Test tasks.
403+
* When set, adds -Djava.library.path to test Exec tasks.
388404
*/
389405
val nativeLibDir: org.gradle.api.file.DirectoryProperty = objects.directoryProperty()
390406

ddprof-test/build.gradle.kts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,19 +51,22 @@ dependencies {
5151
}
5252

5353
// Additional test task configuration beyond what the plugin provides
54-
tasks.withType<Test>().configureEach {
54+
// The plugin creates Exec tasks (not Test tasks) for config-specific tests
55+
tasks.withType<org.gradle.api.tasks.Exec>().matching { it.name.startsWith("test") }.configureEach {
5556
// Ensure native test library is built before running tests
5657
dependsOn(":ddprof-test-native:linkLib")
5758

58-
// Extract config name from task name for test-specific JVM args
59+
// Extract config name from task name for test-specific system properties
5960
val configName = name.replace("test", "")
6061
val keepRecordings = project.hasProperty("keepJFRs") || System.getenv("KEEP_JFRS")?.toBoolean() ?: false
6162

62-
jvmArgs(
63-
"-Dddprof_test.keep_jfrs=$keepRecordings",
64-
"-Dddprof_test.config=$configName",
65-
"-Dddprof_test.ci=${project.hasProperty("CI")}",
66-
)
63+
// Pass test configuration as system properties
64+
// The plugin's doFirst preserves args added here (doFirst blocks run in LIFO order)
65+
doFirst {
66+
args("-Dddprof_test.keep_jfrs=$keepRecordings")
67+
args("-Dddprof_test.config=$configName")
68+
args("-Dddprof_test.ci=${project.hasProperty("CI")}")
69+
}
6770
}
6871

6972
// Disable the default 'test' task - we use config-specific tasks instead

0 commit comments

Comments
 (0)