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

Fix codecov badge 'unknown' bug #172

Merged
merged 1 commit into from Mar 16, 2022

Conversation

EricGustin
Copy link
Contributor

This is a fix for the codecov badge in the README displaying as 'unknown' rather than displaying the repository's code coverage percentage. This occurs because the coverage report is never uploaded on the develop branch. Instead, the coverage report is ran on the compare branch of a PR. This is a problem because the codecov badge is linked to the develop branch. Since no coverage report is currently being uploaded from the develop branch, the badge is not able to display the repository's code coverage percentage.

The solution introduced in this PR is to run the coverage tests & upload to codecov on pushes to the develop branch, in addition to the already existing event trigger for all PRs.

@ben-albrecht
Copy link
Contributor

Can you clarify why we want it to run on both pull_request and push? I would think we only want to update the badge when a PR is merged, which would just be push.

@codecov-commenter
Copy link

codecov-commenter commented Mar 16, 2022

Codecov Report

❗ No coverage uploaded for pull request base (develop@187654d). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop     #172   +/-   ##
==========================================
  Coverage           ?   81.27%           
==========================================
  Files              ?       57           
  Lines              ?     2910           
  Branches           ?        0           
==========================================
  Hits               ?     2365           
  Misses             ?      545           
  Partials           ?        0           

@EricGustin
Copy link
Contributor Author

@ben-albrecht yeah this is a good point. The purpose of running on push is for the sake of the badge whereas the purpose of running on pull_request is to ensure all PRs are not introducing any bugs i.e. ensure all tests pass. I guess maybe the discussion we should have is whether running the CI on push to develop is worth it if the only purpose is to get the badge working.

@ben-albrecht
Copy link
Contributor

@EricGustin - would it make sense to split this into 2 separate jobs, where one is checking coverage before merge with trigger on: pull_request, and one is updating the badge post-merge with trigger on: push: branches: [develop, master] ?

Copy link
Contributor

@ben-albrecht ben-albrecht left a comment

Choose a reason for hiding this comment

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

LGTM

@EricGustin EricGustin merged commit d59cd2e into CrayLabs:develop Mar 16, 2022
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

3 participants