CI: Run tests via cargo-tarpaulin

[Turbo87]

This PR adds https://github.com/actions-rs/tarpaulin to our CI pipeline to display a test coverage report.

Resolves #1162

[rust-highfive]

r? @carols10cents

(rust_highfive has picked a reviewer for you, use r? to override)

[jtgeibel]

cc @pietroalbini as I think you had some concerns about running 3rd party actions on CI.

[pietroalbini]

In general I'd prefer if we avoid using third-party actions, as those could be yet another avenue to inject malicious code into the build.

[Turbo87]

can we make them first-party actions?

[JohnTitor]

If the issue is to use "the third party actions", we could use cargo-tarpaulin directly.

[pietroalbini]

Without using the external action this looks good on my side!

[Turbo87]

unfortunately this still has two issues:

1. it doesn't actually work (yet?): 0.00% coverage, 0/2661 lines covered

2. it currently takes 12 minutes to install tarpaulin on CI and then run the tests in it, which seems like quite a lot 😞

[JohnTitor]

it doesn't actually work (yet?): 0.00% coverage, 0/2661 lines covered

Hmm, I'm not sure at all.

it currently takes 12 minutes to install tarpaulin on CI and then run the tests in it, which seems like quite a lot

So, in general, releases don't happen that often and it'd solve the issue to save the binary in the cache.

[Turbo87]

@xd009642 any clue what might be the problem here? 🙏

[xd009642]

oh sorry I realise I've neglected the issue my end. I remember when I tried it last I was struggling to get the backend tests working on fedora but I've got an ubuntu laptop here so I'll try again. I'll tag you on xd009642/tarpaulin#601 if I have any questions just to avoid filling up the PR comments 👍

[xd009642]

Couple of comments on the PR from working on sorting the coverage issue in github actions:

1. Either add --force to the cargo install or check for the existence of tarpaulin first otherwise the install fails
2. You probably only want to run coverage for one specific toolchain, otherwise you'll have multiple reports per commit and I'm not sure if any services like coveralls or codecov support that

[Turbo87]

looks like this is ready to go now! thanks again @xd009642 :)

r? @pietroalbini

[JohnTitor]

This workaround is still needed on CI? I think recent cargo doesn't require it anymore.

[xd009642]

It failed for me running the CI twice consecutively yesterday on my test project to debug the 0% coverage https://github.com/xd009642/crates-io-test/actions

[JohnTitor]

Hm, that's interesting, I guess cargo recognizes it as an untracked file since it's introduced by cache (or another reason) 🤔. Thanks for clarifying, then I'm fine as-is.

[simonsan]

Wouldn't it be reasonable to cache tarpaulin? There is no need to rebuild it every CI run I feel?

https://github.com/marketplace/actions/cache#skipping-steps-based-on-cache-hit
https://github.com/actions/cache/blob/main/examples.md#rust---cargo

[Turbo87]

AFAICT

crates.io/.github/workflows/ci.yml

Lines 119 to 123 in 2b3bdf1

	- name: Cache cargo binaries
	uses: actions/cache@v2
	with:
	path: ~/.cargo/bin
	key: ${{ runner.os }}-cargo-bin-${{ matrix.rust }}-${{ hashFiles('.diesel_version') }}

should already cache the binaries including diesel-cli and cargo-tarpaulin

[simonsan]

Ahh, sorry then. Must have overlooked that part :/

[jtgeibel]

Alright, lets try this out on master and we'll see how well caching works on new PRs.

@bors r+

[bors]

📌 Commit 23bae68 has been approved by jtgeibel

[bors]

⌛ Testing commit 23bae68 with merge e719e04...

[bors]

☀️ Test successful - checks-actions
Approved by: jtgeibel
Pushing e719e04 to master...