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

Improve Codecov settings #1629

Draft
wants to merge 32 commits into
base: master
Choose a base branch
from

Conversation

Vampire
Copy link
Member

@Vampire Vampire commented Apr 11, 2023

  • Fail the CI build if the Codecov uploading failed.
  • Show more detailed information in the Codecov comment.
  • Do not update the Codecov comment 13 times, but post it after all 14 reports were uploaded.
  • Ignore dates in coverage reports to not get "upload expired" because the report was taken from remote build cache.

@Vampire Vampire requested a review from leonard84 April 11, 2023 15:40
@codecov
Copy link

codecov bot commented Apr 11, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.86%. Comparing base (2c7db77) to head (b754360).
Report is 107 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1629      +/-   ##
============================================
+ Coverage     80.44%   81.86%   +1.41%     
- Complexity     4337     4575     +238     
============================================
  Files           441      446       +5     
  Lines         13534    14342     +808     
  Branches       1707     1814     +107     
============================================
+ Hits          10888    11741     +853     
+ Misses         2008     1935      -73     
- Partials        638      666      +28     

see 20 files with indirect coverage changes

codecov.yml Outdated

comment:
layout: "reach, diff, flags, files"
after_n_builds: 14
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like it is likely to break if we change the number of builds in the matrices, at least create a comment pointing to this file/line in the workflows.

Is there a way to somehow set this in the workflow and compute the number of builds from the matix?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, if you upload more reports, you will then have comment updates again.
If you upload less reports, you will notice that no comment is coming in.

I personally always was a bit confused when the first comment reduces the coverage and then edits and edits come in that increase the coverage, so I thought I put this in alongside.
Actually I've read it in a newsletter and it was my main intent to set that property. :-D

I don't think you can calculate it from the matrix unless you add a parser that parses the workflow file and interprets it to calculate how many iterations it will be.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could change the workflow files to instead be written in Kotlin using https://github.com/typesafegithub/github-workflows-kt. That's a really nice project I use for all my GitHub workflows now. And that could indeed then be used to calculate the value and generate a step that updates the Codecov file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added one commit that now calculates the value from the build matrix.
But I also marked the PR as "Draft" as it now depends on #1630.

@Vampire Vampire force-pushed the codecov-settings branch 7 times, most recently from 81ef0a3 to c060507 Compare April 19, 2023 00:38
@Vampire Vampire marked this pull request as draft April 19, 2023 00:43
@Vampire Vampire force-pushed the codecov-settings branch 4 times, most recently from 73dcbcf to a6f5543 Compare April 19, 2023 14:03
@Vampire Vampire force-pushed the codecov-settings branch 2 times, most recently from a2c77f9 to 3239882 Compare April 28, 2023 13:09
@Vampire Vampire force-pushed the codecov-settings branch 4 times, most recently from abcf1d8 to fefe78e Compare June 6, 2023 23:00
@Vampire Vampire force-pushed the codecov-settings branch from fefe78e to e6fee6d Compare June 9, 2023 19:43
@Vampire Vampire force-pushed the codecov-settings branch from e6fee6d to 4918b4c Compare June 28, 2023 01:08
@Vampire Vampire force-pushed the codecov-settings branch 5 times, most recently from 7d65ff6 to b0ce2f9 Compare August 22, 2023 12:23
@Vampire Vampire force-pushed the codecov-settings branch 3 times, most recently from c3a6678 to b754360 Compare August 12, 2024 12:40
Fail the CI build if the Codecov uploading failed.
Show more detailed information in the Codecov comment.
Do not update the Codecov comment 15 times, but post it after all 16 reports were uploaded.
Ignore dates in coverage reports to not get "upload expired" because the report was taken from remote build cache.
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