chore: add code coverage report

[vmx]

Generate a code coverage report and add it as artifact as well as
uploading it to CodeCov.

[vmx]

I still want to improve it a bit, but I thought I push a first version of it.

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (master@63c5033). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #120   +/-   ##
=========================================
  Coverage          ?   83.95%           
=========================================
  Files             ?       87           
  Lines             ?    18091           
  Branches          ?        0           
=========================================
  Hits              ?    15189           
  Misses            ?     2902           
  Partials          ?        0           

[vmx]

@ZenGround0 @galargh Things work! I'm quite happy with the outcome. The report is uploaded to Codecov and also an artifact is created, in case someone wants the fancy llvm report. I think it would be OK to merge it as it is, though things I'd love to see improved (though I'm not sure if those are possible/worth it:

• An direct link to the Codecov coverage report directly from a PR (without clicking through the CI things
• A link to the LLVM created coverage report. One would need to upload it somewhere (e.g. IPFS or GitHub pages)

[galargh]

An direct link to the Codecov coverage report directly from a PR (without clicking through the CI things

I'd expect #120 (comment) to link to codecov. I'll try to look into why the links lead to nowehere.

A link to the LLVM created coverage report. One would need to upload it somewhere (e.g. IPFS or GitHub pages)

This should be a lot easier once github/roadmap#470 is implemented.

[LesnyRumcajs]

Hello! I may be missing something, but have you considered using the simplistic tarpaulin solution like we've done in Forest? We are pretty happy with the codecov reports generated there, and the workflow seems more maintainable. Is it missing some crucial features?

[vmx]

@LesnyRumcajs last time I've uses Tarpaulin, the results were different, while Tarpaulin was wrong. I don't know if that has changed.

[LesnyRumcajs]

@vmx Do you recall what kind of difference have you observed? Is there an issue with that in the Tarpaulin repository?

[vmx]

@vmx Do you recall what kind of difference have you observed?

I can't recall. I think easiest is to just try it out in this repo and see if the output is different.

[vmx]

@LesnyRumcajs I just tried to compare the output with the one from Tarpaulin. But I can't get Tarpaulin working, when running cargo tarpaulin --workspace it fails to compile (Error: "Failed to compile tests! Error: fil_actors_runtime: linking with cc failed: exit status: 1"). I guess it's due to Tarpauling using -Clink-dead-code which currently doesn't work with this code base.

[LesnyRumcajs]

@vmx Thanks for checking this out! Do you know why this flag breaks the compilation?

[vmx]

Do you know why this flag breaks the compilation?

Sounds to me like some library is already compiled with dead code removal, hence the symbols are not there anymore. But linker issues are always tricky and huge time sink. Though in case someone wants to take a look, it's easy to reproduce via RUSTFLAGS='-Clink-dead-code' cargo test --workspace.

[LesnyRumcajs]

@vmx Interesting, thanks for the insights!

But linker issues are always tricky and huge time sink.

Indeed, there be dragons. :)

[galargh]

I figured out why #120 (comment) doesn't contain the correct links to the reports.

This happens because we run the workflow on push trigger - see

builtin-actors/.github/workflows/ci.yml

Line 4 in ae70a6a

push:

In such a setup, the checkout action only checks out this branch and does not rebase it on the PR target - master in this case. That's why we get ❗ No coverage uploaded for pull request base (master@940d99d). warning and the link under Codecov Report leads nowhere.

To address this we could switch the workflow trigger to pull_request or pull_request_target. We've done a similar change in ref-fvm.

[anorth]

@vmx I've only skimmed the convo above, but is this something you think is ready to land, or could be soon?

[vmx]

@vmx I've only skimmed the convo above, but is this something you think is ready to land, or could be soon?

It was ready from my end (see #120 (comment)), when I handed it over to @galargh.

[vmx]

I case someone is looking into this, https://github.com/taiki-e/cargo-llvm-cov also looks quite promising (I haven't used it yet, though).

[galargh]

To address this we could switch the workflow trigger to pull_request or pull_request_target. We've done a similar change in filecoin-project/ref-fvm#467.

It's been waiting for input on the above suggestion. Now, it's been implemented in #510 so I think there are no outstanding issues with it.

Someone just has to resolve the conflicts - there's nothing complicated to resolve, it's just that I don't have write access to this repository.

[anorth]

Thanks, I've resolved conflicts.

I would prefer much of this were in the Makefile so it was easy for a dev to replicate, but I'll approve this and we can improve later.

[anorth]

@galargh CI failed, something about GH_TOKEN