Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split Gradle functional tests by Gradle version #7036

Merged
merged 5 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/detekt-with-type-resolution.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ jobs:
gradle-home-cache-cleanup: true

- name: Run analysis
run: ./gradlew detektMain detektTest detektFunctionalTest detektTestFixtures detektReportMergeSarif --continue
run: ./gradlew detektMain detektTest detektFunctionalTest detektTestFixtures detektFunctionalTestMinSupportedGradle detektReportMergeSarif --continue

- name: Upload SARIF to GitHub using the upload-sarif action
uses: github/codeql-action/upload-sarif@8a470fddafa5cbb6266ee11b37ef4d8aae19c571 # v3
Expand Down
1 change: 1 addition & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ setOf(
"detektMain",
"detektTest",
"detektFunctionalTest",
"detektFunctionalTestMinSupportedGradle",
"detektTestFixtures",
).forEach { taskName ->
tasks.register(taskName) {
Expand Down
30 changes: 29 additions & 1 deletion detekt-gradle-plugin/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,25 @@ testing {
}
}
}
register<JvmTestSuite>("functionalTestMinSupportedGradle") {
dependencies {
implementation(libs.assertj)
implementation(testFixtures(project()))
}
targets {
all {
testTask {
dependsOn(gradleMinVersionPluginUnderTestMetadata)
}
}
}
}
}
}

val testKitRuntimeOnly: Configuration by configurations.creating
val testKitJava17RuntimeOnly: Configuration by configurations.creating
val testKitGradleMinVersionRuntimeOnly: Configuration by configurations.creating

dependencies {
compileOnly(libs.android.gradle.minSupported)
Expand All @@ -75,6 +89,11 @@ dependencies {
compileOnly("io.gitlab.arturbosch.detekt:detekt-cli:1.23.5")

testKitRuntimeOnly(libs.kotlin.gradle)
testKitGradleMinVersionRuntimeOnly(libs.kotlin.gradle) {
attributes {
attribute(GradlePluginApiVersion.GRADLE_PLUGIN_API_VERSION_ATTRIBUTE, objects.named("6.8.3"))
}
}
testKitJava17RuntimeOnly(libs.android.gradle.maxSupported)

// We use this published version of the detekt-formatting to self analyse this project.
Expand Down Expand Up @@ -111,6 +130,7 @@ gradlePlugin {
testSourceSets(
sourceSets["testFixtures"],
sourceSets["functionalTest"],
sourceSets["functionalTestMinSupportedGradle"],
)
}

Expand All @@ -129,6 +149,11 @@ tasks.pluginUnderTestMetadata {
}
}

val gradleMinVersionPluginUnderTestMetadata by tasks.registering(PluginUnderTestMetadata::class) {
pluginClasspath.setFrom(sourceSets.main.get().runtimeClasspath, testKitGradleMinVersionRuntimeOnly)
outputDirectory = layout.buildDirectory.dir(name)
}

tasks.validatePlugins {
enableStricterValidation = true
}
Expand Down Expand Up @@ -164,7 +189,10 @@ tasks {
}

check {
dependsOn(testing.suites.named("functionalTest"))
dependsOn(
testing.suites.named("functionalTest"),
testing.suites.named("functionalTestMinSupportedGradle"),
)
}

ideaModule {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ package io.gitlab.arturbosch.detekt
import io.gitlab.arturbosch.detekt.testkit.DslTestBuilder
import org.assertj.core.api.Assertions.assertThat
import org.gradle.testkit.runner.TaskOutcome
import org.gradle.testkit.runner.internal.PluginUnderTestMetadataReading
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like using an internal method here but if it's removed/changed in Gradle we can easily copy/paste into our project, it's quite self-contained.

import org.junit.jupiter.api.DisplayName
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.condition.EnabledForJreRange
import org.junit.jupiter.api.condition.JRE.JAVA_15
import java.nio.file.Paths

class GradleVersionSpec {

Expand All @@ -15,7 +17,14 @@ class GradleVersionSpec {
@EnabledForJreRange(max = JAVA_15, disabledReason = "Gradle $GRADLE_VERSION unsupported on this Java version")
fun runsOnOldestSupportedGradleVersion() {
val builder = DslTestBuilder.kotlin()
val gradleRunner = builder.withGradleVersion(GRADLE_VERSION).build()
val metadataUrl =
Paths.get("build/gradleMinVersionPluginUnderTestMetadata/plugin-under-test-metadata.properties")
.toUri()
.toURL()
val gradleRunner = builder
.withGradleVersion(GRADLE_VERSION)
.withPluginClasspath(PluginUnderTestMetadataReading.readImplementationClasspath(metadataUrl))
.build()
gradleRunner.runDetektTaskAndCheckResult { result ->
assertThat(result.task(":detekt")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ constructor(
val gradleVersionOrNone: String? = null,
val dryRun: Boolean = false,
val jvmArgs: String = "-Xmx2g -XX:MaxMetaspaceSize=1g",
val customPluginClasspath: List<File> = emptyList(),
val projectScript: Project.() -> Unit = {}
) {

Expand Down Expand Up @@ -141,7 +142,11 @@ constructor(

return GradleRunner.create().apply {
withProjectDir(rootDir)
withPluginClasspath()
if (customPluginClasspath.isNotEmpty()) {
withPluginClasspath(customPluginClasspath)
} else {
withPluginClasspath()
}
withArguments(args)
gradleVersionOrNone?.let(::withGradleVersion)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.gitlab.arturbosch.detekt.testkit

import org.intellij.lang.annotations.Language
import java.io.File

abstract class DslTestBuilder {

Expand All @@ -24,6 +25,7 @@ abstract class DslTestBuilder {
private var configFile: String? = null
private var gradleVersion: String? = null
private var dryRun: Boolean = false
private val customPluginClasspath: MutableList<File> = mutableListOf()

fun withDetektConfig(@Language("gradle.kts") config: String): DslTestBuilder {
detektConfig = config
Expand All @@ -50,6 +52,11 @@ abstract class DslTestBuilder {
return this
}

fun withPluginClasspath(files: Collection<File>): DslTestBuilder {
customPluginClasspath.addAll(files)
return this
}

fun dryRun(): DslTestBuilder {
dryRun = true
return this
Expand All @@ -64,6 +71,7 @@ abstract class DslTestBuilder {
baselineFiles = baselineFile?.let { listOf(it) }.orEmpty(),
gradleVersionOrNone = gradleVersion,
dryRun = dryRun,
customPluginClasspath = customPluginClasspath,
)
runner.setupProject()
return runner
Expand Down