From 7126e4ba3e1b40f55fad9b17af477f78fea67add Mon Sep 17 00:00:00 2001 From: Matthew Haughton <3flex@users.noreply.github.com> Date: Tue, 8 Feb 2022 07:49:00 +1100 Subject: [PATCH] Migrate detekt-rules-performance tests to JUnit (#4569) --- detekt-rules-performance/build.gradle.kts | 3 +- .../rules/performance/ArrayPrimitiveSpec.kt | 158 ++++++++++++------ .../rules/performance/ForEachOnRangeSpec.kt | 31 ++-- .../rules/performance/SpreadOperatorSpec.kt | 82 +++++---- .../UnnecessaryTemporaryInstantiationSpec.kt | 19 ++- 5 files changed, 191 insertions(+), 102 deletions(-) diff --git a/detekt-rules-performance/build.gradle.kts b/detekt-rules-performance/build.gradle.kts index 78e6820b8..e0469ce85 100644 --- a/detekt-rules-performance/build.gradle.kts +++ b/detekt-rules-performance/build.gradle.kts @@ -5,6 +5,5 @@ plugins { dependencies { compileOnly(projects.detektApi) testImplementation(projects.detektTest) - testImplementation(libs.bundles.testImplementation) - testRuntimeOnly(libs.spek.runner) + testImplementation(libs.assertj) } diff --git a/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/ArrayPrimitiveSpec.kt b/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/ArrayPrimitiveSpec.kt index ac59de62c..f5124439c 100644 --- a/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/ArrayPrimitiveSpec.kt +++ b/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/ArrayPrimitiveSpec.kt @@ -1,210 +1,266 @@ package io.gitlab.arturbosch.detekt.rules.performance -import io.gitlab.arturbosch.detekt.rules.setupKotlinEnvironment +import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest 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.spekframework.spek2.Spek -import org.spekframework.spek2.style.specification.describe +import org.junit.jupiter.api.DisplayName +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test -class ArrayPrimitiveSpec : Spek({ - setupKotlinEnvironment() +@KotlinCoreEnvironmentTest +class ArrayPrimitiveSpec(val env: KotlinCoreEnvironment) { - val env: KotlinCoreEnvironment by memoized() - val subject by memoized { ArrayPrimitive() } + val subject = ArrayPrimitive() - describe("one function parameter") { - it("is an array of primitive type") { + @Nested + inner class `one function parameter` { + @Test + fun `is an array of primitive type`() { val code = "fun function(array: Array) {}" assertThat(subject.compileAndLint(code)).hasSize(1) } - it("is not an array") { + @Test + fun `is not an array`() { val code = "fun function(i: Int) {}" assertThat(subject.compileAndLint(code)).isEmpty() } - it("is a specialized array") { + @Test + fun `is a specialized array`() { val code = "fun function(array: ByteArray) {}" assertThat(subject.compileAndLint(code)).isEmpty() } - it("is a star-projected array") { + @Test + fun `is a star-projected array`() { val code = "fun function(array: Array<*>) {}" assertThat(subject.compileAndLint(code)).isEmpty() } - it("is not present") { + @Test + fun `is not present`() { val code = "fun function() {}" assertThat(subject.compileAndLint(code)).isEmpty() } - it("is an array of a non-primitive type") { + @Test + fun `is an array of a non-primitive type`() { val code = "fun function(array: Array) {}" assertThat(subject.compileAndLint(code)).isEmpty() } - it("is an array of an array of a primitive type") { + @Test + fun `is an array of an array of a primitive type`() { val code = "fun function(array: Array>) {}" assertThat(subject.compileAndLint(code)).hasSize(1) } - it("is a dictionary with an array of a primitive type as key") { + @Test + fun `is a dictionary with an array of a primitive type as key`() { val code = "fun function(dict: java.util.Dictionary>) {}" assertThat(subject.compileAndLint(code)).hasSize(1) } } - describe("multiple function parameters") { - it("one is Array and the other is not") { + @Nested + inner class `multiple function parameters` { + @Test + @DisplayName("one is Array and the other is not") + fun oneArrayPrimitive() { val code = "fun function(array: Array, array2: IntArray) {}" assertThat(subject.compileAndLint(code)).hasSize(1) } - it("both are arrays of primitive types") { + @Test + fun `both are arrays of primitive types`() { val code = "fun function(array: Array, array2: Array) {}" assertThat(subject.compileAndLint(code)).hasSize(2) } } - describe("return type") { - it("is Array") { + @Nested + inner class `return type` { + @Test + @DisplayName("is Array") + fun isArrayPrimitive() { val code = "fun returningFunction(): Array { return emptyArray() }" assertThat(subject.compileAndLint(code)).hasSize(1) } - it("is not an array") { + @Test + fun `is not an array`() { val code = "fun returningFunction(): Int { return 1 }" assertThat(subject.compileAndLint(code)).isEmpty() } - it("is a specialized array") { + @Test + fun `is a specialized array`() { val code = "fun returningFunction(): CharArray { return CharArray(0) }" assertThat(subject.compileAndLint(code)).isEmpty() } - it("is a star-projected array") { + @Test + fun `is a star-projected array`() { val code = "fun returningFunction(): Array<*> { return emptyArray() }" assertThat(subject.compileAndLint(code)).isEmpty() } - it("is not explicitly set") { + @Test + fun `is not explicitly set`() { val code = "fun returningFunction() {}" assertThat(subject.compileAndLint(code)).isEmpty() } } - describe("variable type") { - it("is Array") { + @Nested + inner class `variable type` { + @Test + @DisplayName("is Array") + fun isArrayPrimitive() { val code = "val foo: Array? = null" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } } - describe("receiver type") { - it("is Array") { + @Nested + inner class `receiver type` { + @Test + @DisplayName("is Array") + fun isArrayPrimitive() { val code = "fun Array.foo() { println(this) }" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } } - describe("arrayOf") { - it("is arrayOf(Char)") { + @Nested + inner class `arrayOf` { + @Test + fun `is arrayOf(Char)`() { val code = "fun foo(x: Char) = arrayOf(x)" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } - it("is arrayOf(Byte)") { + @Test + fun `is arrayOf(Byte)`() { val code = "fun foo(x: Byte) = arrayOf(x)" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } - it("is arrayOf(Short)") { + @Test + fun `is arrayOf(Short)`() { val code = "fun foo(x: Short) = arrayOf(x)" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } - it("is arrayOf(Int)") { + @Test + fun `is arrayOf(Int)`() { val code = "fun foo(x: Int) = arrayOf(x)" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } - it("is arrayOf(Long)") { + @Test + fun `is arrayOf(Long)`() { val code = "fun foo(x: Long) = arrayOf(x)" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } - it("is arrayOf(Float)") { + @Test + fun `is arrayOf(Float)`() { val code = "fun foo(x: Float) = arrayOf(x)" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } - it("is arrayOf(Double)") { + @Test + fun `is arrayOf(Double)`() { val code = "fun foo(x: Double) = arrayOf(x)" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } - it("is arrayOf(Boolean)") { + @Test + fun `is arrayOf(Boolean)`() { val code = "fun foo(x: Boolean) = arrayOf(x)" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } - it("is arrayOf(String)") { + @Test + fun `is arrayOf(String)`() { val code = "fun foo(x: String) = arrayOf(x)" assertThat(subject.compileAndLintWithContext(env, code)).isEmpty() } - it("is intArrayOf()") { + @Test + fun `is intArrayOf()`() { val code = "fun test(x: Int) = intArrayOf(x)" assertThat(subject.compileAndLintWithContext(env, code)).isEmpty() } } - describe("emptyArray") { - it("is emptyArray()") { + @Nested + inner class `emptyArray` { + @Test + @DisplayName("is emptyArray()") + fun isEmptyArrayChar() { val code = "val a = emptyArray()" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } - it("is emptyArray()") { + @Test + @DisplayName("is emptyArray()") + fun isEmptyArrayByte() { val code = "val a = emptyArray()" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } - it("is emptyArray()") { + @Test + @DisplayName("is emptyArray()") + fun isEmptyArrayShort() { val code = "val a = emptyArray()" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } - it("is emptyArray()") { + @Test + @DisplayName("is emptyArray()") + fun isEmptyArrayInt() { val code = "val a = emptyArray()" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } - it("is emptyArray()") { + @Test + @DisplayName("is emptyArray()") + fun isEmptyArrayLong() { val code = "val a = emptyArray()" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } - it("is emptyArray()") { + @Test + @DisplayName("is emptyArray()") + fun isEmptyArrayFloat() { val code = "val a = emptyArray()" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } - it("is emptyArray()") { + @Test + @DisplayName("is emptyArray()") + fun isEmptyArrayDouble() { val code = "val a = emptyArray()" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } - it("is emptyArray()") { + @Test + @DisplayName("is emptyArray()") + fun isEmptyArrayBoolean() { val code = "val a = emptyArray()" assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1) } - it("is emptyArray()") { + @Test + @DisplayName("is emptyArray()") + fun isEmptyArrayString() { val code = "val a = emptyArray()" assertThat(subject.compileAndLintWithContext(env, code)).isEmpty() } } -}) +} diff --git a/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/ForEachOnRangeSpec.kt b/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/ForEachOnRangeSpec.kt index 51d086fa1..43198b501 100644 --- a/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/ForEachOnRangeSpec.kt +++ b/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/ForEachOnRangeSpec.kt @@ -2,16 +2,18 @@ package io.gitlab.arturbosch.detekt.rules.performance import io.gitlab.arturbosch.detekt.test.compileAndLint 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.api.Test -class ForEachOnRangeSpec : Spek({ +class ForEachOnRangeSpec { - val subject by memoized { ForEachOnRange() } + val subject = ForEachOnRange() - describe("ForEachOnRange rule") { + @Nested + inner class `ForEachOnRange rule` { - context("using a forEach on a range") { + @Nested + inner class `using a forEach on a range` { val code = """ fun test() { (1..10).forEach { @@ -29,26 +31,30 @@ class ForEachOnRangeSpec : Spek({ } """ - it("should report the forEach usage") { + @Test + fun `should report the forEach usage`() { val findings = subject.compileAndLint(code) assertThat(findings).hasSize(4) } } - context("using any other method on a range") { + @Nested + inner class `using any other method on a range` { val code = """ fun test() { (1..10).isEmpty() } """ - it("should not report any issues") { + @Test + fun `should not report any issues`() { val findings = subject.compileAndLint(code) assertThat(findings).isEmpty() } } - context("using a forEach on a list") { + @Nested + inner class `using a forEach on a list` { val code = """ fun test() { listOf(1, 2, 3).forEach { @@ -57,10 +63,11 @@ class ForEachOnRangeSpec : Spek({ } """ - it("should not report any issues") { + @Test + fun `should not report any issues`() { val findings = subject.compileAndLint(code) assertThat(findings).isEmpty() } } } -}) +} diff --git a/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/SpreadOperatorSpec.kt b/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/SpreadOperatorSpec.kt index 23ff9b6dd..6b0dc7df1 100644 --- a/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/SpreadOperatorSpec.kt +++ b/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/SpreadOperatorSpec.kt @@ -1,31 +1,33 @@ package io.gitlab.arturbosch.detekt.rules.performance -import io.gitlab.arturbosch.detekt.rules.setupKotlinEnvironment +import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest 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.spekframework.spek2.Spek -import org.spekframework.spek2.style.specification.describe +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test -class SpreadOperatorSpec : Spek({ - setupKotlinEnvironment() +@KotlinCoreEnvironmentTest +class SpreadOperatorSpec(val env: KotlinCoreEnvironment) { - val env: KotlinCoreEnvironment by memoized() - val subject by memoized { SpreadOperator() } + val subject = SpreadOperator() - describe("SpreadOperator rule") { + @Nested + inner class `SpreadOperator rule` { /** * This rule has different behaviour depending on whether type resolution is enabled in detekt or not. The two * `context` blocks are there to test behaviour when type resolution is enabled and type resolution is disabled * as different warning messages are shown in each case. */ - context("with type resolution") { + @Nested + inner class `with type resolution` { val typeResolutionEnabledMessage = "Used in this way a spread operator causes a full copy of the array to" + " be created before calling a method. This may result in a performance penalty." - it("reports when array copy required using named parameters") { + @Test + fun `reports when array copy required using named parameters`() { val code = """ val xsArray = intArrayOf(1) fun foo(vararg xs: Int) {} @@ -35,7 +37,9 @@ class SpreadOperatorSpec : Spek({ assertThat(actual).hasSize(1) assertThat(actual.first().message).isEqualTo(typeResolutionEnabledMessage) } - it("reports when array copy required without using named parameters") { + + @Test + fun `reports when array copy required without using named parameters`() { val code = """ val xsArray = intArrayOf(1) fun foo(vararg xs: Int) {} @@ -45,7 +49,9 @@ class SpreadOperatorSpec : Spek({ assertThat(actual).hasSize(1) assertThat(actual.first().message).isEqualTo(typeResolutionEnabledMessage) } - it("doesn't report when using array constructor with spread operator") { + + @Test + fun `doesn't report when using array constructor with spread operator`() { val code = """ fun foo(vararg xs: Int) {} val testVal = foo(xs = *intArrayOf(1)) @@ -53,7 +59,8 @@ class SpreadOperatorSpec : Spek({ assertThat(subject.compileAndLintWithContext(env, code)).isEmpty() } - it("doesn't report when using array constructor with spread operator when varargs parameter comes first") { + @Test + fun `doesn't report when using array constructor with spread operator when varargs parameter comes first`() { val code = """ fun asList(vararg ts: T, stringValue: String): List = listOf(1,2,3) val list = asList(-1, 0, *arrayOf(1, 2, 3), 4, stringValue = "5") @@ -61,7 +68,8 @@ class SpreadOperatorSpec : Spek({ assertThat(subject.compileAndLintWithContext(env, code)).isEmpty() } - it("doesn't report when passing values directly") { + @Test + fun `doesn't report when passing values directly`() { val code = """ fun asList(vararg ts: T, stringValue: String): List = listOf(1,2,3) val list = asList(-1, 0, 1, 2, 3, 4, stringValue = "5") @@ -69,7 +77,8 @@ class SpreadOperatorSpec : Spek({ assertThat(subject.compileAndLintWithContext(env, code)).isEmpty() } - it("doesn't report when function doesn't take a vararg parameter") { + @Test + fun `doesn't report when function doesn't take a vararg parameter`() { val code = """ fun test0(strs: Array) { test(strs) @@ -82,7 +91,8 @@ class SpreadOperatorSpec : Spek({ assertThat(subject.compileAndLintWithContext(env, code)).isEmpty() } - it("doesn't report with expression inside params") { + @Test + fun `doesn't report with expression inside params`() { val code = """ fun test0(strs: Array) { test(2*2) @@ -95,7 +105,8 @@ class SpreadOperatorSpec : Spek({ assertThat(subject.compileAndLintWithContext(env, code)).isEmpty() } - it("respects pass through of vararg parameter - #3145") { + @Test + fun `respects pass through of vararg parameter - #3145`() { val code = """ fun b(vararg bla: Int) = Unit fun a(vararg bla: Int) { @@ -105,7 +116,8 @@ class SpreadOperatorSpec : Spek({ assertThat(subject.compileAndLintWithContext(env, code)).isEmpty() } - it("reports shadowed vararg declaration which may lead to array copy - #3145") { + @Test + fun `reports shadowed vararg declaration which may lead to array copy - #3145`() { val code = """ fun b(vararg bla: String) = Unit @@ -118,12 +130,14 @@ class SpreadOperatorSpec : Spek({ } } - context("without type resolution") { + @Nested + inner class `without type resolution` { val typeResolutionDisabledMessage = "In most cases using a spread operator causes a full copy of the " + "array to be created before calling a method. This may result in a performance penalty." - it("reports when array copy required using named parameters") { + @Test + fun `reports when array copy required using named parameters`() { val code = """ val xsArray = intArrayOf(1) fun foo(vararg xs: Int) {} @@ -133,7 +147,9 @@ class SpreadOperatorSpec : Spek({ assertThat(actual).hasSize(1) assertThat(actual.first().message).isEqualTo(typeResolutionDisabledMessage) } - it("reports when array copy required without using named parameters") { + + @Test + fun `reports when array copy required without using named parameters`() { val code = """ val xsArray = intArrayOf(1) fun foo(vararg xs: Int) {} @@ -143,7 +159,9 @@ class SpreadOperatorSpec : Spek({ assertThat(actual).hasSize(1) assertThat(actual.first().message).isEqualTo(typeResolutionDisabledMessage) } - it("doesn't report when using array constructor with spread operator") { + + @Test + fun `doesn't report when using array constructor with spread operator`() { val code = """ fun foo(vararg xs: Int) {} val testVal = foo(xs = *intArrayOf(1)) @@ -153,7 +171,8 @@ class SpreadOperatorSpec : Spek({ assertThat(actual.first().message).isEqualTo(typeResolutionDisabledMessage) } - it("doesn't report when using array constructor with spread operator when varargs parameter comes first") { + @Test + fun `doesn't report when using array constructor with spread operator when varargs parameter comes first`() { val code = """ fun asList(vararg ts: T, stringValue: String): List = listOf(1,2,3) val list = asList(-1, 0, *arrayOf(1, 2, 3), 4, stringValue = "5") @@ -163,7 +182,8 @@ class SpreadOperatorSpec : Spek({ assertThat(actual.first().message).isEqualTo(typeResolutionDisabledMessage) } - it("doesn't report when passing values directly") { + @Test + fun `doesn't report when passing values directly`() { val code = """ fun asList(vararg ts: T, stringValue: String): List = listOf(1,2,3) val list = asList(-1, 0, 1, 2, 3, 4, stringValue = "5") @@ -171,7 +191,8 @@ class SpreadOperatorSpec : Spek({ assertThat(subject.compileAndLint(code)).isEmpty() } - it("doesn't report when function doesn't take a vararg parameter") { + @Test + fun `doesn't report when function doesn't take a vararg parameter`() { val code = """ fun test0(strs: Array) { test(strs) @@ -184,7 +205,8 @@ class SpreadOperatorSpec : Spek({ assertThat(subject.compileAndLint(code)).isEmpty() } - it("doesn't report with expression inside params") { + @Test + fun `doesn't report with expression inside params`() { val code = """ fun test0(strs: Array) { test(2*2) @@ -197,7 +219,8 @@ class SpreadOperatorSpec : Spek({ assertThat(subject.compileAndLint(code)).isEmpty() } - it("respects pass through of vararg parameter - #3145") { + @Test + fun `respects pass through of vararg parameter - #3145`() { val code = """ fun b(vararg bla: Int) = Unit fun a(vararg bla: Int) { @@ -207,7 +230,8 @@ class SpreadOperatorSpec : Spek({ assertThat(subject.compileAndLint(code)).isEmpty() } - it("does not report shadowed vararg declaration, we except this false negative here - #3145") { + @Test + fun `does not report shadowed vararg declaration, we except this false negative here - #3145`() { val code = """ fun b(vararg bla: String) = Unit @@ -220,4 +244,4 @@ class SpreadOperatorSpec : Spek({ } } } -}) +} diff --git a/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/UnnecessaryTemporaryInstantiationSpec.kt b/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/UnnecessaryTemporaryInstantiationSpec.kt index a3e9ee272..f82c70b7f 100644 --- a/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/UnnecessaryTemporaryInstantiationSpec.kt +++ b/detekt-rules-performance/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/performance/UnnecessaryTemporaryInstantiationSpec.kt @@ -2,22 +2,25 @@ package io.gitlab.arturbosch.detekt.rules.performance import io.gitlab.arturbosch.detekt.test.compileAndLint 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.api.Test -class UnnecessaryTemporaryInstantiationSpec : Spek({ - val subject by memoized { UnnecessaryTemporaryInstantiation() } +class UnnecessaryTemporaryInstantiationSpec { + val subject = UnnecessaryTemporaryInstantiation() - describe("UnnecessaryTemporaryInstantiation rule") { + @Nested + inner class `UnnecessaryTemporaryInstantiation rule` { - it("temporary instantiation for conversion") { + @Test + fun `temporary instantiation for conversion`() { val code = "val i = Integer(1).toString()" assertThat(subject.compileAndLint(code)).hasSize(1) } - it("right conversion without instantiation") { + @Test + fun `right conversion without instantiation`() { val code = "val i = Integer.toString(1)" assertThat(subject.compileAndLint(code)).isEmpty() } } -}) +}