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

Move codecov push to its own job in Actions workflow #502

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

santisoler
Copy link
Member

Remove the push to codecov step from the test job into a new job that depends on the test job. Upload the coverage report as an artifact after testing, and reuse that artifact in the new job. Only the coverage from the ubuntu runner with the latest dependencies is uploaded to minimize hits to codecov.

Relevant issues/PRs:

Related to fatiando/community#151

Remove the push to codecov step from the `test` job into a new job that
depends on the test job. Upload the coverage report as an artifact after
testing, and reuse that artifact in the new job. Only the coverage from
the ubuntu runner with the latest dependencies is uploaded to minimize
hits to codecov.
Try to see if this downloads the artifact and decompress it in the root
of the git repo.
Upload coverage report obtained by ubuntu with optional dependencies.
@santisoler santisoler changed the title Add job for pushing coverage to codecov in Actions Move codecov push to its own job in Actions workflow May 8, 2024
@santisoler santisoler marked this pull request as ready for review May 8, 2024 23:50
@santisoler santisoler requested a review from leouieda May 8, 2024 23:50
Copy link
Member

@leouieda leouieda left a comment

Choose a reason for hiding this comment

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

Nice! For Harmonica this will work great but for other packages that have optional dependencies, the full coverage is only obtained when merging reports from multiple jobs. Then this won't work. Do you think it would be possible to upload artifacts for all of the jobs?

@santisoler
Copy link
Member Author

I think we can. We would just have to name each artifact differently and then download them all and pushed them all in a single job. Does it make sense?

@leouieda
Copy link
Member

👍🏾

Simplify dir structure of downloaded artifacts. Improve comments.
Since we are uploading all files at once, we don't need to keep track
from which runner each one of them was generated.
This way we avoid having artifacts names `coverage-ubuntu-latest-latest`
that could be confusing. This artifact would be now
`coverage_ubuntu-latest_latest`.
@santisoler
Copy link
Member Author

Done! Now all runners upload the coverage.xml file as a separate artifact, and the codecov-upload job downloads them all and pushes them all together to Codecov in a single run (lowering the hits to their servers).

Let me know if you like it. I'm looking forward to try this out on a PR opened from a fork.

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.

None yet

2 participants