Skip to content

Commit

Permalink
Rework the way "JDK tests" are done
Browse files Browse the repository at this point in the history
The integration test task is no longer run using a toolchain,
but toolchain is used to determine the installation path and
pass it as system property, that's then used in the TestKit
test project's gradle.properties as org.gradle.java.home.
The Java version is also passed as a system property to
replace JavaVersion.current() in tests.
  • Loading branch information
tbroyer committed Dec 10, 2023
1 parent 4cca0c4 commit 43a06e5
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 21 deletions.
10 changes: 5 additions & 5 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,11 @@ testing {

val testJavaToolchain = project.findProperty("test.java-toolchain")
testJavaToolchain?.also {
javaLauncher.set(
project.javaToolchains.launcherFor {
languageVersion.set(JavaLanguageVersion.of(testJavaToolchain.toString()))
},
)
val metadata = project.javaToolchains.launcherFor {
languageVersion.set(JavaLanguageVersion.of(testJavaToolchain.toString()))
}.get().metadata
systemProperty("test.java-version", metadata.languageVersion.asInt())
systemProperty("test.java-home", metadata.installationPath.asFile.canonicalPath)
}

val testGradleVersion = project.findProperty("test.gradle-version")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package net.ltgt.gradle.errorprone
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.io.TempDir
import java.io.File
import java.util.Properties

abstract class AbstractPluginIntegrationTest {

Expand All @@ -13,6 +14,12 @@ abstract class AbstractPluginIntegrationTest {

@BeforeEach
open fun setupProject() {
testProjectDir.resolve("gradle.properties").outputStream().use {
Properties().apply {
setProperty("org.gradle.java.home", testJavaHome)
store(it, null)
}
}
settingsFile = testProjectDir.resolve("settings.gradle.kts").apply {
createNewFile()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class ErrorPronePluginIntegrationTest : AbstractPluginIntegrationTest() {
dependencies {
compileOnly("com.google.errorprone:error_prone_check_api:$errorproneVersion")
}
${if (JavaVersion.current().isJava8) {
${if (testJavaVersion.isJava8) {
""
} else {
"""
Expand Down Expand Up @@ -170,7 +170,7 @@ class ErrorPronePluginIntegrationTest : AbstractPluginIntegrationTest() {
@Test
fun `is configuration-cache friendly`() {
assume().that(
JavaVersion.current() >= JavaVersion.VERSION_16 &&
testJavaVersion >= JavaVersion.VERSION_16 &&
GradleVersion.version(testGradleVersion) < GradleVersion.version("7.0"),
).isFalse()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import org.gradle.testkit.runner.GradleRunner
import org.gradle.util.GradleVersion
import java.io.File

val testJavaVersion = JavaVersion.toVersion(System.getProperty("test.java-version") ?: JavaVersion.current())
val testJavaHome = System.getProperty("test.java-home", System.getProperty("java.home"))
val testGradleVersion = System.getProperty("test.gradle-version", GradleVersion.current().version)

const val MAX_JDK8_COMPATIBLE_ERRORPRONE_VERSION = "2.10.0"

val errorproneVersion = if (JavaVersion.current().isJava8) MAX_JDK8_COMPATIBLE_ERRORPRONE_VERSION else System.getProperty("errorprone.version")!!
val errorproneVersion = if (testJavaVersion.isJava8) MAX_JDK8_COMPATIBLE_ERRORPRONE_VERSION else System.getProperty("errorprone.version")!!

const val FAILURE_SOURCE_COMPILATION_ERROR = "Failure.java:6: error: [ArrayEquals]"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class GroovyDslIntegrationTest {
@BeforeEach
fun setupProject() {
assume().that(
JavaVersion.current() >= JavaVersion.VERSION_16 &&
testJavaVersion >= JavaVersion.VERSION_16 &&
GradleVersion.version(testGradleVersion) < GradleVersion.version("7.0"),
).isFalse()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {

@Test
fun `does not configure forking in non-Java 8 or 16+ VM`() {
assume().withMessage("isJava8").that(JavaVersion.current().isJava8).isFalse()
assume().withMessage("isJava16Compatible").that(JavaVersion.current()).isLessThan(JavaVersion.VERSION_16)
assume().withMessage("isJava8").that(testJavaVersion.isJava8).isFalse()
assume().withMessage("isJava16Compatible").that(testJavaVersion).isLessThan(JavaVersion.VERSION_16)

// when
testProjectDir.buildWithArgs("compileJava").also { result ->
Expand Down Expand Up @@ -94,7 +94,7 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {

@Test
fun `configure forking in Java 8 VM`() {
assume().withMessage("isJava8").that(JavaVersion.current().isJava8).isTrue()
assume().withMessage("isJava8").that(testJavaVersion.isJava8).isTrue()

// when
testProjectDir.buildWithArgs("compileJava").also { result ->
Expand All @@ -115,7 +115,7 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {

@Test
fun `configure forking in Java 16+ VM`() {
assume().withMessage("isJava16Compatible").that(JavaVersion.current()).isAtLeast(JavaVersion.VERSION_16)
assume().withMessage("isJava16Compatible").that(testJavaVersion).isAtLeast(JavaVersion.VERSION_16)

// when
testProjectDir.buildWithArgs("compileJava").also { result ->
Expand All @@ -138,7 +138,7 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {

@Test
fun `does not configure forking if Error Prone is disabled`() {
assume().withMessage("isJava8Or16plus").that(JavaVersion.current().run { isJava8 || this >= JavaVersion.VERSION_16 }).isTrue()
assume().withMessage("isJava8Or16plus").that(testJavaVersion.run { isJava8 || this >= JavaVersion.VERSION_16 }).isTrue()

// given
buildFile.appendText(
Expand All @@ -161,7 +161,7 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {

@Test
fun `configure bootclasspath for already-forked tasks without javaHome or executable`() {
assume().withMessage("isJava8").that(JavaVersion.current().isJava8).isTrue()
assume().withMessage("isJava8").that(testJavaVersion.isJava8).isTrue()

// given
buildFile.appendText(
Expand All @@ -185,7 +185,7 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {

@Test
fun `does not configure bootclasspath for already-forked tasks using javaHome`() {
assume().withMessage("isJava8").that(JavaVersion.current().isJava8).isTrue()
assume().withMessage("isJava8").that(testJavaVersion.isJava8).isTrue()

// given
val javaHome = System.getProperty("java.home")
Expand Down Expand Up @@ -221,7 +221,7 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {

@Test
fun `does not configure bootclasspath for already-forked tasks using executable`() {
assume().withMessage("isJava8").that(JavaVersion.current().isJava8).isTrue()
assume().withMessage("isJava8").that(testJavaVersion.isJava8).isTrue()

// given
val javaHome = System.getProperty("java.home")
Expand Down Expand Up @@ -261,7 +261,7 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {

@Test
fun `is build-cache friendly`() {
assume().withMessage("isJava8").that(JavaVersion.current().isJava8).isTrue()
assume().withMessage("isJava8").that(testJavaVersion.isJava8).isTrue()

// given
settingsFile.appendText(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() {
fun setup(testInfo: TestInfo) {
testProjectDir.resolve("gradle.properties").appendText(
"""
org.gradle.java.installations.auto-download=false
""".trimIndent(),
)
Expand Down Expand Up @@ -253,7 +254,7 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() {
// then
result.assumeToolchainAvailable()
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
assertThat(result.output).contains(if (JavaVersion.current() == JavaVersion.VERSION_17) FORKED else NOT_FORKED)
assertThat(result.output).contains(if (testJavaVersion == JavaVersion.VERSION_17) FORKED else NOT_FORKED)
assertThat(result.output).contains(JVM_ARGS_STRONG_ENCAPSULATION)
// Check that the configured jvm arg is preserved
assertThat(result.output).contains(jvmArg("-XshowSettings"))
Expand All @@ -273,7 +274,7 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() {
fun `does not configure forking in Java 16+ VM if current JVM has appropriate JVM args`() {
// https://github.com/gradle/gradle/issues/16857#issuecomment-931610187
assume().that(GradleVersion.version(testGradleVersion)).isAtLeast(GradleVersion.version("7.0"))
assume().withMessage("isJava16Compatible").that(JavaVersion.current()).isAtLeast(JavaVersion.VERSION_16)
assume().withMessage("isJava16Compatible").that(testJavaVersion).isAtLeast(JavaVersion.VERSION_16)

testProjectDir.resolve("gradle.properties").appendText(
"""
Expand All @@ -288,7 +289,7 @@ class ToolchainsIntegrationTest : AbstractPluginIntegrationTest() {
java {
toolchain {
languageVersion.set(JavaLanguageVersion.of(${JavaVersion.current().majorVersion}))
languageVersion.set(JavaLanguageVersion.of(${testJavaVersion.majorVersion}))
}
}
""".trimIndent(),
Expand Down

0 comments on commit 43a06e5

Please sign in to comment.