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: store merged coverage as artifact #1613

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

Conversation

saiaunghlyanhtet
Copy link

Integrate Code Coverage Reporting in CI Pipeline

Summary:

This pull request integrates code coverage reporting into the existing CI pipeline. The following updates have been made to the GitHub Actions workflow to ensure that code coverage is tracked and uploaded to the Coveralls service.

Changes:

  • Added Code Coverage Steps:

    • Introduced coverage generation after running tests in the CI pipeline.
    • Integrated the Coveralls reporting tool to upload the coverage data.
    • Modified the test job in the CI workflow to include coverage steps.
  • Test Coverage Reporting:

    • Coverage is generated for each test run and uploaded as an artifact.
    • This allows the team to track the test coverage on each pull request.

Impact:

  • Continuous monitoring of code coverage ensures that new code contributions maintain high-quality tests.
  • Developers can review coverage metrics and ensure new features are adequately tested.

Related Issues:

@saiaunghlyanhtet saiaunghlyanhtet marked this pull request as ready for review November 18, 2024 12:18
@saiaunghlyanhtet saiaunghlyanhtet requested a review from a team as a code owner November 18, 2024 12:18
Copy link
Collaborator

@ti-mo ti-mo left a comment

Choose a reason for hiding this comment

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

Hi, please undo all the style changes and limit your PR to only adding the extra build step.

@saiaunghlyanhtet
Copy link
Author

Hi, please undo all the style changes and limit your PR to only adding the extra build step.

Sorry, I think it's because of my editor. Made changes as requested.

@ti-mo
Copy link
Collaborator

ti-mo commented Nov 27, 2024

@saiaunghlyanhtet Looks like none of our CI jobs currently generate coverage reports, so you'll have to enable those first. run-tests.sh used to do that, but that's now gone.

@saiaunghlyanhtet
Copy link
Author

@saiaunghlyanhtet Looks like none of our CI jobs currently generate coverage reports, so you'll have to enable those first. run-tests.sh used to do that, but that's now gone.

Thank you. I will fix it.

@saiaunghlyanhtet
Copy link
Author

@ti-mo I have made changes. Can you review it again, please?

@github-actions github-actions bot added the breaking-change Changes exported API label Dec 9, 2024
@ti-mo
Copy link
Collaborator

ti-mo commented Dec 10, 2024

@saiaunghlyanhtet Why do coverage reports need to go into a separate directory? You're still not merging the output of the matrix jobs.

@ti-mo ti-mo marked this pull request as draft December 10, 2024 12:37
@ti-mo ti-mo removed the breaking-change Changes exported API label Dec 10, 2024
@github-actions github-actions bot added the breaking-change Changes exported API label Dec 10, 2024
@saiaunghlyanhtet
Copy link
Author

saiaunghlyanhtet commented Dec 11, 2024

@saiaunghlyanhtet Why do coverage reports need to go into a separate directory? You're still not merging the output of the matrix jobs.

@ti-mo
First, I thought I needed it, but now I am uploading them into action artifacts as default and downloading them before merging in merge step. This time I think all coverage reports will be included, including outputs for matrix jobs.


- name: Merge Coverage Reports
run: |
go tool covdata -merge coverage-reports/coverage-*.out -o coverage-merged.out
Copy link
Collaborator

Choose a reason for hiding this comment

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

go tool covdata doesn't work on the plaintext files generated with go test -coverprofile=. It only works with the new binary format, which is currently a bit annoying to produce with go test, see golang/go#66225 (comment). The cover directory needs to exist for this to work.

Documentation:
https://go.dev/doc/build-cover
https://go.dev/blog/integration-test-coverage

Please make sure your code (the commands executed, at least) work before pushing up your changes. It looks like you've been pushing commits blindly so far, which is not very respectful of a reviewer's time.

If you want to experiment with GHA workflows offline, take a look at https://github.com/nektos/act.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Changes exported API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants