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.

Sign up for GitHub

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

Jump to bottom

Migrate detekt-gradle-plugin tests to JUnit #4529

Merged
merged 4 commits into from
Feb 10, 2022
Merged

Migrate detekt-gradle-plugin tests to JUnit #4529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
baa326c
Migrate detekt-gradle-plugin functional tests to JUnit
3flex Jan 26, 2022
8752d19
Migrate detekt-gradle-plugin tests to JUnit
3flex Feb 5, 2022
e675225
Remove unused dependencies
3flex Feb 6, 2022
3e12b6e
Removed unused function
3flex Feb 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions config/detekt/detekt.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,16 @@ formatting:
naming:
ClassNaming:
excludes: ['**/*Spec.kt']
FunctionNaming:
active: true
excludes:
- '**/test/**'
- '**/androidTest/**'
- '**/commonTest/**'
- '**/functionalTest/**'
- '**/jvmTest/**'
- '**/jsTest/**'
- '**/iosTest/**'
TopLevelPropertyNaming:
constantPattern: '[a-z][_A-Za-z0-9]*|[A-Z][_A-Z0-9]*'
InvalidPackageDeclaration:
Expand Down
4 changes: 0 additions & 4 deletions detekt-gradle-plugin/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ testing {
getByName("test", JvmTestSuite::class) {
dependencies {
implementation(libs.assertj)
implementation(libs.spek.dsl)
implementation(libs.kotlin.gradle)
implementation(gradleKotlinDsl())
runtimeOnly(libs.spek.runner)

// Workaround for gradle/gradle#16774, see
// https://github.com/gradle/gradle/issues/16774#issuecomment-853407822
Expand Down Expand Up @@ -51,8 +49,6 @@ testing {

dependencies {
implementation(libs.assertj)
implementation(libs.spek.dsl)
runtimeOnly(libs.spek.runner)
}
}
}
Expand Down
138 changes: 69 additions & 69 deletions ...plugin/src/functionalTest/kotlin/io/gitlab/arturbosch/detekt/CreateBaselineTaskDslSpec.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,83 +4,83 @@ import io.gitlab.arturbosch.detekt.testkit.DslTestBuilder
import io.gitlab.arturbosch.detekt.testkit.ProjectLayout
import org.assertj.core.api.Assertions.assertThat
import org.gradle.testkit.runner.TaskOutcome
import org.spekframework.spek2.Spek
import org.spekframework.spek2.style.specification.describe
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource

internal class CreateBaselineTaskDslSpec : Spek({
describe("The detektBaseline task of the Detekt Gradle plugin") {
listOf(DslTestBuilder.groovy(), DslTestBuilder.kotlin()).forEach { builder ->
describe("using ${builder.gradleBuildName}") {
it("can be executed when baseline file is specified") {
val baselineFilename = "baseline.xml"
class CreateBaselineTaskDslSpec {
@ParameterizedTest(name = "Using {0}, detektBaseline task can be executed when baseline file is specified")
@MethodSource("io.gitlab.arturbosch.detekt.testkit.DslTestBuilder#builders")
fun baselineTaskExecutableWhenBaselineFileSpecified(builder: DslTestBuilder) {
val baselineFilename = "baseline.xml"

val detektConfig = """
|detekt {
| baseline = file("$baselineFilename")
|}
"""
val gradleRunner = builder
.withProjectLayout(
ProjectLayout(
numberOfSourceFilesInRootPerSourceDir = 1,
numberOfCodeSmellsInRootPerSourceDir = 1,
)
)
.withDetektConfig(detektConfig)
.build()
val detektConfig = """
|detekt {
| baseline = file("$baselineFilename")
|}
"""
val gradleRunner = builder
.withProjectLayout(
ProjectLayout(
numberOfSourceFilesInRootPerSourceDir = 1,
numberOfCodeSmellsInRootPerSourceDir = 1,
)
)
.withDetektConfig(detektConfig)
.build()

gradleRunner.runTasksAndCheckResult("detektBaseline") { result ->
assertThat(result.task(":detektBaseline")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
assertThat(projectFile(baselineFilename)).exists()
assertThat(projectFile(DEFAULT_BASELINE_FILENAME)).doesNotExist()
}
}
gradleRunner.runTasksAndCheckResult("detektBaseline") { result ->
assertThat(result.task(":detektBaseline")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
assertThat(projectFile(baselineFilename)).exists()
assertThat(projectFile(DEFAULT_BASELINE_FILENAME)).doesNotExist()
}
}

it("can be executed when baseline file is not specified") {
val detektConfig = """
|detekt {
|}
"""
val gradleRunner = builder
.withProjectLayout(
ProjectLayout(
numberOfSourceFilesInRootPerSourceDir = 1,
numberOfCodeSmellsInRootPerSourceDir = 1,
)
)
.withDetektConfig(detektConfig)
.build()
@ParameterizedTest(name = "Using {0}, detektBaseline task can be executed when baseline file is not specified")
@MethodSource("io.gitlab.arturbosch.detekt.testkit.DslTestBuilder#builders")
fun baselineTaskExecutableWhenBaselineFileNotSpecified(builder: DslTestBuilder) {
val detektConfig = """
|detekt {
|}
"""
val gradleRunner = builder
.withProjectLayout(
ProjectLayout(
numberOfSourceFilesInRootPerSourceDir = 1,
numberOfCodeSmellsInRootPerSourceDir = 1,
)
)
.withDetektConfig(detektConfig)
.build()

gradleRunner.runTasksAndCheckResult("detektBaseline") { result ->
assertThat(result.task(":detektBaseline")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
assertThat(projectFile(DEFAULT_BASELINE_FILENAME)).exists()
}
}
gradleRunner.runTasksAndCheckResult("detektBaseline") { result ->
assertThat(result.task(":detektBaseline")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
assertThat(projectFile(DEFAULT_BASELINE_FILENAME)).exists()
}
}

it("can not be executed when baseline file is specified null") {
val detektConfig = """
|detekt {
| baseline = null
|}
"""
val gradleRunner = builder
.withProjectLayout(
ProjectLayout(
numberOfSourceFilesInRootPerSourceDir = 1,
numberOfCodeSmellsInRootPerSourceDir = 1,
)
)
.withDetektConfig(detektConfig)
.build()
@ParameterizedTest(name = "Using {0}, detektBaseline task can not be executed when baseline file is specified null")
@MethodSource("io.gitlab.arturbosch.detekt.testkit.DslTestBuilder#builders")
fun baselineTaskNotExecutableWhenBaselineFileIsNull(builder: DslTestBuilder) {
val detektConfig = """
|detekt {
| baseline = null
|}
"""
val gradleRunner = builder
.withProjectLayout(
ProjectLayout(
numberOfSourceFilesInRootPerSourceDir = 1,
numberOfCodeSmellsInRootPerSourceDir = 1,
)
)
.withDetektConfig(detektConfig)
.build()

gradleRunner.runTasksAndExpectFailure("detektBaseline") { result ->
assertThat(result.output).contains("property 'baseline' doesn't have a configured value")
assertThat(projectFile(DEFAULT_BASELINE_FILENAME)).doesNotExist()
}
}
}
gradleRunner.runTasksAndExpectFailure("detektBaseline") { result ->
assertThat(result.output).contains("property 'baseline' doesn't have a configured value")
assertThat(projectFile(DEFAULT_BASELINE_FILENAME)).doesNotExist()
}
}
})
}

private const val DEFAULT_BASELINE_FILENAME = "detekt-baseline.xml"
149 changes: 77 additions & 72 deletions ...dle-plugin/src/functionalTest/kotlin/io/gitlab/arturbosch/detekt/DetektReportMergeSpec.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,19 @@ import io.gitlab.arturbosch.detekt.testkit.DslGradleRunner
import io.gitlab.arturbosch.detekt.testkit.DslTestBuilder
import io.gitlab.arturbosch.detekt.testkit.ProjectLayout
import org.assertj.core.api.Assertions.assertThat
import org.spekframework.spek2.Spek
import org.spekframework.spek2.style.specification.describe
import org.junit.jupiter.api.Nested
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.Arguments
import org.junit.jupiter.params.provider.Arguments.arguments
import org.junit.jupiter.params.provider.MethodSource

internal class DetektReportMergeSpec : Spek({
class DetektReportMergeSpec {

describe("Sarif merge is configured correctly for multi module project") {
@Nested
inner class `Sarif merge is configured correctly for multi module project` {

val groovy by memoized { DslTestBuilder.groovy() }
val groovyBuildFileContent by memoized {
val groovy = DslTestBuilder.groovy()
val groovyBuildFileContent =
"""
|${groovy.gradlePlugins}
|
Expand Down Expand Up @@ -41,9 +45,8 @@ internal class DetektReportMergeSpec : Spek({
| }
|}
|""".trimMargin()
}
val kotlin by memoized { DslTestBuilder.kotlin() }
val kotlinBuildFileContent by memoized {
val kotlin = DslTestBuilder.kotlin()
val kotlinBuildFileContent =
"""
|${kotlin.gradlePlugins}
|
Expand Down Expand Up @@ -72,45 +75,47 @@ internal class DetektReportMergeSpec : Spek({
| }
|}
|""".trimMargin()
}

it("using Groovy and Kotlin") {
listOf(
groovy to groovyBuildFileContent,
kotlin to kotlinBuildFileContent
).forEach { (builder, mainBuildFileContent) ->
val projectLayout = ProjectLayout(numberOfSourceFilesInRootPerSourceDir = 0).apply {
addSubmodule(
name = "child1",
numberOfSourceFilesPerSourceDir = 2,
numberOfCodeSmells = 2
)
addSubmodule(
name = "child2",
numberOfSourceFilesPerSourceDir = 4,
numberOfCodeSmells = 4
)
}
fun scenarios(): List<Arguments> = listOf(
arguments(groovy, groovyBuildFileContent),
arguments(kotlin, kotlinBuildFileContent)
)

@ParameterizedTest(name = "Using {0}")
@MethodSource("scenarios")
Comment on lines +79 to +85
Copy link
Member

Choose a reason for hiding this comment

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

Don't commit this! For sure it will not compile.

Suggested change
fun scenarios(): List<Arguments> = listOf(
arguments(groovy, groovyBuildFileContent),
arguments(kotlin, kotlinBuildFileContent)
)
@ParameterizedTest(name = "Using {0}")
@MethodSource("scenarios")
class Scenarios : ArgumentsProvider {
override fun provideArguments(context: ExtensionContext) = Stream.of(
arguments(DslTestBuilder.groovy(), groovyBuildFileContent),
arguments(DslTestBuilder.kotlin(), kotlinBuildFileContent),
)
}
@ParameterizedTest(name = "Using {0}")
@ArgumentsSource(DetektReportMergeSpec.Scenarios::class)

But what do you think of something like this? It avoid String references that are easier to break. The problem is that it's more verbose. It's just a suggestion.

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 think the string reference is OK in this case, given the function sits directly above the test method. Also I just tried and IntelliJ will update the string in the annotation if the method is renamed using Refactor -> Rename in the IDE, and also if the string itself is renamed with Refactor -> Rename.

fun sarifMerge(builder: DslTestBuilder, mainBuildFileContent: String) {
val projectLayout = ProjectLayout(numberOfSourceFilesInRootPerSourceDir = 0).apply {
addSubmodule(
name = "child1",
numberOfSourceFilesPerSourceDir = 2,
numberOfCodeSmells = 2
)
addSubmodule(
name = "child2",
numberOfSourceFilesPerSourceDir = 4,
numberOfCodeSmells = 4
)
}

val gradleRunner = DslGradleRunner(projectLayout, builder.gradleBuildName, mainBuildFileContent)
gradleRunner.setupProject()
gradleRunner.runTasksAndExpectFailure("detekt", "sarifReportMerge", "--continue") { _ ->
assertThat(projectFile("build/reports/detekt/detekt.sarif")).doesNotExist()
assertThat(projectFile("build/reports/detekt/merge.sarif")).exists()
assertThat(projectFile("build/reports/detekt/merge.sarif").readText())
.contains("\"ruleId\": \"detekt.style.MagicNumber\"")
projectLayout.submodules.forEach {
assertThat(projectFile("${it.name}/build/reports/detekt/detekt.sarif")).exists()
}
val gradleRunner = DslGradleRunner(projectLayout, builder.gradleBuildName, mainBuildFileContent)
gradleRunner.setupProject()
gradleRunner.runTasksAndExpectFailure("detekt", "sarifReportMerge", "--continue") { _ ->
assertThat(projectFile("build/reports/detekt/detekt.sarif")).doesNotExist()
assertThat(projectFile("build/reports/detekt/merge.sarif")).exists()
assertThat(projectFile("build/reports/detekt/merge.sarif").readText())
.contains("\"ruleId\": \"detekt.style.MagicNumber\"")
projectLayout.submodules.forEach {
assertThat(projectFile("${it.name}/build/reports/detekt/detekt.sarif")).exists()
}
}
}
}

describe("XML merge is configured correctly for multi module project") {
@Nested
inner class `XML merge is configured correctly for multi module project` {

val groovy by memoized { DslTestBuilder.groovy() }
val groovyBuildFileContent by memoized {
val groovy = DslTestBuilder.groovy()
val groovyBuildFileContent =
"""
|${groovy.gradlePlugins}
|
Expand Down Expand Up @@ -139,9 +144,8 @@ internal class DetektReportMergeSpec : Spek({
| }
|}
|""".trimMargin()
}
val kotlin by memoized { DslTestBuilder.kotlin() }
val kotlinBuildFileContent by memoized {
val kotlin = DslTestBuilder.kotlin()
val kotlinBuildFileContent =
"""
|${kotlin.gradlePlugins}
|
Expand Down Expand Up @@ -170,38 +174,39 @@ internal class DetektReportMergeSpec : Spek({
| }
|}
|""".trimMargin()
}

it("using Groovy and Kotlin") {
listOf(
groovy to groovyBuildFileContent,
kotlin to kotlinBuildFileContent
).forEach { (builder, mainBuildFileContent) ->
val projectLayout = ProjectLayout(numberOfSourceFilesInRootPerSourceDir = 0).apply {
addSubmodule(
name = "child1",
numberOfSourceFilesPerSourceDir = 2,
numberOfCodeSmells = 2
)
addSubmodule(
name = "child2",
numberOfSourceFilesPerSourceDir = 4,
numberOfCodeSmells = 4
)
}
fun scenarios(): List<Arguments> = listOf(
arguments(groovy, groovyBuildFileContent),
arguments(kotlin, kotlinBuildFileContent)
)

@ParameterizedTest(name = "Using {0}")
@MethodSource("scenarios")
fun sarifMerge(builder: DslTestBuilder, mainBuildFileContent: String) {
val projectLayout = ProjectLayout(numberOfSourceFilesInRootPerSourceDir = 0).apply {
addSubmodule(
name = "child1",
numberOfSourceFilesPerSourceDir = 2,
numberOfCodeSmells = 2
)
addSubmodule(
name = "child2",
numberOfSourceFilesPerSourceDir = 4,
numberOfCodeSmells = 4
)
}

val gradleRunner = DslGradleRunner(projectLayout, builder.gradleBuildName, mainBuildFileContent)
gradleRunner.setupProject()
gradleRunner.runTasksAndExpectFailure("detekt", "xmlReportMerge", "--continue") { _ ->
assertThat(projectFile("build/reports/detekt/detekt.xml")).doesNotExist()
assertThat(projectFile("build/reports/detekt/merge.xml")).exists()
assertThat(projectFile("build/reports/detekt/merge.xml").readText())
.contains("<error column=\"30\" line=\"4\"")
projectLayout.submodules.forEach {
assertThat(projectFile("${it.name}/build/reports/detekt/detekt.xml")).exists()
}
val gradleRunner = DslGradleRunner(projectLayout, builder.gradleBuildName, mainBuildFileContent)
gradleRunner.setupProject()
gradleRunner.runTasksAndExpectFailure("detekt", "xmlReportMerge", "--continue") { _ ->
assertThat(projectFile("build/reports/detekt/detekt.xml")).doesNotExist()
assertThat(projectFile("build/reports/detekt/merge.xml")).exists()
assertThat(projectFile("build/reports/detekt/merge.xml").readText())
.contains("<error column=\"30\" line=\"4\"")
projectLayout.submodules.forEach {
assertThat(projectFile("${it.name}/build/reports/detekt/detekt.xml")).exists()
}
}
}
}
})
}
Loading