Skip to content

Commit 1916c90

Browse files
bric3devflow.devflow-routing-intake
andauthored
Explicit Gradle daemon isolation in build logic tests (#10912)
fix: Moved Gradle TestKit directory to separate OS temp directory In 748292c 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. gradle/gradle#12535 fix: Improve Gradle daemon isolation and proper gradle daemon shutdown fix: actually move testkit dir outside JUnit @tempdir Commit 41002d9 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. Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent ff3b3f5 commit 1916c90

File tree

1 file changed

+77
-6
lines changed

1 file changed

+77
-6
lines changed

buildSrc/src/test/kotlin/datadog/gradle/plugin/GradleFixture.kt

Lines changed: 77 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,40 +6,111 @@ import org.gradle.testkit.runner.UnexpectedBuildResultException
66
import org.intellij.lang.annotations.Language
77
import org.w3c.dom.Document
88
import java.io.File
9+
import java.nio.file.Files
910
import javax.xml.parsers.DocumentBuilderFactory
1011

1112
/**
1213
* Base fixture for Gradle plugin integration tests.
1314
* Provides common functionality for setting up test projects and running Gradle builds.
1415
*/
1516
internal open class GradleFixture(protected val projectDir: File) {
17+
// Each fixture gets its own testkit dir in the system temp directory (NOT under
18+
// projectDir) so that JUnit's @TempDir cleanup doesn't race with daemon file locks.
19+
// See https://github.com/gradle/gradle/issues/12535
20+
// A fresh daemon is started per test — ensuring withEnvironment() vars (e.g.
21+
// MAVEN_REPOSITORY_PROXY) are correctly set on the daemon JVM and not inherited
22+
// from a previously-started daemon with a different test's environment.
23+
// A JVM shutdown hook removes the directory after all tests have run (and daemons
24+
// have been stopped), so file locks are guaranteed to be released by then.
25+
private val testKitDir: File by lazy {
26+
Files.createTempDirectory("gradle-testkit-").toFile().also { dir ->
27+
Runtime.getRuntime().addShutdownHook(Thread { dir.deleteRecursively() })
28+
}
29+
}
30+
1631
/**
1732
* Runs Gradle with the specified arguments.
1833
*
34+
* After the build completes, any Gradle daemons started by TestKit are killed
35+
* so their file locks on the testkit cache are released before JUnit `@TempDir`
36+
* cleanup. See https://github.com/gradle/gradle/issues/12535
37+
*
1938
* @param args Gradle task names and arguments
2039
* @param expectFailure Whether the build is expected to fail
2140
* @param env Environment variables to set (merged with system environment)
2241
* @return The build result
2342
*/
2443
fun run(vararg args: String, expectFailure: Boolean = false, env: Map<String, String> = emptyMap()): BuildResult {
2544
val runner = GradleRunner.create()
26-
// Use a testkit dir scoped to this fixture's projectDir. The Tooling API always uses a
27-
// daemon and ignores org.gradle.daemon=false. By giving each test its own testkit dir,
28-
// we force a fresh daemon per test — ensuring withEnvironment() vars (e.g.
29-
// MAVEN_REPOSITORY_PROXY) are correctly set on the daemon JVM and not inherited from
30-
// a previously-started daemon with a different test's environment.
31-
.withTestKitDir(file(".testkit"))
45+
.withTestKitDir(testKitDir)
3246
.withPluginClasspath()
3347
.withProjectDir(projectDir)
48+
// Using withDebug prevents starting a daemon, but it doesn't work with withEnvironment
3449
.withEnvironment(System.getenv() + env)
3550
.withArguments(*args)
3651
return try {
3752
if (expectFailure) runner.buildAndFail() else runner.build()
3853
} catch (e: UnexpectedBuildResultException) {
3954
e.buildResult
55+
} finally {
56+
stopDaemons()
4057
}
4158
}
4259

60+
/**
61+
* Kills Gradle daemons started by TestKit for this fixture's testkit dir.
62+
*
63+
* The Gradle Tooling API (used by [GradleRunner]) always spawns a daemon and
64+
* provides no public API to stop it (https://github.com/gradle/gradle/issues/12535).
65+
* We replicate the strategy Gradle uses in its own integration tests
66+
* ([DaemonLogsAnalyzer.killAll()][1]):
67+
*
68+
* 1. Scan `<testkit>/daemon/<version>/` for log files matching
69+
* `DaemonLogConstants.DAEMON_LOG_PREFIX + pid + DaemonLogConstants.DAEMON_LOG_SUFFIX`,
70+
* i.e. `daemon-<pid>.out.log`.
71+
* 2. Extract the PID from the filename and kill the process.
72+
*
73+
* Trade-offs of the PID-from-filename approach:
74+
* - **PID recycling**: between the build finishing and `kill` being sent, the OS
75+
* could theoretically recycle the PID. In practice the window is short
76+
* (the `finally` block runs immediately after the build) so the risk is negligible.
77+
* - **Filename convention is internal**: Gradle's `DaemonLogConstants.DAEMON_LOG_PREFIX`
78+
* (`"daemon-"`) / `DAEMON_LOG_SUFFIX` (`".out.log"`) are not public API; a future
79+
* Gradle version could change them. The `toLongOrNull()` guard safely skips entries
80+
* that don't parse as a PID (including the UUID fallback Gradle uses when the PID
81+
* is unavailable).
82+
* - **Java 8 compatible**: uses `kill`/`taskkill` via [ProcessBuilder] instead of
83+
* `ProcessHandle` (Java 9+) because build logic targets JVM 1.8.
84+
*
85+
* [1]: https://github.com/gradle/gradle/blob/43b381d88/testing/internal-distribution-testing/src/main/groovy/org/gradle/integtests/fixtures/daemon/DaemonLogsAnalyzer.groovy
86+
*/
87+
private fun stopDaemons() {
88+
val daemonDir = File(testKitDir, "daemon")
89+
if (!daemonDir.exists()) return
90+
91+
daemonDir.walkTopDown()
92+
.filter { it.isFile && it.name.endsWith(".out.log") && !it.name.startsWith("hs_err") }
93+
.forEach { logFile ->
94+
val pid = logFile.nameWithoutExtension // daemon-12345.out
95+
.removeSuffix(".out") // daemon-12345
96+
.removePrefix("daemon-") // 12345
97+
.toLongOrNull() ?: return@forEach // skip UUIDs / unparseable names
98+
99+
val isWindows = System.getProperty("os.name").lowercase().contains("win")
100+
val killProcess = if (isWindows) {
101+
ProcessBuilder("taskkill", "/F", "/PID", pid.toString())
102+
} else {
103+
ProcessBuilder("kill", pid.toString())
104+
}
105+
try {
106+
val process = killProcess.redirectErrorStream(true).start()
107+
process.waitFor(5, java.util.concurrent.TimeUnit.SECONDS)
108+
} catch (_: Exception) {
109+
// best effort — daemon may already be stopped
110+
}
111+
}
112+
}
113+
43114
/**
44115
* Adds a subproject to the build.
45116
* Updates settings.gradle and creates the build script for the subproject.

0 commit comments

Comments
 (0)