Kotlin / kotlinx-kover

Apache License 2.0
1.27k stars 47 forks source link

BR class not excluded #310

Closed pvegh closed 1 year ago

pvegh commented 1 year ago

BR classes should be excluded by *.databinding.*, Probably also should be default, no config needed?

Using config:

koverAndroid {
    // filters for all report types of all build variants
    common {
        filters {
            excludes {
                classes(
                    "*Fragment",
                    "*Fragment\$*",
                    "*Activity",
                    "*Activity\$*",
                    "*.databinding.*",
                    "*.BuildConfig",
                )
            }
        }
    }
}

Expected behavior BR classes shouldn't show up in the report

Environment

shanshin commented 1 year ago

Hi, the exclusion rules use fully-qualified class names. If you want to exclude a class, you need to write either the full name of the class (e.g. "my.package.Fragment") or use wildcards ("*.Fragment", "*.Fragment\$*", "*.databinding.*" etc). Also see filters examples

pvegh commented 1 year ago

Looks exactly like what I havem but the code block was missing, asterisk disappearing. Sorry, fixed.

shanshin commented 1 year ago

If you specify this filters, then classes are excluded from the report? What task are you running to build a report?

pvegh commented 1 year ago

No, I see many BR classes in the report in app/build/reports/kover/htmlLocalDebug/index.html I run the html task for the localDebug variant: ./gradlew koverHRLD

Funny thing is, I see BR classes from multiple modules (not just app). But I don't see actual real stuff from those modules. But that would be another bug report, I wanted to clear the BR classes first

shanshin commented 1 year ago

Your project is open source, is it possible to get acquainted with it?

pvegh commented 1 year ago

Sorry but it isn't. I'm not sure when I'll find time to create a reproducer for this. Hoping someone else also has the problem with more time on hand.

shanshin commented 1 year ago

Can you show the entire koverAndroid configuration?

If you have specified a filters for a specific variant, then they completely override all exclusions.

pvegh commented 1 year ago

That's the full config above actually, wanted to start with the official setup

pvegh commented 1 year ago

is one asterisk really correct, instead of double to define whatever subpackage it might be?

shanshin commented 1 year ago

Yes, a double asterisk works exactly the same as a single one

shanshin commented 1 year ago

If you specify a class from your application in the filters, will it disappear from the report?

pvegh commented 1 year ago

Yes the following does hide the 2 classes (from 2 different modules) in the report: (The others BR classes are still there, it's not realistic to exclude them one by one)

            excludes {
                classes(
                    "foo.bar.BR",
                    "foo.bar2.BR",
                    "*Fragment",
                    "*Fragment\$*",
                    "*Activity",
                    "*Activity\$*",
                    "*.databinding.*",
                    "*.BuildConfig",
                )
            }
shanshin commented 1 year ago

I will try to reproduce this locally

shanshin commented 1 year ago

Please try the new DSL in version 0.7.0-Beta. Feel free to open new bug issue if the problem reproduced again.

pvegh commented 1 year ago

I tried again with the Beta, not sure anything changed. See comments in my config

koverReport {
        androidReports("debug") {
            filters {
                excludes {
                    classes(
                        "*BR", // TODO doesn't exclude them in either app module nor in submodule
                        "*Fragment",
                        "*Fragment\$*",
                        "*Activity",
                        "*Activity\$*",
                        "*.databinding.*",
                        "*.BuildConfig", // doesn't exclude them in either app module nor in submodule
                        // excludes debug classes
                        "*.DebugUtil",
                    )
                }
            }
        }
    }
shanshin commented 1 year ago

Could you show a screenshot of the report for any BR class?

pvegh commented 1 year ago
kover beta BR
shanshin commented 1 year ago

Do you specify this configuration in the same project in which the report generation task is called?

This is a merged report (with the addition of kover(project(":subproject")) dependencies)?

pvegh commented 1 year ago

The config is in the app module and I do ./gradlew koverHRLocalDebug (local is an app module specific flavor, all the other modules have only debug and release. This might be a problem here. This config should be valid for localDebug and debug variants, I'll check what I have to do in this case.

Yes I added just one another module to it just for testing. It did add many classes from that module so it seems to be effective.

shanshin commented 1 year ago

This might be a problem here.

In your configuration for the app module, you specified the debug build variant, but there is no such variant in the module (debug is the build type), it looks like you have the localDebug build variant.

Try to correct the name of the build variant

koverReport {
        androidReports("localDebug") {
            filters {
                excludes {
                    classes(
                        "*BR", // TODO doesn't exclude them in either app module nor in submodule
                        "*Fragment",
                        "*Fragment\$*",
                        "*Activity",
                        "*Activity\$*",
                        "*.databinding.*",
                        "*.BuildConfig", // doesn't exclude them in either app module nor in submodule
                        // excludes debug classes
                        "*.DebugUtil",
                    )
                }
            }
        }
    }

or specify a common filter for all variants

koverReport {
        filters {
            excludes {
                classes(
                    "*BR", // TODO doesn't exclude them in either app module nor in submodule
                    "*Fragment",
                    "*Fragment\$*",
                    "*Activity",
                    "*Activity\$*",
                    "*.databinding.*",
                    "*.BuildConfig", // doesn't exclude them in either app module nor in submodule
                    // excludes debug classes
                    "*.DebugUtil",
                    )
                }
            }
    }
pvegh commented 1 year ago

Not using androidReports at all solves the problem. I though without it nothing would be generated. I'll have to use this a little more but it seems like this exact issue is indeed solved when using the right config. Thanks!

shanshin commented 1 year ago

I though without it nothing would be generated

Calling the corresponding task always starts the report generation. Configuration is optional, it only allows you to change the contents of the report.

Unfortunately, in this version, the code missed checking for the existence of a build variant, so there was no error even when trying to configure a report for a variant that is not in the project.