Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set up code coverage properly #740

Merged
merged 18 commits into from
Dec 6, 2021
Merged

Set up code coverage properly #740

merged 18 commits into from
Dec 6, 2021

Conversation

clux
Copy link
Member

@clux clux commented Dec 5, 2021

Getting it from 45% to 55% then 60% by running some of the ignored lib tests in k3d on github actions and then porting a few example basics into ignored tests. Also moved from coveralls to codecov since it seems more modern (and was easy to plumb with the action).

codecov

We should have a higher coverage than what it shows if we account for examples, but we need to either add an extra coverage run for examples (it is very slow with building all), or port some example tests into kube (like what I did for crd_api and dynamic_api here).

Ultimately, what's shown here is a tradeoff. Doc tests are slow, seems to require nightly. Example tests are slow because we need to build and run all of them sequentially. It's probably better to focus on coverage the traditional way with some core tests in kube or the relevant crates.

It doesn't seem to count kube-derive properly. Maybe it's possible to tweak the command to get that in. See config "reference"

Commands to replicate what is running here (minus output format) is:

cargo tarpaulin -- --test-threads 1

with cargo-tarpaulin 0.18.5 and the new embedded tarpaulin.toml to encapsulate flags (needed a lot of tweaks to get the ignored, feature-dependent tests in kube/src/lib to run).

For now this is at least enough to satisfy the "most code is covered" requirement from #737.

clux added 7 commits December 5, 2021 00:15
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
still not getting all the results i want because some ignored tests are
not running

Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
@clux clux mentioned this pull request Dec 5, 2021
6 tasks
@codecov-commenter
Copy link

codecov-commenter commented Dec 5, 2021

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #740   +/-   ##
=========================================
  Coverage          ?   60.34%           
=========================================
  Files             ?       52           
  Lines             ?     3051           
  Branches          ?        0           
=========================================
  Hits              ?     1841           
  Misses            ?     1210           
  Partials          ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20642bb...03de8ab. Read the comment docs.

clux added 5 commits December 5, 2021 12:47
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
@clux
Copy link
Member Author

clux commented Dec 5, 2021

Can run --doc tests as well, which adds half a percent in coverage (because most of them are marked as no_run), but adds 5 minute to runtime on actions.

clux added 2 commits December 5, 2021 14:00
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
@clux clux requested a review from a team December 5, 2021 15:01
@clux
Copy link
Member Author

clux commented Dec 5, 2021

Managed to get ignored lib tests to run using a tarpaulin.toml config file, and pushed coverage a few more percent without having to turn on nightly and doc tests (which added tons of time). Happy enough with this, but obviously things we could do to make this number a little more attractive.

clux added 2 commits December 5, 2021 15:11
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
@clux
Copy link
Member Author

clux commented Dec 5, 2021

A few more percent, at a respectable 60 now. Going to leave it now, don't want to mess more until there's a base report and we see how the reports come in from the bot as PRs come in. A cool feature of this is that it doesn't make new comments, it just updates the existing comment.

clux added 2 commits December 5, 2021 15:51
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
@clux
Copy link
Member Author

clux commented Dec 6, 2021

I am just going to assume that this is uncontroversial and merge this for now. If the coverage message gets annoying, we can remove the pr/push build, but need to test after merging so we have a baseline.

@clux clux merged commit c88addd into master Dec 6, 2021
@clux clux deleted the coverage2 branch December 6, 2021 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants