Kotlin / kotlinx-kover

Apache License 2.0
1.28k stars 48 forks source link

Kover thinks there are 7 million branches #278

Closed NikolayMetchev closed 1 year ago

NikolayMetchev commented 1 year ago

Describe the bug I have attached a source file which for some reason seems to think there are 7 million branches causing our code coverage numbers to be very low. I have also attached the xml file produced by the kover tool. Jacoco produces the same problem which makes me think this is a kotlin problem but I didn't know where to file it. Here is the relevant section from the xml file:

<method name="invokeSuspend" desc="(Ljava/lang/Object;)Ljava/lang/Object;">
<counter type="INSTRUCTION" missed="0" covered="48"/>
<counter type="BRANCH" missed="7615047" covered="0"/>
<counter type="LINE" missed="0" covered="7"/>
</method>
<counter type="INSTRUCTION" missed="0" covered="48"/>
<counter type="BRANCH" missed="7615047" covered="0"/>
<counter type="LINE" missed="0" covered="7"/>
<counter type="METHOD" missed="0" covered="1"/>
</class>

Errors If present, stacktraces or files from build/kover/errors directory

Expected behavior There should be far less than 7 million branches

Reproducer Not sure how to reproduce for a small case. Hopefully the code and xml file give you some pointers.

Reports If applicable, report files or screenshots.

Environment

zuevmaxim commented 1 year ago

@NikolayMetchev Could you please share com/paxos/metalsapi/routes/LedgerRoutes$addRoutes$1$39.class file?

NikolayMetchev commented 1 year ago

@zuevmaxim I have shared it. it is in LedgerRoutes.kt.gz

NikolayMetchev commented 1 year ago

oh you mean the actual class file? sorry bear with me.

NikolayMetchev commented 1 year ago

LedgerRoutes.tar.gz

shanshin commented 1 year ago

@NikolayMetchev, may you also send the contents of the directory build/kover? Binary coverage reports are located there, for each test task there should be its own .ic file

NikolayMetchev commented 1 year ago

kover.zip

NikolayMetchev commented 1 year ago

@shanshin you tagged the issue as confirmed. Can you clarify what the problem is? Is there possibly a workaround? What are the chances of a fix?

shanshin commented 1 year ago

@NikolayMetchev, yes, the problem is really present, at the moment we are trying to find the cause

shanshin commented 1 year ago

@NikolayMetchev, after the build, is the build/kover/errors directory empty in the project where the incorrect report is generated?

shanshin commented 1 year ago

Is it possible that in your test the code inside post("/ledger/advices/process/{id}") has been called more than 2 billion times?

NikolayMetchev commented 1 year ago

Hi, there isn't a directory build/kover/errors at all. That endpoint is not called 2 billion times, at most it is a few hundred.

NikolayMetchev commented 1 year ago

Yeah, I checked. It is called roughtly 10-12 times. Using io.ktor.server.testing.TestApplicationEngine

zuevmaxim commented 1 year ago

@NikolayMetchev Could you please try a new version of the coverage agent?

for Kotlin Gradle script

kover {
    engine.set(kotlinx.kover.api.IntellijEngine("1.0.689"))
}

for Groovy

kover {
    engine = kotlinx.kover.api.IntellijEngine("1.0.689")
}

If the issue persists, please share the content of build/kover and build/kover/errors directories

NikolayMetchev commented 1 year ago

Hi I am using the groovy version, but for some reason I can't get it to work. I get the following error:

kover {
    engine = kotlinx.kover.api.IntellijEngine(libs.versions.intellijCoverageAgent.get())
}
NikolayMetchev commented 1 year ago

I think the problem is that some parts are internal such as CoverageEngineVendor

shanshin commented 1 year ago

@NikolayMetchev, try

kover {
    engine = new kotlinx.kover.api.IntellijEngine("1.0.689")
} 
NikolayMetchev commented 1 year ago

we look to have made progress. Down to 2 million from 7.:


<method name="invokeSuspend" desc="(Ljava/lang/Object;)Ljava/lang/Object;">
<counter type="INSTRUCTION" missed="0" covered="48"/>
<counter type="BRANCH" missed="2136016" covered="0"/>
<counter type="LINE" missed="0" covered="7"/>
</method>
<counter type="INSTRUCTION" missed="0" covered="48"/>
<counter type="BRANCH" missed="2136016" covered="0"/>
<counter type="LINE" missed="0" covered="7"/>
<counter type="METHOD" missed="0" covered="1"/>
</class>```
NikolayMetchev commented 1 year ago

no errors

zuevmaxim commented 1 year ago

@NikolayMetchev could you please share the content of build/kover ? I need .ic file from there

NikolayMetchev commented 1 year ago

kover (1).zip

zuevmaxim commented 1 year ago

@NikolayMetchev Thank you for the attached files! I have analyzed them, and I see that after the tests run, the coverage report already includes an enormous number of branches, but there just a few in the byte code. Still, I cannot understand the reason for it, and cannot reproduce the issue with the attached class files. So, without a reproducer, it is unclear where the bug comes from.

Is your project open source? If so, could you please share a link to it?

If not, we would be happy to get some more clues from you:

NikolayMetchev commented 1 year ago

Hi, It seems there was an error file. I will spend some time today trying to create a reproducer. Maybe this log file will help you figure out what is wrong? test.log

NikolayMetchev commented 1 year ago

Here some more information. I just tried to reproduce locally (M1 Mac). In order to do that I had to ignore some tests as they only pass in CI due to locale issues. In CI we use linux debian and Intel CPUs. If I filter locally to just a few tests that actually call the function in question the problem doesn't reproduce. If I run the entire test suite locally a different class has a problem of much smaller magnitude see below. I have attached the test.ic and report.xml from the local run. The error file is attached in the previous comment. I will now try and run with Jacoco and see if the same thing happens. This feels like there is a race condition somewhere which will make it very hard for me to make a small reproducer.

[report.xml.gz](https://github.com/Kotlin/kotlinx-kover/files/10112385/report.xml.gz)

[test.ic.gz](https://github.com/Kotlin/kotlinx-kover/files/10112381/test.ic.gz)

<class name="com/paxos/metalsapi/routes/LedgerRoutes$addRoutes$1$38" sourcefilename="LedgerRoutes.kt">
<method name="invokeSuspend" desc="(Ljava/lang/Object;)Ljava/lang/Object;">
<counter type="INSTRUCTION" missed="0" covered="56"/>
<counter type="BRANCH" missed="27450" covered="0"/>
<counter type="LINE" missed="0" covered="7"/>
</method>
<counter type="INSTRUCTION" missed="0" covered="56"/>
<counter type="BRANCH" missed="27450" covered="0"/>
<counter type="LINE" missed="0" covered="7"/>
<counter type="METHOD" missed="0" covered="1"/>
</class>
zuevmaxim commented 1 year ago

@NikolayMetchev Do you have tests running in a fork? maxParallelForks or forkEvery options?

NikolayMetchev commented 1 year ago

Yes I do:

    maxParallelForks = Runtime.runtime.availableProcessors().intdiv(2) ?: 1
NikolayMetchev commented 1 year ago

As a workaround I am trying to filter that class from the xml report but no incantation seems to be working. Here is my latest attempt:

kover {
    filters {
        classes {
            excludes.addAll('com.paxos.metalsapi.routes.LedgerRoutes.addRoutes.1.*')
        }
    }
    xmlReport {
        overrideFilters {
            classes {
                excludes.addAll('com.paxos.metalsapi.routes.LedgerRoutes.addRoutes.1.*')
            }
        }
    }
}

Please can you tell me if it is possible to exclude that class from the xml report?

zuevmaxim commented 1 year ago

I suppose it should be com.paxos.metalsapi.routes.LedgerRoutes\$addRoutes\$1\$*

NikolayMetchev commented 1 year ago

Thank you that worked!

NikolayMetchev commented 1 year ago

One annoying thing is that changing the filter causes the tests to be re-run (cache miss in gradle). Is this intended behaviour?

shanshin commented 1 year ago

One annoying thing is that changing the filter causes the tests to be re-run (cache miss in gradle). Is this intended behaviour?

This is expected behavior, because in version 0.6.1, changing the report filters also changes the instrumentation filter of the classes being run, so you need to rerun the test to collect coverage of only the classes being instrumented.

NikolayMetchev commented 1 year ago

Good to know. Thanks for the clarification.

NikolayMetchev commented 1 year ago

Perhaps you need to add logging as a feature so we can debug?

zuevmaxim commented 1 year ago

@NikolayMetchev Could you please try to rerun with maxParallelForks=1 ? And upload a file with errors

shanshin commented 1 year ago

@NikolayMetchev, is the problem still reproducible on the 1.0.709 version?

NikolayMetchev commented 1 year ago

sorry about not responding earlier. It went off my radar. Will try running with the latest version shortly. (I will need to reproduce first to make sure I have it correct again).

NikolayMetchev commented 1 year ago

The issue doesn't seem to reproduce any longer with version 1.0.689. A few things have changed since the last time I tested it:

Gradle 7.5.1 Kotlin 1.8.0

I will now try it with 1.0.709 to see if that behaves differently

NikolayMetchev commented 1 year ago

Yep, looks like it may get fixed by upgrading Gradle, kotiln or possibly just changing the code. I can't be sure what has fixed this issue but I can't reproduce it anymore.

shanshin commented 1 year ago

Thanks!

Feel free to open new bug issue if the problem reproduced again.