search

rust-vmm / rust-vmm-ci

Apache License 2.0
18 stars 33 forks source link

Is using "function coverage" metric instead of "line coverage" intended? #170

Closed mtjhrc closed 1 day ago

mtjhrc commented 2 days ago

While working with @dorindabassey on a gpu in vhost-device, she has noticed the CI seem to be using the "function coverage" metric of llvm-cov. Then I also noticed the comment in the python code of the CI, that seems to suggest "line coverage" is intended to be used.

https://github.com/rust-vmm/rust-vmm-ci/blob/209c04eb5ebed01b1dc3fbcfcb3ba2d563695773/integration_tests/test_coverage.py#L101-L106 The comment here doesn't seem to be correct - at least on my machine the order of the columns in the CLI output of cargo llvm-cov test --summary-only is: Region coverage, Function coverage, Line coverage.

Confusingly the order of the columns is different when using the html output (cargo llvm-cov test --open): Function coverage, Line coverage, Region coverage.

Btw there is also cargo llvm-cov test --json --summary-only which could prevent such confusion.

This means that it seems like the projects using this CI, use the function coverage metric instead of line coverage. Is this intended or it is it a bug in the parsing of the output? I would think the original intention was to use line coverage.

roypat commented 2 days ago

Hey, thanks for reporting this! Yes, that is indeed supposed to be line coverage! I wonder something changed on llvm-cov's side recently (there was a rust toolchain update that caused weird changes of the coverage numbers all over the place). The json option does indeed sound more robust. I'll have a look into fixing this tomorrow morning (although if you already have a fix, please feel free to open a PR and I'll have a look at that instead :)