Skip to content

Commit

Permalink
Refactor KotlinCoreEnvironment test setup (detekt#1880)
Browse files Browse the repository at this point in the history 
* Refactor KotlinCoreEnvironment test setup

This PR adds an extension function to `Root` to automatically create and
dispose the KotlinCoreEnvironment for testing purposes.
The objects in the KotlinCoreEnvironment are wrapped to simplify the
object creation and disposal.

* Inline KotlinCoreEnvironment test creation

* Refactor KotlinCoreEnvironmentWrapper environment
  • Loading branch information
@schalkms @sowmyav24
schalkms authored and sowmyav24 committed Sep 17, 2019
1 parent 9e459f4 commit 15bbe87
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 123 deletions.
30 changes: 9 additions & 21 deletions detekt-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MissingWhenCaseSpec.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,17 @@ package io.gitlab.arturbosch.detekt.rules.bugs
import io.gitlab.arturbosch.detekt.test.KtTestCompiler
import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext
import org.assertj.core.api.Assertions.assertThat
import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
import org.jetbrains.kotlin.com.intellij.openapi.Disposable
import org.jetbrains.kotlin.com.intellij.openapi.util.Disposer
import org.spekframework.spek2.Spek
import org.spekframework.spek2.style.specification.describe

object MissingWhenCaseSpec : Spek({

val subject by memoized { MissingWhenCase() }

var environment: KotlinCoreEnvironment? = null
lateinit var disposable: Disposable

beforeEachTest {
val pair = KtTestCompiler.createEnvironment()
disposable = pair.first
environment = pair.second
}
val wrapper by memoized(
factory = { KtTestCompiler.createEnvironment() },
destructor = { it.dispose() }
)

describe("MissingWhenCase rule") {
context("enum") {
Expand All @@ -39,7 +32,7 @@ object MissingWhenCaseSpec : Spek({
}
}
"""
val actual = subject.compileAndLintWithContext(environment!!, code)
val actual = subject.compileAndLintWithContext(wrapper.env, code)
assertThat(actual).hasSize(1)
assertThat(actual.first().issue.id).isEqualTo("MissingWhenCase")
assertThat(actual.first().message).isEqualTo("When expression is missing cases: RED. Either add missing cases or a default `else` case.")
Expand Down Expand Up @@ -68,7 +61,7 @@ object MissingWhenCaseSpec : Spek({
}
}
"""
assertThat(subject.compileAndLintWithContext(environment!!, code)).isEmpty()
assertThat(subject.compileAndLintWithContext(wrapper.env, code)).isEmpty()
}
}
context("sealed classes") {
Expand All @@ -87,7 +80,7 @@ object MissingWhenCaseSpec : Spek({
}
}
"""
val actual = subject.compileAndLintWithContext(environment!!, code)
val actual = subject.compileAndLintWithContext(wrapper.env, code)
assertThat(actual).hasSize(1)
assertThat(actual.first().issue.id).isEqualTo("MissingWhenCase")
assertThat(actual.first().message).isEqualTo("When expression is missing cases: VariantC. Either add missing cases or a default `else` case.")
Expand Down Expand Up @@ -117,7 +110,7 @@ object MissingWhenCaseSpec : Spek({
}
}
"""
assertThat(subject.compileAndLintWithContext(environment!!, code)).isEmpty()
assertThat(subject.compileAndLintWithContext(wrapper.env, code)).isEmpty()
}
}
context("standard when") {
Expand Down Expand Up @@ -155,13 +148,8 @@ object MissingWhenCaseSpec : Spek({
}
}
"""
assertThat(subject.compileAndLintWithContext(environment!!, code)).isEmpty()
assertThat(subject.compileAndLintWithContext(wrapper.env, code)).isEmpty()
}
}
}

afterEachTest {
Disposer.dispose(disposable)
environment = null
}
})
38 changes: 13 additions & 25 deletions ...t-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/RedundantElseInWhenSpec.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,17 @@ package io.gitlab.arturbosch.detekt.rules.bugs
import io.gitlab.arturbosch.detekt.test.KtTestCompiler
import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext
import org.assertj.core.api.Assertions.assertThat
import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
import org.jetbrains.kotlin.com.intellij.openapi.Disposable
import org.jetbrains.kotlin.com.intellij.openapi.util.Disposer
import org.spekframework.spek2.Spek
import org.spekframework.spek2.style.specification.describe

object RedundantElseInWhenSpec : Spek({

val subject by memoized { RedundantElseInWhen() }

var environment: KotlinCoreEnvironment? = null
lateinit var disposable: Disposable

beforeEachTest {
val pair = KtTestCompiler.createEnvironment()
disposable = pair.first
environment = pair.second
}
val wrapper by memoized(
factory = { KtTestCompiler.createEnvironment() },
destructor = { it.dispose() }
)

describe("RedundantElseInWhen rule") {
context("enum") {
Expand All @@ -41,7 +34,7 @@ object RedundantElseInWhenSpec : Spek({
}
}
"""
val actual = subject.compileAndLintWithContext(environment!!, code)
val actual = subject.compileAndLintWithContext(wrapper.env, code)
assertThat(actual).hasSize(1)
}

Expand All @@ -62,7 +55,7 @@ object RedundantElseInWhenSpec : Spek({
}
}
"""
val actual = subject.compileAndLintWithContext(environment!!, code)
val actual = subject.compileAndLintWithContext(wrapper.env, code)
assertThat(actual).hasSize(1)
}

Expand All @@ -88,7 +81,7 @@ object RedundantElseInWhenSpec : Spek({
}
}
"""
assertThat(subject.compileAndLintWithContext(environment!!, code)).isEmpty()
assertThat(subject.compileAndLintWithContext(wrapper.env, code)).isEmpty()
}

it("does not report when `when` expression does not contain else case") {
Expand Down Expand Up @@ -120,7 +113,7 @@ object RedundantElseInWhenSpec : Spek({
}
}
"""
assertThat(subject.compileAndLintWithContext(environment!!, code)).isEmpty()
assertThat(subject.compileAndLintWithContext(wrapper.env, code)).isEmpty()
}
}
context("sealed classes") {
Expand All @@ -141,7 +134,7 @@ object RedundantElseInWhenSpec : Spek({
}
}
"""
val actual = subject.compileAndLintWithContext(environment!!, code)
val actual = subject.compileAndLintWithContext(wrapper.env, code)
assertThat(actual).hasSize(1)
}

Expand All @@ -162,7 +155,7 @@ object RedundantElseInWhenSpec : Spek({
}
}
"""
val actual = subject.compileAndLintWithContext(environment!!, code)
val actual = subject.compileAndLintWithContext(wrapper.env, code)
assertThat(actual).hasSize(1)
}

Expand All @@ -188,7 +181,7 @@ object RedundantElseInWhenSpec : Spek({
}
}
"""
assertThat(subject.compileAndLintWithContext(environment!!, code)).isEmpty()
assertThat(subject.compileAndLintWithContext(wrapper.env, code)).isEmpty()
}
it("does not report when `when` expression does not contain else case") {
val code = """
Expand All @@ -205,7 +198,7 @@ object RedundantElseInWhenSpec : Spek({
}
}
"""
assertThat(subject.compileAndLintWithContext(environment!!, code)).isEmpty()
assertThat(subject.compileAndLintWithContext(wrapper.env, code)).isEmpty()
}
}
context("standard when") {
Expand Down Expand Up @@ -243,13 +236,8 @@ object RedundantElseInWhenSpec : Spek({
}
}
"""
assertThat(subject.compileAndLintWithContext(environment!!, code)).isEmpty()
assertThat(subject.compileAndLintWithContext(wrapper.env, code)).isEmpty()
}
}
}

afterEachTest {
Disposer.dispose(disposable)
environment = null
}
})
34 changes: 11 additions & 23 deletions ...rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/SpreadOperatorSpec.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ import io.gitlab.arturbosch.detekt.test.KtTestCompiler
import io.gitlab.arturbosch.detekt.test.compileAndLint
import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext
import org.assertj.core.api.Assertions.assertThat
import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
import org.jetbrains.kotlin.com.intellij.openapi.Disposable
import org.jetbrains.kotlin.com.intellij.openapi.util.Disposer
import org.spekframework.spek2.Spek
import org.spekframework.spek2.style.specification.describe

Expand All @@ -20,14 +17,10 @@ class SpreadOperatorSpec : Spek({
*/
context("with type resolution") {

var environment: KotlinCoreEnvironment? = null
lateinit var disposable: Disposable

beforeEachTest {
val pair = KtTestCompiler.createEnvironment()
disposable = pair.first
environment = pair.second
}
val wrapper by memoized(
factory = { KtTestCompiler.createEnvironment() },
destructor = { it.dispose() }
)

val typeResolutionEnabledMessage = "Used in this way a spread operator causes a full copy of the array to" +
" be created before calling a method which has a very high performance penalty."
Expand All @@ -38,7 +31,7 @@ class SpreadOperatorSpec : Spek({
fun foo(vararg xs: Int) {}
val testVal = foo(xs = *xsArray)
"""
val actual = subject.compileAndLintWithContext(environment!!, code)
val actual = subject.compileAndLintWithContext(wrapper.env, code)
assertThat(actual).hasSize(1)
assertThat(actual.first().message).isEqualTo(typeResolutionEnabledMessage)
}
Expand All @@ -48,7 +41,7 @@ class SpreadOperatorSpec : Spek({
fun foo(vararg xs: Int) {}
val testVal = foo(*xsArray)
"""
val actual = subject.compileAndLintWithContext(environment!!, code)
val actual = subject.compileAndLintWithContext(wrapper.env, code)
assertThat(actual).hasSize(1)
assertThat(actual.first().message).isEqualTo(typeResolutionEnabledMessage)
}
Expand All @@ -57,23 +50,23 @@ class SpreadOperatorSpec : Spek({
fun foo(vararg xs: Int) {}
val testVal = foo(xs = *intArrayOf(1))
"""
assertThat(subject.compileAndLintWithContext(environment!!, code)).isEmpty()
assertThat(subject.compileAndLintWithContext(wrapper.env, code)).isEmpty()
}

it("doesn't report when using array constructor with spread operator when varargs parameter comes first") {
val code = """
fun <T> asList(vararg ts: T, stringValue: String): List<Int> = listOf(1,2,3)
val list = asList(-1, 0, *arrayOf(1, 2, 3), 4, stringValue = "5")
"""
assertThat(subject.compileAndLintWithContext(environment!!, code)).isEmpty()
assertThat(subject.compileAndLintWithContext(wrapper.env, code)).isEmpty()
}

it("doesn't report when passing values directly") {
val code = """
fun <T> asList(vararg ts: T, stringValue: String): List<Int> = listOf(1,2,3)
val list = asList(-1, 0, 1, 2, 3, 4, stringValue = "5")
"""
assertThat(subject.compileAndLintWithContext(environment!!, code)).isEmpty()
assertThat(subject.compileAndLintWithContext(wrapper.env, code)).isEmpty()
}

it("doesn't report when function doesn't take a vararg parameter") {
Expand All @@ -86,7 +79,7 @@ class SpreadOperatorSpec : Spek({
strs.forEach { println(it) }
}
"""
assertThat(subject.compileAndLintWithContext(environment!!, code)).isEmpty()
assertThat(subject.compileAndLintWithContext(wrapper.env, code)).isEmpty()
}

it("doesn't report with expression inside params") {
Expand All @@ -99,12 +92,7 @@ class SpreadOperatorSpec : Spek({
println(test)
}
"""
assertThat(subject.compileAndLintWithContext(environment!!, code)).isEmpty()
}

afterEachTest {
Disposer.dispose(disposable)
environment = null
assertThat(subject.compileAndLintWithContext(wrapper.env, code)).isEmpty()
}
}

Expand Down

0 comments on commit 15bbe87

Please sign in to comment.