Used compile dependency to Kover Features in Kover Gradle Plugin #574

shanshin · 2024-03-21T11:11:50Z

Since the version of the aggregator and report generator has been enshrined in the Kover Gradle Plugin for a long time, it makes no sense to use dynamic classpath of and classloader isolations in Gradle Workers.

We can use Kover Features as a compile dependency and use its API.

This change can also eliminate the error with access to Freemarker template inside Gradle Worker, and simplify the implementation of warn on verification error.

Resolves #567

Since the version of the aggregator and report generator has been enshrined in the Kover Gradle Plugin for a long time, it makes no sense to use dynamic classpath of and classloader isolations in Gradle Workers. We can use Kover Features as a compile dependency and use its API. This change can also eliminate the error with access to Freemarker template inside Gradle Worker, and simplify the implementation of warn on verification error. Resolves #567

sandwwraith · 2024-03-27T11:30:37Z

kover-features-jvm/src/main/java/kotlinx/kover/features/jvm/KoverFeatures.java

+     * @param template Template string in Kover format
+     * @return Regular expression corresponding given Kover template
+     */
+    public static String koverWildcardToRegex(String template) {


Why this function has to be in the public API? Are users of Kover Features (besides our plugin) supposed to call it?

It's also used in CLI, and I don't see a reason why the user shouldn't be able to use this function - these are not implementation details.

sandwwraith · 2024-03-27T11:32:24Z

kover-features-jvm/src/main/java/kotlinx/kover/features/jvm/KoverLegacyFeatures.java

-        ReportApi.htmlReport(htmlDir, title, null, binaryReports, classfileDirs, sourceDirs, convertFilters(filters));
+        // repeat reading freemarker temple from resources if error occurred, see https://github.com/Kotlin/kotlinx-kover/issues/510
+        // the values are selected empirically so that the maximum report generation time is not much more than a second
+        ReportApi.setFreemarkerRetry(7, 150);


You said that 'This change can also eliminate the error with access to Freemarker template inside Gradle Worker', so maybe we do not need a lot of retries anymore?

Ok, let's try )

sandwwraith · 2024-03-27T11:37:41Z

kover-features-jvm/src/main/java/kotlinx/kover/features/jvm/KoverLegacyFeatures.java

+    /**
+     * Describes a single bound for the verification rule to enforce
+     */
+    public static class Bound {


These classes were data in Kotlin. Should we add toString() + equals() here?

It's data class again )

sandwwraith · 2024-03-27T11:42:16Z

kover-features-jvm/src/main/java/kotlinx/kover/features/jvm/KoverLegacyFeatures.java

+    /**
+     * Violation of verification bound.
+     */
+    public static class BoundViolation {


I think toString() will be especially beneficial for *Violation classes in case someone would like to print list with results

It's data class again )

kover-features-jvm/build.gradle.kts

kover-features-jvm/src/main/java/kotlinx/kover/features/jvm/KoverFeatures.kt

kover-features-jvm/src/main/java/kotlinx/kover/features/jvm/KoverLegacyFeatures.kt

sandwwraith · 2024-03-27T15:39:19Z

kover-features-jvm/src/main/java/kotlinx/kover/features/jvm/KoverLegacyFeatures.kt

+    /**
+     * Verify coverage by specified verification rules.
+     *
+     * @param tempDir       Directory to create temporary files


@param rules is undocumented

sandwwraith · 2024-03-27T15:45:52Z

kover-features-jvm/src/main/java/kotlinx/kover/features/jvm/Verification.kt

+    MISSED_PERCENTAGE
+}
+
+public data class CoverageValue(val entityName: String?, val value: BigDecimal)


Undocumented

sandwwraith · 2024-03-27T15:52:44Z

kover-features-jvm/src/main/java/kotlinx/kover/features/jvm/impl/LegacyVerification.kt

+        for (ruleIndex in rules.indices) {
+            val rule = rules[ruleIndex]
+
+            val bounds: MutableList<com.intellij.rt.coverage.verify.api.Bound> = ArrayList()


Suggested change

val bounds: MutableList<com.intellij.rt.coverage.verify.api.Bound> = ArrayList()

val bounds: MutableList<IntellijBound> = ArrayList()

sandwwraith · 2024-03-27T15:55:15Z

kover-features-jvm/src/main/java/kotlinx/kover/features/jvm/impl/LegacyVerification.kt

+        return aggregationType == AggregationType.COVERED_PERCENTAGE || aggregationType == AggregationType.MISSED_PERCENTAGE
+    }
+
+    private class ViolationId(private val index: Int, private val entityName: String?) : Comparable<ViolationId> {


TreeMap only requires Comparable, but shouldn't we add equals/hashCode anyway just in case? Or simply make this class data?

Because TreeMap guarantees order, in this case, the validation result will always be stable and predictable.

Yes. I'm talking about making it data class while also being Comparable in case we want to use it tostring or equals for e.g. testing purposes

sandwwraith · 2024-03-27T15:58:27Z

kover-gradle-plugin/src/main/kotlin/kotlinx/kover/gradle/plugin/tools/Verification.kt

-        CoverageUnit.INSTRUCTION -> "instructions"
-        CoverageUnit.BRANCH -> "branches"
+    val metricText = when (bound.coverageUnits) {
+        kotlinx.kover.features.jvm.CoverageUnit.LINE -> "lines"


Redundant fq import?

Create alias

sandwwraith · 2024-03-27T16:01:33Z

...e-plugin/src/main/kotlin/kotlinx/kover/gradle/plugin/tools/kover/KoverFeaturesIntegration.kt

+
+internal fun CoverageUnit.convert(): kotlinx.kover.features.jvm.CoverageUnit {
+    return when (this) {
+        CoverageUnit.LINE -> kotlinx.kover.features.jvm.CoverageUnit.LINE


Can't we use classes from kotlinx.kover.features.jvm. directly?

Create alias

sandwwraith · 2024-03-27T18:04:04Z

kover-features-jvm/build.gradle.kts

+            jvmTarget.set(JvmTarget.JVM_1_8)
+            languageVersion.set(KotlinVersion.KOTLIN_1_5)
+            apiVersion.set(KotlinVersion.KOTLIN_1_5)
+            freeCompilerArgs.add("-Xsuppress-version-warnings")


Should we use also -Xjdk-release here?

I'm not sure if this is necessary, the class files are now 52 versions.
I think jvmTarget.set(JvmTarget.JVM_1_8) is enough

It's not about a class file version, it's about JDK API. To prevent errors like this one: Kotlin/kotlinx.serialization#2328

Release flag of Javac should be used for the same reason (https://docs.gradle.org/current/userguide/building_java_projects.html#sec:compiling_with_release)

OK, let it be

kover-features-jvm/src/main/java/kotlinx/kover/features/jvm/KoverLegacyFeatures.kt

shanshin requested a review from sandwwraith March 21, 2024 11:11

sandwwraith reviewed Mar 27, 2024

View reviewed changes

shanshin added 2 commits March 27, 2024 16:09

Rename .java to .kt

37bbae3

~translate features to Kotlin

075ac6f

shanshin requested a review from sandwwraith March 27, 2024 15:10

sandwwraith requested changes Mar 27, 2024

View reviewed changes

~review fixes

56bf463

shanshin force-pushed the features-instead-intellij branch from 779e192 to 56bf463 Compare March 27, 2024 16:54

shanshin requested a review from sandwwraith March 27, 2024 16:54

sandwwraith reviewed Mar 27, 2024

View reviewed changes

~small fixes

721c753

shanshin requested a review from sandwwraith March 27, 2024 18:18

shanshin force-pushed the features-instead-intellij branch from c449c95 to 6e49c2e Compare March 27, 2024 18:26

~JDK version

198a96d

shanshin force-pushed the features-instead-intellij branch from 6e49c2e to 198a96d Compare March 27, 2024 18:28

sandwwraith approved these changes Mar 27, 2024

View reviewed changes

shanshin merged commit 8af377c into main Mar 27, 2024

shanshin deleted the features-instead-intellij branch March 27, 2024 18:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Used compile dependency to Kover Features in Kover Gradle Plugin #574

Used compile dependency to Kover Features in Kover Gradle Plugin #574

shanshin commented Mar 21, 2024

sandwwraith Mar 27, 2024

shanshin Mar 27, 2024

sandwwraith Mar 27, 2024

shanshin Mar 27, 2024

sandwwraith Mar 27, 2024

shanshin Mar 27, 2024

sandwwraith Mar 27, 2024

shanshin Mar 27, 2024

sandwwraith Mar 27, 2024

sandwwraith Mar 27, 2024

sandwwraith Mar 27, 2024

sandwwraith Mar 27, 2024

shanshin Mar 27, 2024

sandwwraith Mar 27, 2024

sandwwraith Mar 27, 2024

shanshin Mar 27, 2024

sandwwraith Mar 27, 2024

shanshin Mar 27, 2024

sandwwraith Mar 27, 2024

shanshin Mar 27, 2024

sandwwraith Mar 27, 2024

sandwwraith Mar 27, 2024

shanshin Mar 27, 2024

	val bounds: MutableList<com.intellij.rt.coverage.verify.api.Bound> = ArrayList()
	val bounds: MutableList<IntellijBound> = ArrayList()

Used compile dependency to Kover Features in Kover Gradle Plugin #574

Used compile dependency to Kover Features in Kover Gradle Plugin #574

Conversation

shanshin commented Mar 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment