Kotlin / kotlinx-kover

Apache License 2.0
1.25k stars 46 forks source link

0.8.0-Beta DSL confusion between `minBound` and `minValue`. #572

Closed mgroth0 closed 1 month ago

mgroth0 commented 1 month ago

I have spent a long time being very confused, stepping through debuggers, and creating a reproducer only to find out that I was using the DSL incorrectly. I think it is a design issue, as it is too easy for users to get mixed up.

I was doing this:

kover {
    reports {
        verify {
            rule {
                bound {
                    coverageUnits.set(CoverageUnit.BRANCH)
                    minBound(75)
                }
            }
        }
    }
}

And my output kept looking like this:

Rule violated: lines covered percentage is 12.500000, but expected minimum is 75

I was very confused why it kept using lines instead of branches. Finally, I discovered I needed to do this:

kover {
    reports {
        verify {
            rule {
                bound {
                    coverageUnits.set(CoverageUnit.BRANCH)
                    minValue = 75 // <-- Change here
                }
            }
        }
    }
}

And now it works:

Rule violated: branches covered percentage is 0.000000, but expected minimum is 75

I think that the correct solution here is to use the @DslMarker annotation on all DSL objects. I mean, create an annotation called KoverDsl, annotate that annotation with @DslMarker, and put KoverDsl on all kover dsl interfaces.

shanshin commented 1 month ago

Hi, yes, in fact, it was worth adding an @DslMarker annotation a long time ago.

But could you clarify, having encountered the problem, have you looked at the documentation for the bound function? It may also be worth improving the documentation for it somehow.

mgroth0 commented 1 month ago

I did not look at the documentation, no. I was creating a new reproducer project and was already familiar with the kover API so I was going off memory. However, my memory was slightly off. It seem like an easy mistake to make.

shanshin commented 1 month ago

Implemented in 0.8.0-Beta2