From 41002d918d52c6f9c208f92ed1c3e24dc9b966ad Mon Sep 17 00:00:00 2001 From: Brice Dutheil Date: Fri, 20 Mar 2026 13:40:25 +0100 Subject: [PATCH 1/3] fix: Moved Gradle TestKit directory to separate OS temp directory In 748292c43c17dad22a78edd508773ecdd2f2bea7 the GradleRuner used the temporary directory of the current test (projectDir) which comes from JUnit's `@TempDir`. When JUnit finishes its test it tries to cleanupt the temporary folder, however the daemon may still be running and lock files under this folder. With the change each fixture still gets its own isolated testkit dir to ensure the daemon isolation preserved. https://github.com/gradle/gradle/issues/12535 --- .../datadog/gradle/plugin/GradleFixture.kt | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/buildSrc/src/test/kotlin/datadog/gradle/plugin/GradleFixture.kt b/buildSrc/src/test/kotlin/datadog/gradle/plugin/GradleFixture.kt index 9401f399170..d12b0ae501e 100644 --- a/buildSrc/src/test/kotlin/datadog/gradle/plugin/GradleFixture.kt +++ b/buildSrc/src/test/kotlin/datadog/gradle/plugin/GradleFixture.kt @@ -6,6 +6,7 @@ import org.gradle.testkit.runner.UnexpectedBuildResultException import org.intellij.lang.annotations.Language import org.w3c.dom.Document import java.io.File +import java.nio.file.Files import javax.xml.parsers.DocumentBuilderFactory /** @@ -13,6 +14,15 @@ import javax.xml.parsers.DocumentBuilderFactory * Provides common functionality for setting up test projects and running Gradle builds. */ internal open class GradleFixture(protected val projectDir: File) { + // Keep test-kit caches outside @TempDir so Gradle daemon file-locks on + // .testkit/caches/*/transforms don't cause JUnit @TempDir cleanup failures + // (DirectoryNotEmptyException). Each fixture still gets its own testkit dir, + // ensuring daemon isolation (fresh MAVEN_REPOSITORY_PROXY per test). + // Cleanup is left to the OS (/tmp is ephemeral on CI containers). + private val testKitDir: File by lazy { + Files.createTempDirectory("gradle-testkit-").toFile() + } + /** * Runs Gradle with the specified arguments. * @@ -23,12 +33,7 @@ internal open class GradleFixture(protected val projectDir: File) { */ fun run(vararg args: String, expectFailure: Boolean = false, env: Map = emptyMap()): BuildResult { val runner = GradleRunner.create() - // Use a testkit dir scoped to this fixture's projectDir. The Tooling API always uses a - // daemon and ignores org.gradle.daemon=false. By giving each test its own testkit dir, - // we force a fresh daemon per test — ensuring withEnvironment() vars (e.g. - // MAVEN_REPOSITORY_PROXY) are correctly set on the daemon JVM and not inherited from - // a previously-started daemon with a different test's environment. - .withTestKitDir(file(".testkit")) + .withTestKitDir(testKitDir) .withPluginClasspath() .withProjectDir(projectDir) .withEnvironment(System.getenv() + env) From 7578a30338bb29721ba46c5ea62c7b73d2c70e16 Mon Sep 17 00:00:00 2001 From: Brice Dutheil Date: Fri, 20 Mar 2026 15:50:12 +0100 Subject: [PATCH 2/3] fix: Improve Gradle daemon isolation and proper gradle daemon shutdown --- .../datadog/gradle/plugin/GradleFixture.kt | 75 ++++++++++++++++--- 1 file changed, 66 insertions(+), 9 deletions(-) diff --git a/buildSrc/src/test/kotlin/datadog/gradle/plugin/GradleFixture.kt b/buildSrc/src/test/kotlin/datadog/gradle/plugin/GradleFixture.kt index d12b0ae501e..f74f70dafb6 100644 --- a/buildSrc/src/test/kotlin/datadog/gradle/plugin/GradleFixture.kt +++ b/buildSrc/src/test/kotlin/datadog/gradle/plugin/GradleFixture.kt @@ -6,7 +6,6 @@ import org.gradle.testkit.runner.UnexpectedBuildResultException import org.intellij.lang.annotations.Language import org.w3c.dom.Document import java.io.File -import java.nio.file.Files import javax.xml.parsers.DocumentBuilderFactory /** @@ -14,18 +13,19 @@ import javax.xml.parsers.DocumentBuilderFactory * Provides common functionality for setting up test projects and running Gradle builds. */ internal open class GradleFixture(protected val projectDir: File) { - // Keep test-kit caches outside @TempDir so Gradle daemon file-locks on - // .testkit/caches/*/transforms don't cause JUnit @TempDir cleanup failures - // (DirectoryNotEmptyException). Each fixture still gets its own testkit dir, - // ensuring daemon isolation (fresh MAVEN_REPOSITORY_PROXY per test). - // Cleanup is left to the OS (/tmp is ephemeral on CI containers). - private val testKitDir: File by lazy { - Files.createTempDirectory("gradle-testkit-").toFile() - } + // Each fixture gets its own testkit dir so that a fresh daemon is started per + // test — ensuring withEnvironment() vars (e.g. MAVEN_REPOSITORY_PROXY) are + // correctly set on the daemon JVM and not inherited from a previously-started + // daemon with a different test's environment. + private val testKitDir: File by lazy { file(".testkit") } /** * Runs Gradle with the specified arguments. * + * After the build completes, any Gradle daemons started by TestKit are killed + * so their file locks on the testkit cache are released before JUnit `@TempDir` + * cleanup. See https://github.com/gradle/gradle/issues/12535 + * * @param args Gradle task names and arguments * @param expectFailure Whether the build is expected to fail * @param env Environment variables to set (merged with system environment) @@ -36,15 +36,72 @@ internal open class GradleFixture(protected val projectDir: File) { .withTestKitDir(testKitDir) .withPluginClasspath() .withProjectDir(projectDir) + // Using withDebug prevents starting a daemon, but it doesn't work with withEnvironment .withEnvironment(System.getenv() + env) .withArguments(*args) return try { if (expectFailure) runner.buildAndFail() else runner.build() } catch (e: UnexpectedBuildResultException) { e.buildResult + } finally { + stopDaemons() } } + /** + * Kills Gradle daemons started by TestKit for this fixture's testkit dir. + * + * The Gradle Tooling API (used by [GradleRunner]) always spawns a daemon and + * provides no public API to stop it (https://github.com/gradle/gradle/issues/12535). + * We replicate the strategy Gradle uses in its own integration tests + * ([DaemonLogsAnalyzer.killAll()][1]): + * + * 1. Scan `/daemon//` for log files matching + * `DaemonLogConstants.DAEMON_LOG_PREFIX + pid + DaemonLogConstants.DAEMON_LOG_SUFFIX`, + * i.e. `daemon-.out.log`. + * 2. Extract the PID from the filename and kill the process. + * + * Trade-offs of the PID-from-filename approach: + * - **PID recycling**: between the build finishing and `kill` being sent, the OS + * could theoretically recycle the PID. In practice the window is short + * (the `finally` block runs immediately after the build) so the risk is negligible. + * - **Filename convention is internal**: Gradle's `DaemonLogConstants.DAEMON_LOG_PREFIX` + * (`"daemon-"`) / `DAEMON_LOG_SUFFIX` (`".out.log"`) are not public API; a future + * Gradle version could change them. The `toLongOrNull()` guard safely skips entries + * that don't parse as a PID (including the UUID fallback Gradle uses when the PID + * is unavailable). + * - **Java 8 compatible**: uses `kill`/`taskkill` via [ProcessBuilder] instead of + * `ProcessHandle` (Java 9+) because build logic targets JVM 1.8. + * + * [1]: https://github.com/gradle/gradle/blob/43b381d88/testing/internal-distribution-testing/src/main/groovy/org/gradle/integtests/fixtures/daemon/DaemonLogsAnalyzer.groovy + */ + private fun stopDaemons() { + val daemonDir = File(testKitDir, "daemon") + if (!daemonDir.exists()) return + + daemonDir.walkTopDown() + .filter { it.isFile && it.name.endsWith(".out.log") && !it.name.startsWith("hs_err") } + .forEach { logFile -> + val pid = logFile.nameWithoutExtension // daemon-12345.out + .removeSuffix(".out") // daemon-12345 + .removePrefix("daemon-") // 12345 + .toLongOrNull() ?: return@forEach // skip UUIDs / unparseable names + + val isWindows = System.getProperty("os.name").lowercase().contains("win") + val killProcess = if (isWindows) { + ProcessBuilder("taskkill", "/F", "/PID", pid.toString()) + } else { + ProcessBuilder("kill", pid.toString()) + } + try { + val process = killProcess.redirectErrorStream(true).start() + process.waitFor(5, java.util.concurrent.TimeUnit.SECONDS) + } catch (_: Exception) { + // best effort — daemon may already be stopped + } + } + } + /** * Adds a subproject to the build. * Updates settings.gradle and creates the build script for the subproject. From ddc823dcf09adcd5f2705a0d8cfcdc7d41c473fd Mon Sep 17 00:00:00 2001 From: Brice Dutheil Date: Fri, 20 Mar 2026 18:40:41 +0100 Subject: [PATCH 3/3] fix: actually move testkit dir outside JUnit @TempDir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 41002d918d intended to place the testkit directory in a separate OS temp location, but the change still used file(".testkit") which resolves to File(projectDir, ".testkit") — still inside the JUnit @TempDir. So closing the daemon is not enough because the Gradle daemon held file locks on the testkit caches when JUnit attempted cleanup. --- .../datadog/gradle/plugin/GradleFixture.kt | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/buildSrc/src/test/kotlin/datadog/gradle/plugin/GradleFixture.kt b/buildSrc/src/test/kotlin/datadog/gradle/plugin/GradleFixture.kt index f74f70dafb6..5d6ca45b6fd 100644 --- a/buildSrc/src/test/kotlin/datadog/gradle/plugin/GradleFixture.kt +++ b/buildSrc/src/test/kotlin/datadog/gradle/plugin/GradleFixture.kt @@ -6,6 +6,7 @@ import org.gradle.testkit.runner.UnexpectedBuildResultException import org.intellij.lang.annotations.Language import org.w3c.dom.Document import java.io.File +import java.nio.file.Files import javax.xml.parsers.DocumentBuilderFactory /** @@ -13,11 +14,19 @@ import javax.xml.parsers.DocumentBuilderFactory * Provides common functionality for setting up test projects and running Gradle builds. */ internal open class GradleFixture(protected val projectDir: File) { - // Each fixture gets its own testkit dir so that a fresh daemon is started per - // test — ensuring withEnvironment() vars (e.g. MAVEN_REPOSITORY_PROXY) are - // correctly set on the daemon JVM and not inherited from a previously-started - // daemon with a different test's environment. - private val testKitDir: File by lazy { file(".testkit") } + // Each fixture gets its own testkit dir in the system temp directory (NOT under + // projectDir) so that JUnit's @TempDir cleanup doesn't race with daemon file locks. + // See https://github.com/gradle/gradle/issues/12535 + // A fresh daemon is started per test — ensuring withEnvironment() vars (e.g. + // MAVEN_REPOSITORY_PROXY) are correctly set on the daemon JVM and not inherited + // from a previously-started daemon with a different test's environment. + // A JVM shutdown hook removes the directory after all tests have run (and daemons + // have been stopped), so file locks are guaranteed to be released by then. + private val testKitDir: File by lazy { + Files.createTempDirectory("gradle-testkit-").toFile().also { dir -> + Runtime.getRuntime().addShutdownHook(Thread { dir.deleteRecursively() }) + } + } /** * Runs Gradle with the specified arguments.