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

Add Kover plugin and set up basic rules #35

Merged
merged 3 commits into from
Sep 12, 2022
+11 −0
Merged

Add Kover plugin and set up basic rules #35

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
818429f
Add `Kover` plugin and set up basic rules
IgnatBeresnev Sep 5, 2022
c35306f
Prefer IntelliJ engine and raise the rule bound bar
IgnatBeresnev Sep 9, 2022
1f54ec3
Remove unused import
IgnatBeresnev Sep 9, 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
15 changes: 15 additions & 0 deletions build.gradle.kts
View file
Original file line number Diff line number Diff line change
@@ -2,13 +2,16 @@
* Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
*/

import kotlinx.kover.api.DefaultJacocoEngine

plugins {
kotlin("jvm")
`java-gradle-plugin`
`maven-publish`
signing
id("org.jetbrains.kotlinx.binary-compatibility-validator")
id("me.champeau.jmh") version "0.6.7"
id("org.jetbrains.kotlinx.kover") version "0.6.0"
}

repositories {
@@ -44,6 +47,18 @@ kotlin {
explicitApi()
}

kover {
verify {
// JaCoCo engine instead of IntelliJ for better integration with codecov.io
engine.set(DefaultJacocoEngine)
rule {
bound {
minValue = 60
}
}
Copy link
Member Author

@IgnatBeresnev IgnatBeresnev Sep 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

JaCoCo engine is required for integration with codecov, as IntelliJ engine does not seem to be supported (Kotlin/kotlinx-kover#16). The downside here is that we're not dogfooding our engine, and reported values of these two engines might differ.

As an alternative I can add two tasks: one for IntelliJ engine, and another one for JaCoCo, but it can lead to more inconsistencies.

Not sure what the best practices are for setting rule bounds, right now I set it to 60% line coverage, which should give 10-15% of headroom before build failures (currently it's at 70-75% lines covered, depending on the engine). This should both catch big PRs without tests and overtime degradation in coverage, but not be annoying.

Can add additional bounds for instructions/branches/etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest sticking to the default IDEA engine. Multiple reasons for that:

  • AFAIR, only multiplatform projects are not supported by Kover in codecov. There are JVM-only projects that successfully leverage Kover and codecov
  • The default engine is more precise when measuring Kotlin coverage, esp. in some tricky places such as inline functions. We would like to use these capabilities

IMO, if codecov is not working in this setup, my vote is for giving up the codecov until it's supported.

Not sure what the best practices are for setting rule bounds, right now I set it to 60% line coverage

The reasonable lower bound is somewhere around 80-90% depending on the tool and what is not covered (e.g. in coroutines it's mostly toString implementations).
I would suggest picking 75 if tests pass, no need to set the bar any lower. Any new API additional will force authors to write proper coverage and eventually we'll be able to raise this bar.

Copy link
Member Author

Choose a reason for hiding this comment

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

IMO, if codecov is not working in this setup, my vote is for giving up the codecov until it's supported.

Maybe I'm doing something wrong, but xml reports generated by IntelliJ engine are rendered as empty if uploaded to codecov. Removed JaCoCo engine and will postpone codecov then

I would suggest picking 75 if tests pass, no need to set the bar any lower. Any new API additional will force authors to write proper coverage and eventually we'll be able to raise this bar.

Set it to 75 (current line coverage is 77.8). My concern is that it might be impossible or not rational to cover 100% of lines for every PR/feature, while in other situations you actually cover more than just your changes, so it evens out over time.

Having the bound be too close to the current coverage might lead to situations where you'd need to write tests for some other parts of code (not directly changed by you) in order to have high enough coverage so it passes the build.

But I guess nothing is set in stone, so it can be adjusted if the need arises 👍

}
}

val deployVersion = properties.get("DeployVersion")?.toString() ?: version.toString()

publishing {