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.

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

Multiple issues when migrating from custom plugin to kotlinx-kover #398

Closed
realdadfish opened this issue Jun 1, 2023 · 34 comments
Closed
Assignees
Labels
Bug Bug issue type S: in progress Status: implementing or design in process

Comments

@realdadfish
Copy link

realdadfish commented Jun 1, 2023

Describe the bug
I have had a custom build convention plugin with which I set up code coverage for my Gradle apps. Upon getting the news that kotlinx-kover exists I decided to replace my custom implementation with one that internally configures / uses kotlinx-kover.

Upon this I encountered several issues:

  1. In Android projects, the non-variant-aware tasks koverVerify, koverHtmlReport, ... are created, though they're useless and result in being skipped when executed, because there are no tests executed (naturally):

    > Task :mymodule:koverHtmlReport SKIPPED
    Task 'koverHtmlReport' will be skipped because no tests were executed
    

    I think it would be better if the creation of these tasks would either be skipped or if they would act as aggegration tasks, i.e. koverHtmlReport would execute all koverHtmlReport tasks of all active / configured build variants

  2. When I look at the HTML report after all tests have been executed and coverage has been calculated, the Jacoco report tells me only 22% of my example module are covered, whereas previously I had more than 80% coverage. When I drill down into the details I see that especially Robolectric-based UI tests get no coverage reported at all. Are there any special settings needed to get the coverage for those tests as well?

  3. koverXmlReport / koverVerify seems to stall indefinitely. When I connect with the Gradle debugger I see that there are dozens of "Unconstrained build operation" threads that are all in parked state. In smaller modules it eventually seems to finish the tasks after some time, but then the configured thresholds (upper and lower) don't seem to let the build fail; instead the verification task always runs green and is even cached afterwards when I try to execute it again.

This is my configuration (stripped to the basic minimum):

import com.android.build.gradle.api.AndroidBasePlugin
import kotlinx.kover.gradle.plugin.dsl.AggregationType
import kotlinx.kover.gradle.plugin.dsl.GroupingEntityType
import kotlinx.kover.gradle.plugin.dsl.MetricType

plugins {
    id("org.jetbrains.kotlinx.kover")
}

val coverageExtension = project.extensions.create(
    "codeCoverage", CoverageExtension::class.java
)

plugins.withId("org.jetbrains.kotlin.jvm") {
    configureProject(coverageExtension)
}
plugins.withType(AndroidBasePlugin::class.java) {
    configureProject(coverageExtension)
    configureAndroidProject(coverageExtension)
}

fun configureProject(extension: CoverageExtension) {
    kover {
        useJacoco("0.8.8")
    }
    koverReport {
        filters {
            excludes {
                classes(extension.classFilter)
            }
        }
        defaults {
            verify {
                onCheck = true
                rule {
                    isEnabled = true
                    entity = GroupingEntityType.APPLICATION
                    bound {
                        minValue = extension.minCoveragePercentage
                        maxValue = extension.maxCoveragePercentage
                        metric = MetricType.LINE
                        aggregation = AggregationType.COVERED_PERCENTAGE
                    }
                }
            }
        }
    }
}


fun configureAndroidProject(extension: CoverageExtension) {
    koverReport {
        androidReports(extension.androidVariant) {}
    }
}

// alias for bc
tasks.register("createCoverageReport") {
    dependsOn(tasks.matching { it.name.startsWith("koverXmlReport") })
    dependsOn(tasks.matching { it.name.startsWith("koverHtmlReport") })
}

tasks.register("checkCodeCoverage") {
    dependsOn(tasks.matching { it.name.startsWith("koverVerify") })
}

This is CoverageExtension.kt:


open class CoverageExtension {
    /**
     * The name of the Android variant / source set to use for coverage calculation
     *
     * Note that only a single variant can be configured here, because multiple variants might
     * share common classes that will otherwise cause conflicts when eventually merging the
     * coverage results.
     */
    var androidVariant: String = "release"

    /**
     * Expected module coverage, never decrease, only increase!
     */
    var expectedCoverage: Float = 0.80f

    /**
     * Coverage threshold in which a calculated coverage value is valid
     * minCoverage = expectedCoverage - minCoverageThreshold
     */
    var minCoverageThreshold: Float = 0.01f

    /**
     * coverage threshold in which a calculated coverage value is valid
     * maxCoverage = expectedCoverage + maxCoverageThreshold
     */
    var maxCoverageThreshold: Float = 0.10f

    /**
     *  Jacoco works with class (bytecode) filters, so we filter out things that we can't / don't want to cover
     *
     *  Add your own filters via
     *  ```
     *  classFilter += listOf("MyClass.*")
     *  ```
     */
    var classFilter: List<String> = listOf(
        // Generated Android classes
        "**/R.class",
        "**/BR.class",
        "**/R$*.class",
        "**/BuildConfig.*",
        "**/Manifest*.*",
        "**/*Test*.*",
        // Generated data binding classes
        "**/databinding/*Binding*.class",
        "**/DataBinderMapperImpl.class",
        // Generated room classes
        "**/*_Impl.class",
        "**/*_Impl$*.class",
        // Generated Navigation library classes
        "**/*Args.class",
        "**/*Args\$Companion.class",
        "**/*Directions.class",
        "**/*Directions\$Companion.class",
        // Generated Dagger classes and provisions
        "**/*Module.*",
        "**/generated/**/*.*",
        "**/*_MembersInjector.*",
        "**/Dagger*",
        "**/Hilt_*",
        "**/hilt_aggregated_deps/**/*.*",
        "**/*_Factory.*",
        "**/*Factory.*",
        "**/*Module$*.*",
        // Other classes
        "**/*JsonAdapter.*"
    )

    internal val minCoveragePercentage: Int
        get() = (max(0f, expectedCoverage - minCoverageThreshold) * 100).toInt()

    internal val maxCoveragePercentage: Int
        get() = (min(1f, expectedCoverage + maxCoverageThreshold) * 100).toInt()
}

This is the configuration of an example module that reported around 90% coverage:

plugins {
   id("my.coverage.plugin")
}

// ...

codeCoverage {
    expectedCoverage = 0.90f
    minCoverageThreshold = 0.03f
    maxCoverageThreshold = 0.02f
}

Environment

  • Kover Gradle Plugin version: 0.7.0, 0.7.1
  • Gradle version: 8.1.1
  • Kotlin project type: Kotlin/JVM, Kotlin/Android
  • Coverage Toolset (if customized in build script): JaCoCo("0.8.8")
  • Other context important for this bug: macOS 10.15
@realdadfish realdadfish added Bug Bug issue type S: untriaged Status: issue reported but unprocessed labels Jun 1, 2023
@realdadfish
Copy link
Author

I figured that this seemed to be the hot path for issue 3, when I print "$file" in the debugger I see that the project's class files are printed multiple times (like 20 or 30 times), not just once.
Bildschirmfoto 2023-06-02 um 11 34 38

@shanshin
Copy link
Collaborator

shanshin commented Jun 2, 2023

I figured that this seemed to be the hot path for issue 3, when I print "$file" in the debugger I see that the project's class files are printed multiple times (like 20 or 30 times), not just once.

Hi,
unfortunately, filtering when working with JaCoCo can be organized only in this way.
This line will be executed frequently because it is called for each class file.

However, the filtering code should not work very long.

@shanshin
Copy link
Collaborator

shanshin commented Jun 2, 2023

In Android projects, the non-variant-aware tasks koverVerify, koverHtmlReport, ... are created, though they're useless and result in being skipped when executed, because there are no tests executed (naturally):

This is the expected behavior, Kover default tasks are always created.

For Android, they are empty so that the user himself can add a report of interesting build variants to them with mergeWith(...).

In general, it can take a long time to build a report for all build variants and often causes errors, because some classes with the same name may be duplicated in different variants.

@realdadfish
Copy link
Author

I figured that this seemed to be the hot path for issue 3, when I print "$file" in the debugger I see that the project's class files are printed multiple times (like 20 or 30 times), not just once.

Hi, unfortunately, filtering when working with JaCoCo can be organized only in this way. This line will be executed frequently because it is called for each class file.

However, the filtering code should not work very long.

I'm checking the execution with --profile now to see if there is anything noticable.

@shanshin
Copy link
Collaborator

shanshin commented Jun 2, 2023

When I drill down into the details I see that especially Robolectric-based UI tests get no coverage reported at all. Are there any special settings needed to get the coverage for those tests as well?

At the moment, tests executed on Android devices or in the emulator are not supported.

@realdadfish
Copy link
Author

In general, it can take a long time to build a report for all build variants and often causes errors, because some classes with the same name may be duplicated in different variants.

I have only a single build variant, release, for all but my app module, for performance reasons.

@realdadfish
Copy link
Author

When I drill down into the details I see that especially Robolectric-based UI tests get no coverage reported at all. Are there any special settings needed to get the coverage for those tests as well?

At the moment, tests executed on Android devices or in the emulator are not supported.

No, these are pure JVM tests, Robolectric does not need a device or emulator. Still, it is noticable that even though the JVM UI tests run, I get no coverage reported for any UI-related classes.

@shanshin
Copy link
Collaborator

shanshin commented Jun 2, 2023

I have only a single build variant, release, for all but my app module, for performance reasons.

For your case, you may specify mergeWith("release"), (like in example)[https://kotlin.github.io/kotlinx-kover/gradle-plugin/#kotlin-multiplatform-project]

@realdadfish
Copy link
Author

I'm a bit reluctant to switch to kover instrumentation / coverage, because a) I have a partially mixed source with legacy code being written in Java (and IIRC this is only supported with Jacoco, right?) and b) the coverage results will probably vary quite a bit when the tool is changed (at least I got completely different coverage values when I switched between IntelliJ coverage and Jacoco in the IDE in the past), but I guess I'll try anyway.

The reason why I also stick to Jacoco 0.8.8 is because I also saw coverage deviations between different Jacoco versions and for now I kind of want a "reproducible" coverage result. But I'll also try 0.8.10 which is the newest IIRC.

@shanshin
Copy link
Collaborator

shanshin commented Jun 2, 2023

Is your project open source ?
A small project reproducer would help to deal with the slowdown

@shanshin shanshin added S: in progress Status: implementing or design in process and removed S: untriaged Status: issue reported but unprocessed labels Jun 2, 2023
@realdadfish
Copy link
Author

I'm checking the execution with --profile now to see if there is anything noticable.

Ok, this has very low resolution, it just tells me that the koverVerifyRelease task executed for 6m17s out of 7m16s total (cached test execution).

@realdadfish
Copy link
Author

Is your project open source ? A small project reproducer would help to deal with the slowdown

Unfortunately not, all closed source. Have you experience with https://github.com/gradle/gradle-profiler-plugin, would this be helpful to create profile data with this?

@shanshin
Copy link
Collaborator

shanshin commented Jun 2, 2023

Ok, this has very low resolution, it just tells me that the koverVerifyRelease task executed for 6m17s out of 7m16s total (cached test execution).

A large number of classes in the project?

Could you remove the useJacoco("0.8.8") and compare how long the validation will take using Kover tool?

@realdadfish
Copy link
Author

A large number of classes in the project?

find . -name "*.class" | grep -v Test | grep -v test | wc -l tells me 2725. It's one of the bigger modules, but not huge.

Could you remove the useJacoco("0.8.8") and compare how long the validation will take using Kover tool?

Yes, running now.

@shanshin
Copy link
Collaborator

shanshin commented Jun 2, 2023

Have you experience with https://github.com/gradle/gradle-profiler-plugin, would this be helpful to create profile data with this?

Not yet, but it will be useful to understand whether the slowdown occurs during filtering or during validation by JaCoCo itself.

For comparison, you may try to remove all report filters and compare whether the build time will speed up (in this case, the code from here will not be executed).

@realdadfish
Copy link
Author

Could you remove the useJacoco("0.8.8") and compare how long the validation will take using Kover tool?

Yes, running now.

koverVerifyRelease with Kover instead of Jacoco only took 2,7s. Trying without class filters now.

@realdadfish
Copy link
Author

koverVerifyRelease with Kover instead of Jacoco only took 2,7s. Trying without class filters now.

The same task with Jacoco, but without class filters, took 1,9s, so the filters are definitely the culprit.

@shanshin
Copy link
Collaborator

shanshin commented Jun 2, 2023

How many class-files do you have in the project?

@realdadfish
Copy link
Author

FYI - koverReport { defaults { mergeWith("release") } androidReports("release") {} } needed to be wrapped into project.afterEvaluate {} because of #362. This was not needed when I just configured koverReport { androidReports("release") {} }.

Also, with the configured mergeWith above I still get

> Task :my-module:koverHtmlReport SKIPPED
Task 'koverHtmlReport' will be skipped because no tests were executed

so this does not seem to work either. So I skip this for now and only call / work with the Android variants of the specific tasks for now.

@realdadfish
Copy link
Author

realdadfish commented Jun 2, 2023

How many class-files do you have in the project?

See above, about 2700 prod class files in the build/ directory. Because this is Kotlin code many of these are lambdas, inlines, etc.

@realdadfish
Copy link
Author

The creation of the Kover (non Jacoco) HTML report took almost 40s, not exactly fast either. What's good is that this reports a somewhat correct line coverage of 81% for the module. Will check the verification task now.

@shanshin
Copy link
Collaborator

shanshin commented Jun 2, 2023

The creation of the Kover (non Jacoco) HTML report took almost 40s, not exactly fast either.

To generate the report, Kover analyzes all class-files and all sources, so it can work slowly on large projects.

@realdadfish
Copy link
Author

Also, I configured a minValue of 47 and a maxValue of 52, but when I execute koverVerify on the said module that has a line coverage of 81%, the task does not fail. Isn't it supposed to do that because the upper limit is hit?

@shanshin
Copy link
Collaborator

shanshin commented Jun 2, 2023

Isn't it supposed to do that because the upper limit is hit?

Yes, verification should fail.

For more info, please add rule minValue=100 and maxValue=0 to see verification error

@realdadfish
Copy link
Author

Will do that.

Btw... thanks for your / JetBrains work on this, even if it is not yet perfect. A proper coverage solution for JVM/Android/multiplatform projects was long overdue. So thanks again, also for your instant support!

@realdadfish
Copy link
Author

Hah, nice that my custom ShowPackageCoverage task still works - apparently kover is reusing the Jacoco XML format!

> Task :my-module:showPackageCoverageRelease
my.app.mymodule.databinding: 74,89%
                                      .di: 51,20%
                                      .domain.filter: 63,77%
                                             .messagenotification: 100,00%
                                             .model: 99,71%
                                      Sum: 85,25%
                                      .io.db.converter: 96,15%
                                            .migrations: 100,00%
                                            .model: 100,00%
                                         Sum: 68,20%
                                         .mqtt.model: 57,91%
                                      Sum: 63,41%
                                      .tech: 71,15%
                                      .view.messagenotification: 100,00%
                                           .screens.actionchippanel.internal: 100,00%
                                                                   .mapper: 100,00%
                                                                   .model: 100,00%
                                                   Sum: 92,16%
                                                   .bla.model: 100,00%
                                                                .state: 88,41%
                                                   Sum: 90,57%
                                                   .common.mapper: 88,78%
                                                          .model: 91,32%
                                                   Sum: 82,42%
                                                   .main.model: 93,48%
                                                   .start.model: 80,00%
                                                   .foo.checkin.mapper: 100,00%
                                                                           .bar.model: 89,74%
                                                                           .baz.model: 99,25%
                                                                   Sum: 87,86%
                                                                   .mapper: 83,56%
                                                                   .model: 100,00%
                                                   Sum: 86,84%
                                                   .topbarinformation.internal: 100,00%
                                                   .bazz.internal: 76,53%
                                                                .model: 100,00%
                                                   Sum: 87,76%
                                                   .buzz.mapping: 100,00%
                                                           .model: 100,00%
                                                           .fizz.internal: 100,00%
                                                                              .model: 100,00%
                                                           Sum: 87,04%
                                                           .state: 96,67%
                                                   Sum: 87,58%
                                                   .bazz.model: 100,00%
                                           Sum: 87,84%
                                      Sum: 88,04%
                              Sum: 82,21%
hilt_aggregated_deps: No coverage

@realdadfish
Copy link
Author

Last post from me here today (will be back on Monday), but I can't get verification to work. I execute it like this

gradle -p my-component my-module:koverVerifyRelease --rerun --profile

and use the output of --profile to check if the task really runs (without --rerun it is marked as UP-TO-DATE, not sure thats right, because it does not output anything, and even though the inputs haven't changed I think it should rather always run when executed).

Problem is that the task succeeds, even with this configuration:

  koverReport {
        androidReports("release") {}
        filters {
            excludes {
                classes(/* my class filter */)
            }
        }
        defaults {
            verify {
                onCheck = true
                rule {
                    isEnabled = true
                    entity = GroupingEntityType.APPLICATION
                    bound {
                        minValue = 100
                        maxValue = 0
                        metric = MetricType.LINE
                        aggregation = AggregationType.COVERED_PERCENTAGE
                    }
                }
            }
        }
    }

@shanshin
Copy link
Collaborator

shanshin commented Jun 5, 2023

Hi,
in your configuration, you configure verification for the task koverVerify. For Android projects, these tasks do nothing by default.

To configure validation for the release build variant, you must specify

koverReport {
        androidReports("release") {
            filters {
                excludes {
                    classes(/* my class filter */)
                }
            }
            verify {
                onCheck = true
                rule {
                    bound {
                        minValue = 100
                        maxValue = 0
                    }
                }
            }
        }
}

@realdadfish
Copy link
Author

Ah I see, I thought this was common configuration that was shared.

@realdadfish
Copy link
Author

I simply don't get it - it will just not execute the verification rule:

// since the plugin does not support lazy configuration and our coverage extension does neither, we need to
// wait until the module's build gradle file is evaluated and has the proper settings filled in
afterEvaluate {
    // We can't use Jacoco because of slow class filtering, see https://github.com/Kotlin/kotlinx-kover/issues/398
    // kover {
    //  useJacoco("0.8.8")
    // }
    plugins.withId("org.jetbrains.kotlin.jvm") {
        configureJvmProject(coverageExtension)
    }
    plugins.withType(AndroidBasePlugin::class.java) {
        configureAndroidProject(coverageExtension)
    }
}

fun configureJvmProject(extension: CoverageExtension) {
    koverReport {
        filters {
            configure(extension)
        }
        defaults {
            configure(extension)
        }
    }
}

fun configureAndroidProject(extension: CoverageExtension) {
    koverReport {
        androidReports(extension.androidVariant) {
            filters {
                configure(extension)
            }
            defaults {
                configure(extension)
            }
        }
    }
}

fun KoverDefaultReportsConfig.configure(extension: CoverageExtension) {
    verify {
        onCheck = true
        rule {
            isEnabled = true
            entity = GroupingEntityType.APPLICATION
            bound {
                minValue = 100 //extension.minCoveragePercentage
                maxValue = 0 // extension.maxCoveragePercentage
                metric = MetricType.LINE
                aggregation = AggregationType.COVERED_PERCENTAGE
            }
            filters {
                configure(extension)
            }
        }
    }
}

fun KoverReportFilters.configure(extension: CoverageExtension) {
    excludes {
        classes(extension.classFilter)
    }
}

I try to make a small example project now. Can't seem to get this working like this.

@shanshin
Copy link
Collaborator

shanshin commented Jun 5, 2023

Please, for checking, write exactly as in the example above (for example, for a variant release)

koverReport {
        androidReports("release") {
            filters {
                excludes {
                    classes(/* my class filter */)
                }
            }
            verify {
                onCheck = true
                rule {
                    bound {
                        minValue = 100
                        maxValue = 0
                    }
                }
            }
        }
}

and execute koverVerifyRelease.

Configuring inside afterEvaluate may lead to unpredictable behavior.

@shanshin
Copy link
Collaborator

shanshin commented Jun 5, 2023

It might be better to postpone the migration until issue #362 is fixed.

Current configuration issues complicate the stable reproduction of complex configuration cases.

@realdadfish
Copy link
Author

The problem I'm facing right now is that I can't lazy-populate the plugin with settings from my build convention extension, without relying onafterEvaluate - and this breaks the plugin right now. I guess I'll postpone my migration here indeed, sadly. I've pushed my test project here: https://github.com/realdadfish/kover-issue-398/

It's also not entirely clear to me how the filters configurations on KoverReportsConfig, KoverVerifyRule, KoverReportExtension and KoverDefaultReportsConfig work altogether. There might be use cases why one wants all these different filter configurations everywhere, but at least for the novel user these all complicate matters a lot.

Thanks so far for your time and effort!

@shanshin
Copy link
Collaborator

shanshin commented Jun 5, 2023

It's also not entirely clear to me how the filters configurations on KoverReportsConfig, KoverVerifyRule, KoverReportExtension and KoverDefaultReportsConfig work altogether.

In most cases, it is enough to use

koverReport {
    filters {
        // these filters are applied to all reports and verifications for all build variants
    }
}

Only if individual filters are needed for any particular build variant, then these filters can be overridden:

koverReport {
    androidReports("specialBuildVariant") {
       filters {
           // the filters specified here will only apply to reports for variant `specialBuildVariant` built by  tasks `koverXmlReportSpecialBuildVariant`, `koverHtmlReportSpecialBuildVariant`, `koverVerifySpecialBuildVariant`
        }
    }
}

However, this is a very rare case.

All filters are inherited from the level above and can overload each other, see

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug issue type S: in progress Status: implementing or design in process
Projects
None yet
Development

No branches or pull requests

2 participants