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 #140

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

rst0git
Copy link
Member

@rst0git rst0git commented Jul 19, 2024

No description provided.

Copy link

github-actions bot commented Jul 19, 2024

Test Results

55 tests  ±0   55 ✅ ±0   1s ⏱️ ±0s
 1 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 62d2ebe. ± Comparison against base commit b1f1c3b.

♻️ This comment has been updated with latest results.

@rst0git rst0git force-pushed the coverage branch 2 times, most recently from 0907b75 to e472bac Compare July 19, 2024 17:05
@snprajwal
Copy link
Member

@rst0git the coverage workflow on the main branch succeeds as of now without any problems: https://github.com/checkpoint-restore/checkpointctl/actions/runs/10009967467/job/27670052231

@rst0git
Copy link
Member Author

rst0git commented Jul 22, 2024

the coverage workflow on the main branch succeeds as of now without any problems: https://github.com/checkpoint-restore/checkpointctl/actions/runs/10009967467/job/27670052231

@snprajwal The workflow is unable to access the upload token in pull requests (e.g., #141, #140)

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

@snprajwal
Copy link
Member

snprajwal commented Jul 22, 2024

Right, so it seems GitHub does not allow accessing secrets implicitly in workflows triggered on commits outside the upstream repo: https://docs.github.com/en/actions/security-guides/using-secrets-in-github-actions#using-secrets-in-a-workflow

Adding a with item with the secret should fix this for us? Scratch that, looks like GitHub just refuses to pass secrets to commits on a fork

@snprajwal
Copy link
Member

Going by the docs, the pull_request trigger does not allow access to secrets, but the pull_request_target trigger does. Can we try this?

@codecov-commenter
Copy link

codecov-commenter commented Jul 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.88%. Comparing base (ee4e4e3) to head (62d2ebe).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #140      +/-   ##
==========================================
+ Coverage   78.73%   80.88%   +2.15%     
==========================================
  Files          11       11              
  Lines        1260      994     -266     
==========================================
- Hits          992      804     -188     
+ Misses        201      123      -78     
  Partials       67       67              

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

@rst0git
Copy link
Member Author

rst0git commented Jul 22, 2024

@snprajwal It looks like tokenless uploading works with PRs:

@rst0git rst0git changed the title ci/coverage: upload codecov report if token is set ci/coverage: moving back to tokenless Jul 22, 2024
@snprajwal
Copy link
Member

@rst0git by the way, reading the issue you linked, it looks like the token is required for PRs coming from the base repo + commits on the base repo itself, and forks can use the tokenless version. If that is indeed the case, we might require another check to decide if the token should be used or not (based on whether the trigger is push or pull_request)

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>
@rst0git
Copy link
Member Author

rst0git commented Jul 23, 2024

@snprajwal This problem seems to be fixed with codecov/codecov-action#1437.

@rst0git rst0git merged commit 723e647 into checkpoint-restore:main Jul 23, 2024
10 checks passed
@rst0git rst0git deleted the coverage branch July 23, 2024 07:05
@rst0git rst0git changed the title ci/coverage: 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