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

ci: update codecov-action from v3 to v4 #185

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

rst0git
Copy link
Member

@rst0git rst0git commented Jul 19, 2024

No description provided.

@snprajwal
Copy link
Member

Are we sure this is a good idea? We definitely want to be tracking coverage, and doing this change would mean that if the token gets removed from the repo in the future, the pipeline will still erroneously succeed. Would it not be better to just let it fail if the token is not set?

@adrianreber
Copy link
Member

Silently skipping something might indeed result that we forget about this again. What was your motivation for this change @rst0git. Maybe that will help us to understand this PR better.

@rst0git
Copy link
Member Author

rst0git commented Jul 21, 2024

Would it not be better to just let it fail if the token is not set?

@snprajwal Adrian has configured the value for CODECOV_TOKEN in repository secrets. However, the GitHub action is unable to use it. This problem does not occur when running the CI workflow in my fork configured with codecov token for my personal repository.

##[debug]..Evaluating secrets:
##[debug]..=> Object
##[debug]..Evaluating String:
##[debug]..=> 'CODECOV_TOKEN'
##[debug]=> null

We can see the code coverage in the CI logs but uploading without a token is not supported:

# Print coverage from this run
go tool covdata percent -i=/home/runner/work/go-criu/go-criu/test/.coverage
	command-line-arguments		coverage: 68.4% of statements
...

What was your motivation for this change

This is a workaround until we confirm that the codecov configuration works or figure out how to fix the problem.

@snprajwal
Copy link
Member

@adrianreber do you want to double-check if the secret is set correctly please? It could be an accidental typo that's stopping the workflow from recognising it

@snprajwal
Copy link
Member

Dropping this here too - the coverage workflow on checkpointctl works fine: https://github.com/checkpoint-restore/checkpointctl/actions/runs/10009967467/job/27670052231

Copy link

codecov bot commented Jul 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.94%. Comparing base (ce0a56d) to head (1acd2d5).
Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #185      +/-   ##
==========================================
+ Coverage   48.13%   49.94%   +1.81%     
==========================================
  Files          21       21              
  Lines        2279     1898     -381     
==========================================
- Hits         1097      948     -149     
+ Misses       1050      814     -236     
- Partials      132      136       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rst0git rst0git changed the title ci: upload codecov report if token is set ci: moving back to tokenless Jul 22, 2024
@rst0git rst0git force-pushed the codecov branch 3 times, most recently from 6be7627 to 1acd2d5 Compare July 23, 2024 06:54
Version 4.4.1 provides a bugfix to correctly detect tokenless upload for
PRs from forks:

codecov/codecov-action#1437
codecov/codecov-action#1431

Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
@adrianreber adrianreber merged commit 2dd9980 into checkpoint-restore:master Jul 23, 2024
13 checks passed
@rst0git rst0git deleted the codecov branch July 23, 2024 07:15
@rst0git rst0git changed the title ci: moving back to tokenless ci: update codecov-action from v3 to v4 Jul 23, 2024
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.

3 participants