Fix multi-jdk testing setup (#1086)

This changes the Gradle build to always create multi-jdk tasks,
and instead use the `enabled` property to determine whether the task
is actually ran or not.

The has the following benefits:
* IntelliJ and other tools understand the task execution graph (e.g. testJdk20 shows up as a task)
* JDK-specific Gradle configurations always exist, so `updateDependencyLocks` is consistent.
This commit is contained in:
Daniel Chao
2025-06-03 08:56:02 -07:00
committed by GitHub
parent 7d50c46c29
commit 4eeb61dc74
5 changed files with 21 additions and 25 deletions

View File

@@ -210,10 +210,18 @@ open class BuildInfo(private val project: Project) {
// Assembles a collection of JDK versions which tests can be run against, considering ancillary // Assembles a collection of JDK versions which tests can be run against, considering ancillary
// parameters like `testAllJdks` and `testExperimentalJdks`. // parameters like `testAllJdks` and `testExperimentalJdks`.
val jdkTestRange: Collection<JavaLanguageVersion> by lazy { val jdkTestRange: Collection<JavaLanguageVersion> by lazy {
JavaVersionRange.inclusive(jdkTestFloor, jdkTestCeiling).filter { version -> JavaVersionRange.inclusive(jdkTestFloor, jdkTestCeiling).toList()
// unless we are instructed to test all JDKs, tests only include LTS releases and }
// versions above the toolchain version.
testAllJdks || (JavaVersionRange.isLTS(version) || version >= jdkToolchainVersion) val JavaLanguageVersion.isEnabled: Boolean
get() = isVersionEnabled(this)
fun isVersionEnabled(version: JavaLanguageVersion): Boolean {
return when {
testAllJdks -> true
multiJdkTesting -> JavaVersionRange.isLTS(version)
testExperimentalJdks -> version > jdkToolchainVersion
else -> false
} }
} }
@@ -287,13 +295,6 @@ open class BuildInfo(private val project: Project) {
// multiply out by jdk vendor // multiply out by jdk vendor
testJdkVendors.map { vendor -> (targetVersion to vendor) } testJdkVendors.map { vendor -> (targetVersion to vendor) }
} }
.filter { (jdkTarget, vendor) ->
// only include experimental tasks in the return suite if the flag is set. if the task
// is withheld from the returned list, it will not be executed by default with `gradle
// check`.
testExperimentalJdks ||
(!namer(jdkTarget, vendor.takeIf { isMultiVendor }).contains("Experimental"))
}
.map { (jdkTarget, vendor) -> .map { (jdkTarget, vendor) ->
if (jdkToolchainVersion == jdkTarget) if (jdkToolchainVersion == jdkTarget)
tasks.register(namer(jdkTarget, vendor)) { tasks.register(namer(jdkTarget, vendor)) {
@@ -310,10 +311,10 @@ open class BuildInfo(private val project: Project) {
) { ) {
targets.all { targets.all {
testTask.configure { testTask.configure {
enabled = jdkTarget.isEnabled
group = Category.VERIFICATION group = Category.VERIFICATION
description = "Run tests against JDK ${jdkTarget.asInt()}" description = "Run tests against JDK ${jdkTarget.asInt()}"
applyConfig(jdkTarget to toolchains.launcherFor { languageVersion = jdkTarget }) applyConfig(jdkTarget to toolchains.launcherFor { languageVersion = jdkTarget })
// fix: on jdk17, we must force the polyglot module on to the modulepath // fix: on jdk17, we must force the polyglot module on to the modulepath
if (jdkTarget.asInt() == 17) if (jdkTarget.asInt() == 17)
jvmArgumentProviders.add( jvmArgumentProviders.add(
@@ -342,9 +343,8 @@ open class BuildInfo(private val project: Project) {
} }
val multiJdkTesting: Boolean by lazy { val multiJdkTesting: Boolean by lazy {
// By default, Pkl is tested against a full range of JDK versions, past and present, within the // Test Pkl against a full range of JDK versions, past and present, within the
// supported bounds of `PKL_TEST_JDK_TARGET` and `PKL_TEST_JDK_MAXIMUM`. To opt-out of this // supported bounds of `PKL_TEST_JDK_TARGET` and `PKL_TEST_JDK_MAXIMUM`.
// behavior, set `-DpklMultiJdkTesting=false` on the Gradle command line.
// //
// In CI, this defaults to `true` to catch potential cross-JDK compat regressions or other bugs. // In CI, this defaults to `true` to catch potential cross-JDK compat regressions or other bugs.
// In local dev, this defaults to `false` to speed up the build and reduce contributor load. // In local dev, this defaults to `false` to speed up the build and reduce contributor load.

View File

@@ -84,6 +84,7 @@ val testStartJavaExecutable by tasks.registering { setupTestStartJavaExecutable(
val testStartJavaExecutableOnOtherJdks = val testStartJavaExecutableOnOtherJdks =
buildInfo.jdkTestRange.map { jdkTarget -> buildInfo.jdkTestRange.map { jdkTarget ->
tasks.register("testStartJavaExecutableJdk${jdkTarget.asInt()}") { tasks.register("testStartJavaExecutableJdk${jdkTarget.asInt()}") {
enabled = buildInfo.isVersionEnabled(jdkTarget)
val toolChainService: JavaToolchainService = serviceOf() val toolChainService: JavaToolchainService = serviceOf()
val launcher = toolChainService.launcherFor { languageVersion = jdkTarget } val launcher = toolChainService.launcherFor { languageVersion = jdkTarget }
setupTestStartJavaExecutable(launcher) setupTestStartJavaExecutable(launcher)
@@ -94,9 +95,7 @@ tasks.assemble { dependsOn(javaExecutable) }
tasks.check { tasks.check {
dependsOn(testStartJavaExecutable) dependsOn(testStartJavaExecutable)
if (buildInfo.multiJdkTesting) { dependsOn(testStartJavaExecutableOnOtherJdks)
dependsOn(testStartJavaExecutableOnOtherJdks)
}
} }
publishing { publishing {

View File

@@ -20,6 +20,7 @@ import org.gradle.accessors.dm.LibrariesForLibs
plugins { plugins {
`java-library` `java-library`
`jvm-toolchains` `jvm-toolchains`
`jvm-test-suite`
id("pklKotlinTest") id("pklKotlinTest")
id("com.diffplug.spotless") id("com.diffplug.spotless")
} }
@@ -135,4 +136,4 @@ tasks.test { configureJdkTestTask(info.javaTestLauncher) }
// inherits the configuration of the default `test` task (aside from an overridden launcher). // inherits the configuration of the default `test` task (aside from an overridden launcher).
val jdkTestTasks = info.multiJdkTestingWith(tasks.test) { (_, jdk) -> configureJdkTestTask(jdk) } val jdkTestTasks = info.multiJdkTestingWith(tasks.test) { (_, jdk) -> configureJdkTestTask(jdk) }
if (info.multiJdkTesting) tasks.check { dependsOn(jdkTestTasks) } tasks.check { dependsOn(jdkTestTasks) }

View File

@@ -99,12 +99,7 @@ val testJavaExecutable by
} }
// Setup `testJavaExecutable` tasks for multi-JDK testing. // Setup `testJavaExecutable` tasks for multi-JDK testing.
val testJavaExecutableOnOtherJdks = val testJavaExecutableOnOtherJdks = buildInfo.multiJdkTestingWith(testJavaExecutable)
if (buildInfo.multiJdkTesting) {
buildInfo.multiJdkTestingWith(testJavaExecutable)
} else {
emptyList()
}
// Prepare a run of the fat JAR, optionally with a specific Java launcher. // Prepare a run of the fat JAR, optionally with a specific Java launcher.
private fun setupJavaExecutableRun( private fun setupJavaExecutableRun(

View File

@@ -120,6 +120,7 @@ for (jdkTarget in buildInfo.jdkTestRange) {
} else { } else {
val task = val task =
tasks.register("testStartFatJarJdk${jdkTarget.asInt()}", Exec::class) { tasks.register("testStartFatJarJdk${jdkTarget.asInt()}", Exec::class) {
enabled = buildInfo.isVersionEnabled(jdkTarget)
val launcher = project.javaToolchains.launcherFor { languageVersion = jdkTarget } val launcher = project.javaToolchains.launcherFor { languageVersion = jdkTarget }
configureTestStartFatJar(launcher) configureTestStartFatJar(launcher)
} }