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

Add codecov upload for forks and main #69

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

andywaltlova
Copy link

Description

Type of change

  • CI configuration change

Related Tickets & Documents

Checklist before requesting a review

  • I have performed a self-review of my code.
    • Someone with access needs to get codecov token and update settings
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • We need to see comments from codecov in the PRs and see the reports on the website

@andywaltlova andywaltlova changed the title #54 Add codecov upload for forks and main Add codecov upload for forks and main Oct 22, 2024
@andywaltlova
Copy link
Author

Also the #54 doesn't specify if we need some treshold or specific config, so I left the one that was already in the repository

Copy link
Collaborator

@tisnik tisnik left a comment

Choose a reason for hiding this comment

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

looks ok, let's check it

@andywaltlova andywaltlova force-pushed the feat/add-codecov-upload branch from 7c24420 to 8cd73f4 Compare October 22, 2024 08:34
@andywaltlova
Copy link
Author

@tisnik let's try using github.repository instead of github.ref , forks can have main branch too .. I don't have preference, but using repository variable seems little bit better

@andywaltlova
Copy link
Author

Hmm the condition for repository doesn't work, I would expect the without token action will be taken 🤔

@andywaltlova
Copy link
Author

@r0x0d could you tell me more about problems in convert2rhel regarding this? Because it looks to me that the PR workflow is running in the main repo context, so adding the token should work even for PRs from forks

@r0x0d
Copy link
Member

r0x0d commented Oct 22, 2024

@r0x0d could you tell me more about problems in convert2rhel regarding this?
It happened with forks in our case, as forks are not allowed to access secrets within GitHub pipeline. If we want to expose this, we would to use one of the permissive events and make it insecure.

What we noticed was that from time to time, we had to retrigger the coverage workflow because it failed to push the results to codecov.

Because it looks to me that the PR workflow is running in the main repo context, so adding the token should work even for PRs from forks

Not sure if runs strictly on main context, as it should work for each PR instead: https://github.com/oamg/convert2rhel/blob/main/.github/workflows/coverage.yml#L45-L75

As seen in the workflow above, we tried to implement a "retry" in the workflow, that helped a bit.

@andywaltlova
Copy link
Author

I did change the conditions little bit with latest commit, will see what happens. Anyway @tisnik I will need someone with acces to settings to set up the codecov token

@tisnik
Copy link
Collaborator

tisnik commented Oct 22, 2024

I did change the conditions little bit with latest commit, will see what happens. Anyway @tisnik I will need someone with acces to settings to set up the codecov token

ok, what should I do? Generate the token on codecov.io first?

@r0x0d
Copy link
Member

r0x0d commented Oct 22, 2024

I did change the conditions little bit with latest commit, will see what happens. Anyway @tisnik I will need someone with acces to settings to set up the codecov token

ok, what should I do? Generate the token on codecov.io first?

Yes, @tisnik. In codecov you can look for the settings of road-core/service and there will be a token... After that, just create a secret CODECOV_TOKEN and use the value from codecov

@r0x0d
Copy link
Member

r0x0d commented Oct 22, 2024

@tisnik this should lead you the way: https://app.codecov.io/gh/road-core/service/config/general if you have configured codecov for this org already 🤔

@tisnik
Copy link
Collaborator

tisnik commented Oct 29, 2024

@andywaltlova registered our repo on codecov.io. Now it waits for maintainer of our group to ack codecov.io as an app for GH.

With codecov v4+ forks are detected automatically and tokenless upload
is used

Signed-off-by: Andrea Waltlova <[email protected]>

Signed-off-by: Andrea Waltlova <[email protected]>
@andywaltlova andywaltlova force-pushed the feat/add-codecov-upload branch from 124d68f to f50450c Compare October 31, 2024 12:33
@andywaltlova
Copy link
Author

Great I will wait for that, I just simplified the workflow because I verified i ndifferent repo that v4+ codecov action is able to detect forks on its own (https://github.com/codecov/codecov-action?tab=readme-ov-file#v4-release)

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.

[RFE] unit test code coverage visible on Codecov.io
3 participants