From 45a0bb661465bcd246b9d4f0bb7a0401360cbb0e Mon Sep 17 00:00:00 2001 From: Gianluca Zuddas Date: Sun, 13 Jun 2021 17:48:14 +0100 Subject: [PATCH 1/7] Coroutines compatibility test --- Dangerfile_ci.df.kts | 22 ++++++++++++++++++++++ dependencyVersions.gradle | 7 +++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/Dangerfile_ci.df.kts b/Dangerfile_ci.df.kts index 8ec78b40..83d4fb2b 100644 --- a/Dangerfile_ci.df.kts +++ b/Dangerfile_ci.df.kts @@ -8,9 +8,14 @@ //Testing plugin @file:DependsOn("danger-kotlin-sample-plugin-sample.jar") +import kotlinx.coroutines.runBlocking +import kotlinx.datetime.Clock +import kotlinx.datetime.toDateTimePeriod import org.apache.commons.text.WordUtils import systems.danger.kotlin.* +import systems.danger.kotlin.models.danger.DangerDSL import systems.danger.samples.plugin.SamplePlugin +import kotlin.time.Duration register plugin SamplePlugin @@ -49,4 +54,21 @@ danger(args) { warn(WordUtils.capitalize("please consider to create new files in Kotlin"), it, 1) } } + + // Coroutines checks in parallel test + val current = Clock.System.now() + runBlocking { + expensiveCheck(1000) + expensiveCheck(3000) + expensiveCheck(2000) + expensiveCheck(5000) + } + val after = Clock.System.now() + val runningTime = after.minus(current) + message("Coroutines checks terminated in ${runningTime.toDateTimePeriod().seconds} seconds.") +} + +suspend fun DangerDSL.expensiveCheck(runForMillis: Long) { + // Example expensive check + Thread.sleep(runForMillis) } diff --git a/dependencyVersions.gradle b/dependencyVersions.gradle index 80c70ca5..1948e0df 100644 --- a/dependencyVersions.gradle +++ b/dependencyVersions.gradle @@ -26,18 +26,21 @@ project.ext.groupIdKotlinx = 'org.jetbrains.kotlinx' project.ext.artifactIdKotlinMain = 'kotlin-main-kts' project.ext.artifactIdKotlinSerializationJson = 'kotlinx-serialization-json' project.ext.artifactIdKotlinDatetime = "kotlinx-datetime" +project.ext.artifactIdKotlinCoroutines = "kotlinx-coroutines-core" project.ext.versionKotlinSerializationJson = '1.0.1' -project.ext.versionKotlinDatetime = '0.1.1' +project.ext.versionKotlinDatetime = '0.2.0' ext.kotlin = [ mainKts: "$groupIdKotlin:$artifactIdKotlinMain:$versionKotlin", serialization: "$groupIdKotlinx:$artifactIdKotlinSerializationJson:$versionKotlinSerializationJson", - datetime: "$groupIdKotlinx:$artifactIdKotlinDatetime:$versionKotlinDatetime" + datetime: "$groupIdKotlinx:$artifactIdKotlinDatetime:$versionKotlinDatetime", + coroutines: "$groupIdKotlinx:$artifactIdKotlinCoroutines:$versionKotlin" ] ext.kotlinDependencies = group { includeJar kotlin.mainKts includeRecursiveJar kotlin.serialization includeRecursiveJar kotlin.datetime + includeRecursiveJar kotlin.coroutines } // Testing From 911e3893208fab8a3c169b0f4f18d1bee2f70ce2 Mon Sep 17 00:00:00 2001 From: Gianluca Zuddas Date: Sun, 13 Jun 2021 18:34:13 +0100 Subject: [PATCH 2/7] Update also kotlin to 1.5.10 and related dependencies --- .github/workflows/ci.yml | 2 +- Dangerfile_ci.df.kts | 2 +- dependencyVersions.gradle | 7 ++++--- gradle.properties | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e8eb1ebf..8bb75689 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,7 +20,7 @@ jobs: - name: Install Kotlin run: | - curl -o kotlin-compiler.zip -L https://github.com/JetBrains/kotlin/releases/download/v1.4.31/kotlin-compiler-1.4.31.zip + curl -o kotlin-compiler.zip -L https://github.com/JetBrains/kotlin/releases/download/v1.5.10/kotlin-compiler-1.5.10.zip if [[ "$OSTYPE" != "darwin"* ]] then diff --git a/Dangerfile_ci.df.kts b/Dangerfile_ci.df.kts index 83d4fb2b..f0ae62ef 100644 --- a/Dangerfile_ci.df.kts +++ b/Dangerfile_ci.df.kts @@ -65,7 +65,7 @@ danger(args) { } val after = Clock.System.now() val runningTime = after.minus(current) - message("Coroutines checks terminated in ${runningTime.toDateTimePeriod().seconds} seconds.") + message("Coroutines checks terminated - runningFor $runningTime") } suspend fun DangerDSL.expensiveCheck(runForMillis: Long) { diff --git a/dependencyVersions.gradle b/dependencyVersions.gradle index 1948e0df..606be4c0 100644 --- a/dependencyVersions.gradle +++ b/dependencyVersions.gradle @@ -20,21 +20,22 @@ project.ext.groupIdOkio = 'com.squareup.okio' project.ext.artifactIdOkio = 'okio' // Kotlin -project.ext.versionKotlin = '1.5.0' +project.ext.versionKotlin = '1.5.10' project.ext.groupIdKotlin = 'org.jetbrains.kotlin' project.ext.groupIdKotlinx = 'org.jetbrains.kotlinx' project.ext.artifactIdKotlinMain = 'kotlin-main-kts' project.ext.artifactIdKotlinSerializationJson = 'kotlinx-serialization-json' project.ext.artifactIdKotlinDatetime = "kotlinx-datetime" project.ext.artifactIdKotlinCoroutines = "kotlinx-coroutines-core" -project.ext.versionKotlinSerializationJson = '1.0.1' +project.ext.versionKotlinSerializationJson = '1.2.1' project.ext.versionKotlinDatetime = '0.2.0' +project.ext.versionKotlinCoroutines = '1.5.0' ext.kotlin = [ mainKts: "$groupIdKotlin:$artifactIdKotlinMain:$versionKotlin", serialization: "$groupIdKotlinx:$artifactIdKotlinSerializationJson:$versionKotlinSerializationJson", datetime: "$groupIdKotlinx:$artifactIdKotlinDatetime:$versionKotlinDatetime", - coroutines: "$groupIdKotlinx:$artifactIdKotlinCoroutines:$versionKotlin" + coroutines: "$groupIdKotlinx:$artifactIdKotlinCoroutines:$versionKotlinCoroutines" ] ext.kotlinDependencies = group { includeJar kotlin.mainKts diff --git a/gradle.properties b/gradle.properties index a598c80b..f02be2af 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,3 +1,3 @@ -kotlinVersion=1.5.0 +kotlinVersion=1.5.10 kotlin.code.style=official systemProp.org.gradle.internal.publish.checksums.insecure=true From a8059c0942a4f73cd7c3d5f09238ecdc2e1432c6 Mon Sep 17 00:00:00 2001 From: Gianluca Zuddas Date: Sun, 13 Jun 2021 18:44:15 +0100 Subject: [PATCH 3/7] Testing coroutines with delay rather than Thread.sleep --- Dangerfile_ci.df.kts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Dangerfile_ci.df.kts b/Dangerfile_ci.df.kts index f0ae62ef..4913e693 100644 --- a/Dangerfile_ci.df.kts +++ b/Dangerfile_ci.df.kts @@ -8,14 +8,13 @@ //Testing plugin @file:DependsOn("danger-kotlin-sample-plugin-sample.jar") +import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking import kotlinx.datetime.Clock -import kotlinx.datetime.toDateTimePeriod import org.apache.commons.text.WordUtils import systems.danger.kotlin.* import systems.danger.kotlin.models.danger.DangerDSL import systems.danger.samples.plugin.SamplePlugin -import kotlin.time.Duration register plugin SamplePlugin @@ -70,5 +69,5 @@ danger(args) { suspend fun DangerDSL.expensiveCheck(runForMillis: Long) { // Example expensive check - Thread.sleep(runForMillis) + delay(runForMillis) } From d4bc2ad236b0563594022c42dd7ad06f1312c459 Mon Sep 17 00:00:00 2001 From: Gianluca Zuddas Date: Sun, 13 Jun 2021 19:48:27 +0100 Subject: [PATCH 4/7] Test checks going asynch --- Dangerfile_ci.df.kts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/Dangerfile_ci.df.kts b/Dangerfile_ci.df.kts index 4913e693..63e922cf 100644 --- a/Dangerfile_ci.df.kts +++ b/Dangerfile_ci.df.kts @@ -8,6 +8,7 @@ //Testing plugin @file:DependsOn("danger-kotlin-sample-plugin-sample.jar") +import kotlinx.coroutines.async import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking import kotlinx.datetime.Clock @@ -57,10 +58,14 @@ danger(args) { // Coroutines checks in parallel test val current = Clock.System.now() runBlocking { - expensiveCheck(1000) - expensiveCheck(3000) - expensiveCheck(2000) - expensiveCheck(5000) + val task1 = async { expensiveCheck(1000) } + val task2 = async { expensiveCheck(3000) } + val task3 = async { expensiveCheck(2000) } + val task4 = async { expensiveCheck(5000) } + task1.await() + task2.await() + task3.await() + task4.await() } val after = Clock.System.now() val runningTime = after.minus(current) From c2aa2c9110a923e3e55dc94a89b8b0e68ce54b23 Mon Sep 17 00:00:00 2001 From: Gianluca Zuddas Date: Mon, 14 Jun 2021 09:09:34 +0100 Subject: [PATCH 5/7] Protecting violations and json output file from concurrent access --- Dangerfile_ci.df.kts | 11 +++--- .../systems/danger/kotlin/MainDangerRunner.kt | 36 +++++++++++-------- .../kotlin/models/danger/DangerResults.kt | 20 ++++++++--- 3 files changed, 44 insertions(+), 23 deletions(-) diff --git a/Dangerfile_ci.df.kts b/Dangerfile_ci.df.kts index 63e922cf..fa230450 100644 --- a/Dangerfile_ci.df.kts +++ b/Dangerfile_ci.df.kts @@ -58,10 +58,10 @@ danger(args) { // Coroutines checks in parallel test val current = Clock.System.now() runBlocking { - val task1 = async { expensiveCheck(1000) } - val task2 = async { expensiveCheck(3000) } - val task3 = async { expensiveCheck(2000) } - val task4 = async { expensiveCheck(5000) } + val task1 = async { expensiveCheck("1", 1000) } + val task2 = async { expensiveCheck("2", 3000) } + val task3 = async { expensiveCheck("3", 2000) } + val task4 = async { expensiveCheck("4", 5000) } task1.await() task2.await() task3.await() @@ -72,7 +72,8 @@ danger(args) { message("Coroutines checks terminated - runningFor $runningTime") } -suspend fun DangerDSL.expensiveCheck(runForMillis: Long) { +suspend fun DangerDSL.expensiveCheck(name: String, runForMillis: Long) { // Example expensive check delay(runForMillis) + message("Coroutine $name terminated in $runForMillis ms") } diff --git a/danger-kotlin-library/src/main/kotlin/systems/danger/kotlin/MainDangerRunner.kt b/danger-kotlin-library/src/main/kotlin/systems/danger/kotlin/MainDangerRunner.kt index f11632fd..5b5cbbe9 100644 --- a/danger-kotlin-library/src/main/kotlin/systems/danger/kotlin/MainDangerRunner.kt +++ b/danger-kotlin-library/src/main/kotlin/systems/danger/kotlin/MainDangerRunner.kt @@ -1,13 +1,14 @@ package systems.danger.kotlin import systems.danger.kotlin.json.JsonParser +import systems.danger.kotlin.models.danger.ConcurrentDangerResults import systems.danger.kotlin.models.danger.DSL import systems.danger.kotlin.models.danger.DangerDSL -import systems.danger.kotlin.models.danger.DangerResults import systems.danger.kotlin.models.git.FilePath import systems.danger.kotlin.sdk.DangerContext import systems.danger.kotlin.sdk.Violation import java.io.File +import java.util.concurrent.atomic.AtomicReference /** * Main Danger runner @@ -23,23 +24,23 @@ internal class MainDangerRunner(jsonInputFilePath: FilePath, jsonOutputPath: Fil val danger: DangerDSL = JsonParser.decodeJson(jsonInputFilePath).danger - private val dangerResults: DangerResults = DangerResults() + private val concurrentDangerResults: ConcurrentDangerResults = ConcurrentDangerResults() override val fails: List get() { - return dangerResults.fails.toList() + return concurrentDangerResults.fails.get() } override val warnings: List get() { - return dangerResults.warnings.toList() + return concurrentDangerResults.warnings.get() } override val messages: List get() { - return dangerResults.messages.toList() + return concurrentDangerResults.messages.get() } override val markdowns: List get() { - return dangerResults.markdowns.toList() + return concurrentDangerResults.markdowns.get() } @@ -95,27 +96,34 @@ internal class MainDangerRunner(jsonInputFilePath: FilePath, jsonOutputPath: Fil } private fun warn(violation: Violation) { - dangerResults.warnings.add(violation) - commit() + concurrentDangerResults.warnings.updateWith(violation) } private fun fail(violation: Violation) { - dangerResults.fails.add(violation) - commit() + concurrentDangerResults.fails.updateWith(violation) } private fun message(violation: Violation) { - dangerResults.messages.add(violation) - commit() + concurrentDangerResults.messages.updateWith(violation) } private fun markdown(violation: Violation) { - dangerResults.markdowns.add(violation) + concurrentDangerResults.markdowns.updateWith(violation) + } + + private fun AtomicReference>.updateWith(violation: Violation) { + this.getAndUpdate { + it.apply { + add(violation) + } + } commit() } // commit all the inline violations into the json output file private fun commit() { - JsonParser.encodeJson(dangerResults, jsonOutputFile) + synchronized(this) { + JsonParser.encodeJson(concurrentDangerResults.toDangerResults(), jsonOutputFile) + } } } diff --git a/danger-kotlin-library/src/main/kotlin/systems/danger/kotlin/models/danger/DangerResults.kt b/danger-kotlin-library/src/main/kotlin/systems/danger/kotlin/models/danger/DangerResults.kt index a86fd55e..42440cea 100644 --- a/danger-kotlin-library/src/main/kotlin/systems/danger/kotlin/models/danger/DangerResults.kt +++ b/danger-kotlin-library/src/main/kotlin/systems/danger/kotlin/models/danger/DangerResults.kt @@ -6,6 +6,7 @@ import kotlinx.serialization.Serializable import kotlinx.serialization.UseSerializers import systems.danger.kotlin.sdk.Violation import systems.danger.kotlin.models.serializers.ViolationSerializer +import java.util.concurrent.atomic.AtomicReference @Serializable internal data class Meta( @@ -15,9 +16,20 @@ internal data class Meta( @Serializable internal data class DangerResults( - var fails: MutableList = mutableListOf(), - var warnings: MutableList = mutableListOf(), - var messages: MutableList = mutableListOf(), - var markdowns: MutableList = mutableListOf(), + val fails: List, + val warnings: List, + val messages: List, + val markdowns: List, val meta: Meta = Meta() ) + +internal data class ConcurrentDangerResults( + val fails: AtomicReference> = AtomicReference(mutableListOf()), + val warnings: AtomicReference> = AtomicReference(mutableListOf()), + val messages: AtomicReference> = AtomicReference(mutableListOf()), + val markdowns: AtomicReference> = AtomicReference(mutableListOf()) +) { + fun toDangerResults() = DangerResults( + fails.get(), warnings.get(), messages.get(), markdowns.get() + ) +} From e6916b744f5f863cad763933a51d76afb7e0cbdf Mon Sep 17 00:00:00 2001 From: Gianluca Zuddas Date: Wed, 1 Sep 2021 17:14:31 +0100 Subject: [PATCH 6/7] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 512c76a1..8a05ec45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ --> ## Master +- Coroutines compatibility [@gianluz] - [#177](https://github.com/danger/kotlin/pull/177) - Improving error message for when a DangerPlugin was not registered [@rojanthomas] - [#181](https://github.com/danger/kotlin/pull/181) - Fix body parameter in github models [@tegorov] - [#175](https://github.com/danger/kotlin/pull/175) From 25a242bc175d9b6d82e09380695bf4f2f19635bf Mon Sep 17 00:00:00 2001 From: Franco Meloni Date: Mon, 30 Aug 2021 20:50:20 +0100 Subject: [PATCH 7/7] Don't unpack kotlin on mac --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8bb75689..c3e91ad8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,11 +25,11 @@ jobs: if [[ "$OSTYPE" != "darwin"* ]] then sudo chmod -R a+rwx /usr/local/ + + unzip -d /usr/local/bin kotlin-compiler.zip + echo "/usr/local/bin/kotlinc/bin" >> $GITHUB_PATH + rm -rf kotlin-compiler.zip fi - - unzip -d /usr/local/bin kotlin-compiler.zip - echo "/usr/local/bin/kotlinc/bin" >> $GITHUB_PATH - rm -rf kotlin-compiler.zip - uses: actions/setup-node@v2 with: