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

Kover 0.7.3 Drastically coverage drop #373

Closed
mustafaozhan opened this issue May 17, 2023 · 24 comments
Closed

Kover 0.7.3 Drastically coverage drop #373

mustafaozhan opened this issue May 17, 2023 · 24 comments
Assignees
Labels
Bug Bug issue type S: in progress Status: implementing or design in process

Comments

@mustafaozhan
Copy link

mustafaozhan commented May 17, 2023

Recently I updated kover to 0.7.3, the changes i made can be seen in this PR: Oztechan/CCC#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.

  • any of the :adnroid:X modules are in the coverage.
  • any modules from :client:X except :client:core:shared

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: Oztechan/CCC#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

  • Kover Gradle Plugin version: 0.7.3
  • Gradle version: 8.0.1
  • Kotlin project type: Kotlin/Multiplatform
  • Coverage Engine version: Default one
@mustafaozhan mustafaozhan added Bug Bug issue type S: untriaged Status: issue reported but unprocessed labels May 17, 2023
@qwwdfsad
Copy link
Contributor

qwwdfsad commented May 17, 2023

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
Copy link
Collaborator

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")
    }
}

@shanshin shanshin added S: waiting for clarification Status: additional information required to proceed and removed S: untriaged Status: issue reported but unprocessed labels May 22, 2023
@mustafaozhan
Copy link
Author

mustafaozhan commented May 23, 2023

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
Copy link

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
Copy link
Collaborator

@fedor7peaks , do you have a reproducer project?

@fedor7peaks
Copy link

@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
Copy link

@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
Copy link
Collaborator

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
Copy link
Author

@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
Copy link
Author

@shanshin update, issue still continues on 0.7.1

@mustafaozhan mustafaozhan changed the title Kover 0.7.0 Drastically coverage drop Kover 0.7.1 Drastically coverage drop Jun 2, 2023
@shanshin shanshin added S: in progress Status: implementing or design in process and removed S: waiting for clarification Status: additional information required to proceed labels Jun 27, 2023
@shanshin
Copy link
Collaborator

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
Copy link
Author

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

@mustafaozhan mustafaozhan changed the title Kover 0.7.1 Drastically coverage drop Kover 0.7.2 Drastically coverage drop Jul 12, 2023
@shanshin
Copy link
Collaborator

@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
Copy link
Author

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
Copy link
Collaborator

shanshin commented Jul 14, 2023

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:

  • mergeWith("*") - use coverage for all build variants in the project
  • mergeWith("*release") - use coverage for all build variants ending with release, e.g. Release, googleRelease etc

@shanshin
Copy link
Collaborator

shanshin commented Jul 14, 2023

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

shanshin added a commit that referenced this issue Jul 14, 2023
Is it now possible to use wildcards in the `mergeWith` function `?` and `*` and the check has become case-insensitive.

Relates #373
shanshin added a commit that referenced this issue Jul 14, 2023
Now it is possible to use wildcards in the `mergeWith` function `?` and `*` and the check has become case-insensitive.

Relates #373
shanshin added a commit that referenced this issue Jul 14, 2023
Now it is possible to use wildcards in the `mergeWith` function `?` and `*` and the check has become case-insensitive.

Relates #373
@mustafaozhan
Copy link
Author

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

@mustafaozhan mustafaozhan changed the title Kover 0.7.2 Drastically coverage drop Kover 0.7.3 Drastically coverage drop Aug 1, 2023
@shanshin
Copy link
Collaborator

shanshin commented Aug 1, 2023

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
Copy link
Collaborator

shanshin commented Aug 1, 2023

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

@mustafaozhan
Copy link
Author

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
Copy link
Collaborator

shanshin commented Aug 1, 2023

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
Copy link
Author

Thank you for the info @shanshin that's true allprojects configuration is not recommended anymore. Btw the #433 seems closed now is there, I will be keep updating my branch with latest kover version

@shanshin
Copy link
Collaborator

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
Copy link
Collaborator

Closed in favor of #462

@shanshin shanshin closed this as not planned Won't fix, can't repro, duplicate, stale Sep 21, 2023
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

4 participants