Skip to content

Commit 7578a30

Browse files
committed
fix: Improve Gradle daemon isolation and proper gradle daemon shutdown
1 parent 41002d9 commit 7578a30

1 file changed

Lines changed: 66 additions & 9 deletions

File tree

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

Lines changed: 66 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,26 +6,26 @@ 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
109
import javax.xml.parsers.DocumentBuilderFactory
1110

1211
/**
1312
* Base fixture for Gradle plugin integration tests.
1413
* Provides common functionality for setting up test projects and running Gradle builds.
1514
*/
1615
internal open class GradleFixture(protected val projectDir: File) {
17-
// Keep test-kit caches outside @TempDir so Gradle daemon file-locks on
18-
// .testkit/caches/*/transforms don't cause JUnit @TempDir cleanup failures
19-
// (DirectoryNotEmptyException). Each fixture still gets its own testkit dir,
20-
// ensuring daemon isolation (fresh MAVEN_REPOSITORY_PROXY per test).
21-
// Cleanup is left to the OS (/tmp is ephemeral on CI containers).
22-
private val testKitDir: File by lazy {
23-
Files.createTempDirectory("gradle-testkit-").toFile()
24-
}
16+
// Each fixture gets its own testkit dir so that a fresh daemon is started per
17+
// test — ensuring withEnvironment() vars (e.g. MAVEN_REPOSITORY_PROXY) are
18+
// correctly set on the daemon JVM and not inherited from a previously-started
19+
// daemon with a different test's environment.
20+
private val testKitDir: File by lazy { file(".testkit") }
2521

2622
/**
2723
* Runs Gradle with the specified arguments.
2824
*
25+
* After the build completes, any Gradle daemons started by TestKit are killed
26+
* so their file locks on the testkit cache are released before JUnit `@TempDir`
27+
* cleanup. See https://github.com/gradle/gradle/issues/12535
28+
*
2929
* @param args Gradle task names and arguments
3030
* @param expectFailure Whether the build is expected to fail
3131
* @param env Environment variables to set (merged with system environment)
@@ -36,15 +36,72 @@ internal open class GradleFixture(protected val projectDir: File) {
3636
.withTestKitDir(testKitDir)
3737
.withPluginClasspath()
3838
.withProjectDir(projectDir)
39+
// Using withDebug prevents starting a daemon, but it doesn't work with withEnvironment
3940
.withEnvironment(System.getenv() + env)
4041
.withArguments(*args)
4142
return try {
4243
if (expectFailure) runner.buildAndFail() else runner.build()
4344
} catch (e: UnexpectedBuildResultException) {
4445
e.buildResult
46+
} finally {
47+
stopDaemons()
4548
}
4649
}
4750

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

0 commit comments

Comments
 (0)