Kotlin / kotlinx-kover

Apache License 2.0
1.27k stars 47 forks source link

Kover 0.7.3 Drastically coverage drop #373

Closed mustafaozhan closed 8 months ago

mustafaozhan commented 1 year ago

Recently I updated kover to 0.7.3, the changes i made can be seen in this PR: https://github.com/Oztechan/CCC/pull/2155

The app has many modules, some of them also git submodules, and it is also a Kotlin Multiplatform app.

While I was expecting a coverage change because kover still includes the test classes coverage in previous version, after the version increase coverage decreased by 15.30%

When I navigate in Codecov's interface about the same PR I realise that many of my modules are not even included ie.

So these missings are explaining the drastically coverage drop

Errors no error found.

Expected behavior The coverage should be calculated correctly and all the modules should be included.

Reproducer Simply check the PR: https://github.com/Oztechan/CCC/pull/2155, repo has also README.md file that can help you running the project.

Reports Here is the COdecov link for the mentioned PR: https://app.codecov.io/gh/Oztechan/CCC/pull/2155/tree/backend

Environment

qwwdfsad commented 1 year ago

AFAIR it's a bug in 0.7.0 due to how reports are filtered -- you can try replacing

 koverReport.defaults {
            filters {
                excludes {
                    annotatedBy("com.oztechan.ccc.android.ui.compose.annotations.ThemedPreviews")
                    annotatedBy("androidx.compose.ui.tooling.preview.Preview")
                    annotatedBy("androidx.compose.runtime.Composable")
                }
            }

with

 koverReport {
            filters {
                excludes {
                    annotatedBy("com.oztechan.ccc.android.ui.compose.annotations.ThemedPreviews")
                    annotatedBy("androidx.compose.ui.tooling.preview.Preview")
                    annotatedBy("androidx.compose.runtime.Composable")
                }
            }

@shanshin is on vacation right now, so please expect a delayed response

shanshin commented 1 year ago

Hi, since version 0.7.0, Kover default tasks do not take into account tests for Android targets.

However, it is possible to add classes and tests run results for a specific build variant to the default reports.

For example, for all projects where there are Android targets, you can add the results of covering the required build variant:

koverReport {
    defaults {
        // adds the contents of the reports of `release` Android build variant to default reports
        mergeWith("release")
    }
}
mustafaozhan commented 1 year ago

Hello @shanshin

Thanks a lot for the answer and suggestion. I have changed how I applied kover to this:

allprojects {
    apply(plugin = rootProject.libs.plugins.kover.get().pluginId).also {
        rootProject.dependencies.add("kover", project(path))
        koverReport {
            if (pluginManager.hasPlugin(rootProject.libs.plugins.androidLib.get().pluginId)) {
                defaults {
                    // adds the contents of the reports of `release` Android build variant to default reports
                    mergeWith("release")
                }
            }
            filters {
                excludes {
                    annotatedBy("com.oztechan.ccc.android.ui.compose.annotations.ThemedPreviews")
                    annotatedBy("androidx.compose.ui.tooling.preview.Preview")
                    annotatedBy("androidx.compose.runtime.Composable")
                }
            }
        }
    }

But unfortunately, drastically coverage drop is still continuing. The PR in the description of the issue is also updated in case you want to take a look.

fedor7peaks commented 1 year ago

Having same issue after updating from 0.6.1 to 0.7.0, the list of exclusions is exactly the same, however, it seems that they've stopped working:

koverReport {
    filters {
        classes {
            excludes.addAll(exclusions)
        }
    }
    defaults {
        mergeWith("release")
    }
}

And none of the exclusions were actually removed in the report. Any solutions?

shanshin commented 1 year ago

@fedor7peaks , do you have a reproducer project?

fedor7peaks commented 1 year ago

@shanshin I just realized there I've been using outdated configuration for filters:

filters {
    classes {
        excludes.addAll(exclusions)
    }
}

Instead of:

filters {
    excludes {
        classes(exclusions)
    }
}

Let me re-test the coverage again and update here if any issues.

fedor7peaks commented 1 year ago

@shanshin After re-testing it looks like there is no difference for coverage for classes/method/branches, the only difference is for line (0.4% lower)/instruction (0.5% lower) in my project, so overall decrease is not that big.

shanshin commented 1 year ago

difference is for line (0.4% lower)/instruction (0.5% lower) in my project, so overall decrease is not that big.

This may be due both to the compiler version being changed, and to the fact that some bugs were fixed in the Kover agent because of which the coverage was calculated incorrectly before.

mustafaozhan commented 1 year ago

@shanshin to give a quick heads up, for me the reason is not related to exclusions, I only excluded some annotations in 1 module.

For me the issue is with Kover 0.7.0 I start not covering multiple modules entirely.

mustafaozhan commented 1 year ago

@shanshin update, issue still continues on 0.7.1

shanshin commented 10 months ago

Sorry for the delayed answer.

Is the problem still reproduced on the 0.7.2 version? If so, could you please update the branch with all the latest changes?

mustafaozhan commented 10 months ago

@shanshin still the same, and the branch is updated with latest version

shanshin commented 10 months ago

@mustafaozhan Here are some minor configuration changes to use Kover version 0.7.2 (Patch for IDEA/Android Studio).

Please check if the code from all the necessary projects is included in the report and if all the tests are running.

mustafaozhan commented 10 months ago

Hello @shanshin thank you for your message and the patch, I applied the patch. The drop was ~15% percent previously, but now it is ~25%. It can be because previously the 3 modules with build flavors are not included. It looks like I can see classes from all the modules in the merged report. But still the overall Drastically coverage drop is continuing.

Btw, not related with the issue but this 0.7.X update also change the setup of kover, and it doesn't look lean like before.

shanshin commented 10 months ago

But still the overall Drastically coverage drop is continuing.

Perhaps this is because the tests were not run from all build variants, only release and googleRelease were used. If you want, you can specify all the build variants for the projects.

@mustafaozhan, also, if it's convenient for you, we can add a merge with all the build variants the following forms:

shanshin commented 10 months ago

We can add such a small change in the next release, #433.

mustafaozhan commented 10 months ago

Not sure if I understand the suggested change good đŸ¤”

mergeWith("*release") fails with

Could not resolve all dependencies for configuration ':backend:app:runtimeClasspath'.
> A problem occurred configuring project ':common:core:database'.
   > Build variant '*release' not found in project ':common:core:database' - impossible to merge default reports with its measurements.
     Available variations: [debug, release]

while mergeWith("*") fails with

Could not resolve all dependencies for configuration ':backend:app:runtimeClasspath'.
> A problem occurred configuring project ':common:core:database'.
   > Build variant '*' not found in project ':common:core:database' - impossible to merge default reports with its measurements.
     Available variations: [debug, release]

Should I wait for the upcoming changes to be implemented first ? Also, this doesn't look so natural to me ideally kover should be able to detect the necessary variants and modules regardless from the project setup

shanshin commented 10 months ago

Should I wait for the upcoming changes to be implemented first ?

These changes are not ready yet, they need to be further thought through.

Also, this doesn't look so natural to me ideally kover should be able to detect the necessary variants and modules regardless from the project setup

Kover is already able to determine the build variants, however, when there are several independent Android applications in a multi-project build, there is no single correct way to match the build variants of one app configuration with the options of another app configuration.

In this case, the users should help Kover "understand" what exactly from each project they wants to see in a specific report.

For example, if you want all build variants with the release build type to be included in default reports, then you can list them in mergeWith (mergeWith("googleRelease") + mergeWith("huaweiRelease") + mergeWith("release") according to the idea , it is reduced in = mergeWith("*release"))

And for another user, it may be necessary to add an build variant with a specific to default reports (for example, only mergeWith("huaweiRelease")).

That's why, Kover expects additional configuration (by mergeWith) to avoid ambiguities.

shanshin commented 10 months ago

@mustafaozhan, could you clarify, did you mean Kover to automatically add all build variants to the report?

mustafaozhan commented 10 months ago

Yeah, let me know if I am missing some point, but coverage tools are mostly mean to calculate the all the project's coverage for the all modules and the codebase. So I would like to cover all the Kotlin classes/files.

Also, I am not sure why the variants plays significant role here. If the variants don't have their specific sources, they should have both same coverages. But again excuse me if I am missing some point here I am looking from the tool perspective, I have no idea about the implementation of the coverage tools. Maybe you can check how it is done in other tools.

shanshin commented 10 months ago

In the latest versions of Gradle, it is not recommended that plugins access other projects, so you need to manually apply the plugin either in all projects or in those projects that are needed in the report.

In the future, you can add a plugin application function in all subprojects, but it will still need to be called manually.

About the build variants: Perhaps in the future we will add a task to build reports for all build variants at once, but not everyone needs it, and often it is necessary to build a report either for a specific variant or for part of them. Therefore, there should be opportunities for merge.

mustafaozhan commented 9 months ago

Thank you for the info @shanshin that's true allprojects configuration is not recommended anymore. Btw the https://github.com/Kotlin/kotlinx-kover/pull/433 seems closed now is there, I will be keep updating my branch with latest kover version

shanshin commented 8 months ago

In the next major release, it was decided to redo the logic of work for tasks with a short name (for example koverHtmlReport) - now they will generate reports for all build variants in the Android project.

You may track the progress in task #462.

shanshin commented 8 months ago

Closed in favor of #462