Added extended reports filtering #581

shanshin · 2024-03-22T13:39:04Z

The following report filtering features have been implemented:

include filter by classes annotated with specified annotations
include and exclude filters by classes that inherit the specified class or implement the specified interface

Resolves #274
Resolves #454

sandwwraith · 2024-03-26T16:37:32Z

kover-cli/src/main/kotlin/kotlinx/kover/cli/commands/ReportCommand.kt

+
+    @Option(
+        name = "--excludeInheritedFrom",
+        usage = "filter to exclude classes inheriting the specified class or implementing an interface, wildcards `*` and `?` are acceptable",


[nit] since you differ class and interface, maybe its better to use standard JVM terminology "... extending the specified class or implementing an interface"

sandwwraith · 2024-03-26T16:38:26Z

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

@@ -186,17 +186,62 @@ public static class ClassFilters {
         */
        public final Set<String> excludeClasses;

+        /**
+         * Classes that have at least one of the annotations specified in this field are presents in the report.
+         * All other classes that are not marked with at least one of the specified annotations are not included in the report.


What happens when both excludeAnnotation and includeAnnotation are specified? I know it is mentioned in markdown docs, but it would nice to have it here also

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

kover-gradle-plugin/docs/configuring.md

sandwwraith · 2024-03-26T16:43:13Z

kover-gradle-plugin/docs/configuring.md

@@ -46,12 +46,19 @@ koverReport {
                packages("com.another.subpackage")
                // excludes all classes and functions, annotated by specified annotations (with BINARY or RUNTIME AnnotationRetention), wildcards '*' and '?' are available
                annotatedBy("*Generated*")
+                // exclude all classes that inherit the specified class or implement the specified interface
+                inheritedFrom("*Repository")


You specified inheritedFrom("*Repository") in both excludes and includes. This makes the example very strange.

Also, these docs should be fully rewritten

sandwwraith · 2024-03-26T16:44:39Z

...Test/kotlin/kotlinx/kover/gradle/plugin/test/functional/cases/ReportAnnotationFilterTests.kt

@@ -37,18 +38,50 @@ internal class ReportAnnotationFilterTests {
                }
            }
        }
+    }
+
+    class Foo : AutoCloseable {


I do not see this class being used in test, accidental leftover?

Dark experiments )))

...t/kotlin/kotlinx/kover/gradle/plugin/test/functional/cases/ReportInheritedFromFilterTests.kt

sandwwraith · 2024-03-26T16:55:33Z

kover-gradle-plugin/src/main/kotlin/kotlinx/kover/gradle/plugin/dsl/KoverReportsConfig.kt

+     *  inheritedFrom("*Repository")
+     * ```
+     *
+     * **_Does not work for JaCoCo_**


I believe that the fact that this doesn't work for JaCoCo should be mentioned in the text in this section: https://github.com/Kotlin/kotlinx-kover/blob/main/kover-gradle-plugin/docs/configuring.md#reports-filtering
Actually, this whole block:

It is acceptable to filter a class from the report by its fully-qualified name - using classes or packages. Also acceptable to filter a class, function or getter marked with the specified annotation - annotatedBy.

needs to be expanded with a new functionality

shanshin · 2024-03-26T18:10:31Z

Tests will fail for now, until reporter be fixed

sandwwraith · 2024-03-27T11:26:05Z

And when reporter will be fixed?

The following report filtering features have been implemented: - include filter by classes annotated with specified annotations - include and exclude filters by classes that inherit the specified class or implement the specified interface Resolves #274 Resolves #454

Co-authored-by: Leonid Startsev <sandwwraith@users.noreply.github.com>

sandwwraith · 2024-05-08T12:45:00Z

kover-cli/docs/index.md

-| --xml <xml-file-path>                 | generate a XML report in the specified path                                                             |          |          |
+| Option                                         | Description                                                                                                                                  | Required | Multiple |
+|------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------|:--------:|:--------:|
+| `<binary-report-path>`                         | list of binary reports files                                                                                                                 |          |    +     |


Maybe binary report files, since we analyzing one report at the time?

<binary-report-path> is a name of multiple parameter, see @Argument(usage = "list of binary reports files", metaVar = "<binary-report-path>").

that is, the user is expected to write something like this /tmp/report1.ic /tmp/report2.ic meaning <binary-report-path> <binary-report-path>

kover-cli/docs/index.md

sandwwraith · 2024-05-08T12:46:07Z

kover-cli/docs/index.md

+| --excludeInheritedFrom <exclude-ancestor-name> | filter to exclude classes extending the specified class or implementing an interface, wildcards `*` and `?` are acceptable                   |          |    +     |
+| --includeInheritedFrom <include-ancestor-name> | filter to include only classes extending the specified class or implementing an interface, wildcards `*` and `?` are acceptable              |          |    +     |
+| --html <html-dir>                              | generate a HTML report in the specified path                                                                                                 |          |          |
+| --include <class-name>                         | filter to include classes, wildcards `*` and `?` are acceptable                                                                              |          |    +     |


Shouldn't it be placed next to --exclude?

sandwwraith · 2024-05-08T12:46:45Z

kover-cli/docs/index.md

+| --include <class-name>                         | filter to include classes, wildcards `*` and `?` are acceptable                                                                              |          |    +     |
+| --src <sources-path>                           | location of the source files root                                                                                                            |    +     |    +     |
+| --title <html-title>                           | title in the HTML report                                                                                                                     |          |          |
+| --xml <xml-file-path>                          | generate a XML report in the specified path                                                                                                  |          |          |


What happens if I specify both --xml and --html? Both reports will be generated?

kover-cli/src/main/kotlin/kotlinx/kover/cli/commands/ReportCommand.kt

kover-gradle-plugin/docs/configuring.md

sandwwraith · 2024-05-08T13:47:25Z

...t/kotlin/kotlinx/kover/gradle/plugin/test/functional/cases/ReportInheritedFromFilterTests.kt

+                classCounter("org.jetbrains.RegularClass").assertFullyMissed()
+                classCounter("org.jetbrains.CloseableClass").assertFullyMissed()
+
+                classCounter("org.jetbrains.B").assertAbsent()


Should we add classCounter("org.jetbrains.A").assertFullyMissed() here?

Yep, forgot it

sandwwraith · 2024-05-08T13:52:09Z

...t/kotlin/kotlinx/kover/gradle/plugin/test/functional/cases/ReportInheritedFromFilterTests.kt

+                classCounter("org.jetbrains.CloseableClass").assertAbsent()
+                classCounter("org.jetbrains.BChild").assertAbsent()
+                classCounter("org.jetbrains.D").assertAbsent()
+                classCounter("org.jetbrains.DChild").assertAbsent()


Also classCounter("org.jetbrains.A").assertAbsent()?

I would add to the documentation that using include { inheritedFrom("com.app.A") } would not add A itself to the filter.

sandwwraith · 2024-05-08T14:00:31Z

kover-gradle-plugin/src/main/kotlin/kotlinx/kover/gradle/plugin/dsl/KoverReportsConfig.kt

@@ -650,19 +651,29 @@ public interface KoverReportFilter {
    /**
     * Add to filters all classes and functions marked by specified annotations.
     *
+     * Use cases:
+     *  - in case of exclusion filters all classes or function marked by at least one of the specified annotation will be excluded from the report


'all classes or function marked by at least one of the specified annotation will be excluded from the report' and 'classes marked by at least one of the specified annotations will be included in the report' looks like repeating itself. Why do you think that 'Add to filters all classes and functions marked by specified annotations.' is not enough?

Because now you can exclude class and function by annotation, but you can't include function by annotation, only whole class.

sandwwraith · 2024-05-08T14:00:58Z

kover-gradle-plugin/src/main/kotlin/kotlinx/kover/gradle/plugin/dsl/KoverReportsConfig.kt

+    /**
+     * Filter classes extending at least one of the specified classes or implementing at least one of the interfaces.
+     *
+     * The entire inheritance tree is analyzed, that is, a class may not extend the specified class directly.


Copy my comments from the similar section in docs

Co-authored-by: Leonid Startsev <sandwwraith@users.noreply.github.com>

shanshin requested a review from sandwwraith March 22, 2024 13:39

sandwwraith requested changes Mar 26, 2024

View reviewed changes

shanshin requested a review from sandwwraith March 26, 2024 18:09

shanshin mentioned this pull request Mar 27, 2024

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

Merged

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

Base automatically changed from features-instead-intellij to main March 27, 2024 18:33

shanshin force-pushed the extended-filters branch from 01d4263 to 779b271 Compare May 1, 2024 12:47

shanshin and others added 9 commits May 4, 2024 19:53

Apply suggestions from code review

2521099

Co-authored-by: Leonid Startsev <sandwwraith@users.noreply.github.com>

~fixes

676a24a

~fix test

351c3e9

~rebased

b7f762e

~fix tests

95b74a7

~dump API

47e1652

~fix

0b8c7d9

~rebase

e995c36

shanshin force-pushed the extended-filters branch from 5c2dca6 to e995c36 Compare May 4, 2024 18:04

sandwwraith requested changes May 8, 2024

View reviewed changes

shanshin and others added 2 commits May 8, 2024 18:32

Apply suggestions from code review

401c3ef

Co-authored-by: Leonid Startsev <sandwwraith@users.noreply.github.com>

~review fixes

8d61484

shanshin requested a review from sandwwraith May 8, 2024 16:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added extended reports filtering #581

Added extended reports filtering #581

shanshin commented Mar 22, 2024

sandwwraith Mar 26, 2024

sandwwraith Mar 26, 2024

sandwwraith Mar 26, 2024

shanshin Mar 26, 2024

sandwwraith Mar 26, 2024

shanshin Mar 26, 2024

sandwwraith Mar 26, 2024

shanshin commented Mar 26, 2024

sandwwraith commented Mar 27, 2024

sandwwraith May 8, 2024

shanshin May 8, 2024

sandwwraith May 8, 2024

sandwwraith May 8, 2024

shanshin May 8, 2024

sandwwraith May 8, 2024

shanshin May 8, 2024

sandwwraith May 8, 2024

sandwwraith May 8, 2024

sandwwraith May 8, 2024

shanshin May 8, 2024

sandwwraith May 8, 2024

Added extended reports filtering #581

Are you sure you want to change the base?

Added extended reports filtering #581

Conversation

shanshin commented Mar 22, 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

shanshin commented Mar 26, 2024

sandwwraith commented Mar 27, 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