informalsystems / tendermint-rs

Client libraries for Tendermint/CometBFT in Rust!
Apache License 2.0
610 stars 224 forks source link

Research and implement tarpaulin instead of grcov #252

Closed greg-szabo closed 3 years ago

greg-szabo commented 4 years ago

Not a done deal, but according to Tony, tarpaulin has less false negatives. https://github.com/xd009642/tarpaulin

liamsi commented 4 years ago

related: https://github.com/informalsystems/tendermint-rs/issues/250

thanethomson commented 3 years ago

I'm assuming this will be superseded by #811?

thanethomson commented 3 years ago

Since our code coverage seems broken (and seems like it has been broken for some time), we should consider fixing it or integrating tarpaulin-based coverage calculation sooner rather than later.

@romac are you planning on taking #811 further? What are the pros/cons at this point of going with cargo-llvm-cov? I see it's still labelled as being "experimental" right now?

romac commented 3 years ago

are you planning on taking #811 further?

I wish I could but I don't have the bandwidth at the moment.

What are the pros/cons at this point of going with cargo-llvm-cov?

Pros:

I see it's still labelled as being "experimental" right now?

The wrapper itself is marked as experimental, but it basically only wraps cargo build (adding nightly-only options), llvm-profdata, and llvm-cov.

romac commented 3 years ago

By the way, we use tarpaulin on ibc-rs but have seen no real improvements (eg. the coverage still jumps around by 30% sometimes, etc) hence why I was looking into source-based code coverage.

thanethomson commented 3 years ago

Currently superseded by #840.

We can reopen this issue again in future if we want to reconsider using tarpaulin.