update coverage workflow

[danieleades]

llvm-cov is better maintained, and supports doc-tests coverage

[danieleades]

@ar3s3ru how attached are you to the cobertura.xml format? what do you use the archived coverage reports for?

[codecov]

Codecov Report

Base: 78.60% // Head: 87.15% // Increases project coverage by +8.54% 🎉

Coverage data is based on head (8fd7834) compared to base (8ca80dd).
Patch has no changes to coverable lines.

❗ Current head 8fd7834 differs from pull request most recent head 0858313. Consider uploading reports for the commit 0858313 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #244      +/-   ##
==========================================
+ Coverage   78.60%   87.15%   +8.54%     
==========================================
  Files          18       18              
  Lines         589     1160     +571     
==========================================
+ Hits          463     1011     +548     
- Misses        126      149      +23     

Impacted Files	Coverage Δ
eventually/src/message.rs	92.30% <0.00%> (-7.70%)	⬇️
eventually-postgres/src/lib.rs	86.36% <0.00%> (-5.31%)	⬇️
eventually/src/command/test.rs	98.41% <0.00%> (-1.59%)	⬇️
eventually/src/aggregate.rs	89.89% <0.00%> (-0.43%)	⬇️
eventually/src/aggregate/repository.rs	88.88% <0.00%> (-0.40%)	⬇️
eventually/src/serde.rs	0.00% <0.00%> (ø)
eventually/src/tracing.rs	0.00% <0.00%> (ø)
eventually/src/serde/prost.rs	0.00% <0.00%> (ø)
eventually/src/serde/protobuf.rs	0.00% <0.00%> (ø)
eventually-postgres/tests/setup/mod.rs
... and 11 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[ar3s3ru]

Hey @danieleades, welcome back!

Not particularly attached to any specific format.
However, couple of things:

• I would stay on stable to generate coverage; if a tool does not work on stable, I don't think we should use it yet honestly,
• cargo-tarpaulin is still pretty active, and seems to me a more stable project than cargo-llvm-cov (also, supports llvm backend); any specific reason to change to that solution? I may be missing some context (not very active in the overall Rust ecosystem).

[danieleades]

• I would stay on stable to generate coverage; if a tool does not work on stable, I don't think we should use it yet honestly,

I don't think there's any reason to avoid using the nightly compiler in CI. If you're really worried you can pin to a specific version of the nightly compiler. What problems are you trying to avoid by not using the nightly compiler in CI?
cargo-llvm-cov itself is not unstable, nor does it need nightly for its core feature set. It relies on nightly compiler features to produce coverage for doctests. tarpaulin doesn't support doc test coverage at all.

• cargo-tarpaulin is still pretty active, and seems to me a more stable project than cargo-llvm-cov (also, supports llvm backend); any specific reason to change to that solution? I may be missing some context (not very active in the overall Rust ecosystem).

llvm-cov is a much simpler implementation, has been stable for years. Anecdotally it produces much more accurate coverage information. Tarpaulin does now support the LLVM-based coverage, but this is brand spanking new (as in, in the last couple of months. the tracking issue is still open).

[ar3s3ru]

What problems are you trying to avoid by not using the nightly compiler in CI?

nightly breaks unexpectedly, as it actually did while I was developing some stuff. Had to disable nightly merge check in the pipeline to make it pass CI (context: 67ed3d2).

So I honestly prefer making coverage running in stable a strong requirement.

In the case of cargo-llvm-cov it seems not to be a huge deal, since I don't think we have many doctests here (or none at all actually), so I can live with it running in stable.

And thank you for the context on Tarpaulin. Seems like I have some digging around to do to get more context. I'll get back to this PR once I do that.

[danieleades]

What problems are you trying to avoid by not using the nightly compiler in CI?

nightly breaks unexpectedly, as it actually did while I was developing some stuff. Had to disable nightly merge check in the pipeline to make it pass CI (context: 67ed3d2).

So I honestly prefer making coverage running in stable a strong requirement.

It sounds like pinning to a specific nightly version might be the most appropriate solution

In the case of cargo-llvm-cov it seems not to be a huge deal, since I don't think we have many doctests here (or none at all actually), so I can live with it running in stable.

That's true, but there's also a massive amount of clippy warnings about the lack of documentation...
Having the doctests coverage in place means you can see the impact on coverage as doctests/examples are added!

And thank you for the context on Tarpaulin. Seems like I have some digging around to do to get more context. I'll get back to this PR once I do that.

no worries. My own experience with tarpaulin is that it works, and the maintaineris very responsive, but it relies on a lot of undocumented or unspecified cargo/rustc features and is therefore very brittle.

[ar3s3ru]

Seems like tarpaulin is creating issues in CI, so I'm looking at this PR again (sorry, these long hiatuses mean stressful days at work 😅).

Would you be ok to change to stable toolchain, as we discussed above?
As I said, we don't have many doctests now so I'd be ok with having it running without it.

If you cannot make the change, I hope it's ok if I make the change myself and merge the PR 😄