Update github workflow to upload test coverage reports #759

Chralt98 · 2022-08-17T08:43:07Z

Fixes #263.

https://blog.rust-lang.org/inside-rust/2020/11/12/source-based-code-coverage.html

Chralt98 · 2022-08-17T12:33:56Z

@sea212 @lsaether Is it okay to use and sign up on https://docs.codecov.com/docs#step-1-sign-up-for-codecov ? I have no admin access to the repository to integrate Codecov.

If you are signing up via GitHub, you may need to request access from an admin to authorize Codecov as a third-party application. For more information see GitHub OAuth Admin Authorization or follow our video guide.

codecov-commenter · 2022-08-17T19:58:52Z

Codecov Report

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

@@           Coverage Diff           @@
##             main     #759   +/-   ##
=======================================
  Coverage        ?   92.23%           
=======================================
  Files           ?       80           
  Lines           ?    14062           
  Branches        ?        0           
=======================================
  Hits            ?    12970           
  Misses          ?     1092           
  Partials        ?        0

Flag	Coverage Δ
tests	`92.23% <0.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

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

vivekvpandya

It looks fine to me. I hope total turn around time is not increased much. Also I am wondering why grcov is used instead of llvm-cov. llvm-cov is available and it makes commands much simple with this cargo wrapper https://github.com/taiki-e/cargo-llvm-cov
however I am not able to find any comparison of grcov vs llvm-cov , only thing is that grcov is written in rust.

maltekliemann · 2022-08-18T18:41:46Z

Just a dumb question: The report says 92.37% coverage. What exactly does that number mean? There seem to be two different workflows running, one for unit tests and one for fuzz tests, so I would have expected at least two numbers... right?

Also, doesn't this number seem surprisingly high? Can we get a complete list of lines of code reached/not reached?

sea212

Does CodeCov still need access to our repository?

sea212 · 2022-08-30T12:48:00Z

.github/workflows/rust.yml

+      - name: Download grcov
+        run: |
+          mkdir -p "${HOME}/.local/bin"
+          curl -sL https://github.com/mozilla/grcov/releases/download/v0.8.11/grcov-x86_64-unknown-linux-gnu.tar.bz2 | tar jxf - -C "${HOME}/.local/bin"
+          echo "$HOME/.local/bin" >> $GITHUB_PATH


Can we use the grcov GH action instead?

I tried it. When running the grcov github action it always returns the following error.

Error: Unable to find any coverage files, was `cargo test` executed correctly?

Although I executed cargo test as step before the grcov github action. I don't really know how to solve this.

In the example they use a github action for cargo test, but we use a script instead. But shouldn't be the result the same?

I couldn't find an example for grcov GH action for source-based coverage.

.github/workflows/rust.yml

sea212 · 2022-09-05T12:31:08Z

.github/workflows/rust.yml

+      - name: Upload to codecov.io
+        uses: codecov/codecov-action@v3
+        with:
+          files: /misc/tests.lcov


Might want to use /tmp/tests.lcov instead to avoid potential permission issues.

addressed in eb06f35

Chralt98 · 2022-09-12T11:07:00Z

Does CodeCov still need access to our repository?

That's a good question. I don't really know. I would assume no, because Codecov could comment on this PR, so I think it has been already added by Logan.

Chralt98 · 2022-09-12T11:12:19Z

The report says 92.37% coverage. What exactly does that number mean?

@maltekliemann I did use the lcov file and produced a html website out of it. And correct me if I am wrong, but I think we reached a code coverage of that level for our tests. I try to get another message from codecov, in which you can click on the report to see the code coverage for the different crates. The current message of Codecov is not correct at the moment. I wonder if this happens, when codecov is not initialised on the main branch. I need investigation time for that.

I would assume, that we need codecov on main, because of the error message from the codecov comment.

No coverage uploaded for pull request base (main@0e27079).

Chralt98 · 2022-09-26T14:01:59Z

https://rust-fuzz.github.io/book/cargo-fuzz/coverage.html

Fuzz test coverage is extremely slow with the current version. I don't know how to solve this.

The fuzz coverage is outdated. Unfortunately I can't append my own compiler flags to get the result of code coverage for fuzz tests. rust-fuzz/cargo-fuzz#320

For the fuzz tests, the proposed way of cargo-fuzz always produces an empty lcov file. So the coverage doesn't seem to work at least for fuzz tests.

When I run cargo fuzz coverage --release --fuzz-dir zrml/swaps/fuzz pool_join for each fuzz target, then it takes extremely long to finish, because each corpus element runs. Even in release mode it takes so long.

For the normal tests I think the result is reasonable. I know, that there are some bugs like you Malte found out, but I think it's the fault of the current Substrate environment. At least we can detect paths, which are not covered at all from the tests. That's useful.

Chralt98 · 2022-09-27T07:08:49Z

scripts/tests/aux-functions.sh

@@ -15,5 +15,6 @@ test_package_with_feature() {
    local features=$2

    /bin/echo -e "\e[0;33m***** Testing '$package' with features '$features' *****\e[0m\n"
-    cargo test --features $features --manifest-path $package/Cargo.toml --no-default-features --release


We should avoid profile release for tests, so that debug_assert! macros are activated and we can see, that something is not as expected.

https://doc.rust-lang.org/cargo/reference/profiles.html#release

Chralt98 · 2022-09-27T07:58:38Z

I tried https://github.com/taiki-e/cargo-llvm-cov out @vivekvpandya . It had exactly the same result as we got with the current solution. Thanks for your investigation! I think it's fine as it is. With using cargo-llvm-cov (because it's a wrapper around cargo test) it's not so clear what actually happens (so the future maintainers would need to look up cargo-llsm-cov to understand that it's basically cargo test with code coverage and grcov).

Chralt98 · 2022-09-27T08:35:01Z

We got a slightly better time for the current fuzz workflow. It's now 56 minutes instead of on the main branch 66 minutes. This is because of the --release profile for the swaps fuzz tests. Just in case if somebody asks about if compile time with dev (debug) beats the speed of --release.

Chralt98 added 8 commits August 17, 2022 10:40

update github workflow to upload coverage reports

4a438c2

avoid using xtask

423cc45

use relative path for codecov CI workflow

8195c5e

improve relative paths

334b510

update relative paths

7bc87b6

update path

a114cdf

use coverage for fuzz

2c6c66a

update lcov path

eb797dd

Chralt98 self-assigned this Aug 17, 2022

Chralt98 marked this pull request as ready for review August 17, 2022 13:20

Chralt98 requested review from maltekliemann, vivekvpandya and sea212 August 17, 2022 13:21

Chralt98 added the s:review-needed The pull request requires reviews label Aug 17, 2022

use release flag for fuzz tests and grcov

7a1df50

vivekvpandya previously approved these changes Aug 18, 2022

View reviewed changes

sea212 reviewed Sep 5, 2022

View reviewed changes

use grcov github action

1d40e59

Chralt98 added s:in-progress The pull requests is currently being worked on and removed s:review-needed The pull request requires reviews labels Sep 12, 2022

Chralt98 added 5 commits September 12, 2022 13:16

Merge branch 'main' into chralt98-code-coverage

5b63c2c

use grcov config

90b452d

use grcov github action test suggestion

13269b5

revert to source based code coverage

f57c2e0

create tmp directory

eb06f35

Chralt98 added 4 commits September 13, 2022 13:22

optimise github actions

8bd17aa

change github workflow

d039f16

fix github actions workflow

db85aa9

provide flag for github action codecov

f51da52

Chralt98 added s:review-needed The pull request requires reviews s:in-progress The pull requests is currently being worked on and removed s:in-progress The pull requests is currently being worked on s:review-needed The pull request requires reviews labels Sep 13, 2022

use debug profile for tests for better cov reports

6d58521

Chralt98 dismissed stale reviews from sea212 and vivekvpandya via 6d58521 September 26, 2022 11:53

add fuzz coverage

b741e76

Chralt98 commented Sep 27, 2022

View reviewed changes

Chralt98 added 3 commits September 27, 2022 09:24

remove fuzz coverage

56ebfdb

add RUST_BACKTRACE to cargo fuzz

684c124

update cargo test comment

0759863

Chralt98 requested a review from vivekvpandya September 27, 2022 07:59

Chralt98 added s:review-needed The pull request requires reviews and removed s:in-progress The pull requests is currently being worked on labels Sep 27, 2022

maltekliemann removed their request for review October 2, 2022 16:06

Merge branch 'main' into chralt98-code-coverage

c21e5a9

sea212 approved these changes Jan 6, 2023

View reviewed changes

vivekvpandya approved these changes Jan 6, 2023

View reviewed changes

sea212 added s:accepted This pull request is ready for merge and removed s:review-needed The pull request requires reviews labels Jan 6, 2023

Chralt98 merged commit 85cefa6 into main Jan 6, 2023

Chralt98 deleted the chralt98-code-coverage branch January 6, 2023 14:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update github workflow to upload test coverage reports #759

Update github workflow to upload test coverage reports #759

Chralt98 commented Aug 17, 2022 •

edited

Loading

Chralt98 commented Aug 17, 2022 •

edited

Loading

codecov-commenter commented Aug 17, 2022 •

edited

Loading

vivekvpandya left a comment

maltekliemann commented Aug 18, 2022 •

edited

Loading

sea212 left a comment

sea212 Aug 30, 2022

Chralt98 Sep 12, 2022 •

edited

Loading

Chralt98 Sep 12, 2022

sea212 Sep 5, 2022

Chralt98 Sep 13, 2022

Chralt98 commented Sep 12, 2022

Chralt98 commented Sep 12, 2022 •

edited

Loading

Chralt98 commented Sep 26, 2022 •

edited

Loading

Chralt98 Sep 27, 2022 •

edited

Loading

Chralt98 commented Sep 27, 2022 •

edited

Loading

Chralt98 commented Sep 27, 2022

Update github workflow to upload test coverage reports #759

Update github workflow to upload test coverage reports #759

Conversation

Chralt98 commented Aug 17, 2022 • edited Loading

Chralt98 commented Aug 17, 2022 • edited Loading

codecov-commenter commented Aug 17, 2022 • edited Loading

Codecov Report

vivekvpandya left a comment

Choose a reason for hiding this comment

maltekliemann commented Aug 18, 2022 • edited Loading

sea212 left a comment

Choose a reason for hiding this comment

sea212 Aug 30, 2022

Choose a reason for hiding this comment

Chralt98 Sep 12, 2022 • edited Loading

Choose a reason for hiding this comment

Chralt98 Sep 12, 2022

Choose a reason for hiding this comment

sea212 Sep 5, 2022

Choose a reason for hiding this comment

Chralt98 Sep 13, 2022

Choose a reason for hiding this comment

Chralt98 commented Sep 12, 2022

Chralt98 commented Sep 12, 2022 • edited Loading

Chralt98 commented Sep 26, 2022 • edited Loading

Chralt98 Sep 27, 2022 • edited Loading

Choose a reason for hiding this comment

Chralt98 commented Sep 27, 2022 • edited Loading

Chralt98 commented Sep 27, 2022

Chralt98 commented Aug 17, 2022 •

edited

Loading

Chralt98 commented Aug 17, 2022 •

edited

Loading

codecov-commenter commented Aug 17, 2022 •

edited

Loading

maltekliemann commented Aug 18, 2022 •

edited

Loading

Chralt98 Sep 12, 2022 •

edited

Loading

Chralt98 commented Sep 12, 2022 •

edited

Loading

Chralt98 commented Sep 26, 2022 •

edited

Loading

Chralt98 Sep 27, 2022 •

edited

Loading

Chralt98 commented Sep 27, 2022 •

edited

Loading