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

RHOAIENG-11142: chore(test): Integrate Codecov coverage reporting for odh-notebook-controller #492

Conversation

andyatmiami
Copy link

@andyatmiami andyatmiami commented Dec 9, 2024

Description

This commit integrates Codecov into our testing process by publishing our coverage report generated during make test to Codecov via the official codecov/codecov-action GHA.

The CODECOV_TOKEN was previously uploaded to our GH repository as a secret (and copied to the Dependabot Secrets as well) by a repo admin:

Please be aware of functionality NOT included in this initial PR (that may be added in the future):

  • unit test results are not being shipped to Codecov (even though that capability is supported)
  • a .codecov.yml file is presently not included. this could come as a fast-follow after the team gets a "feel" for Codecov - but I think its a bit premature to take my personal opinions to create such a file. Example odh-dashboard config can be seen here:

The Codecov integration was added to the odh_notebook_controller_unit_test.yaml GHA - which is already configured to execute on pull_request and push triggers. As such, no additional modification was necessary.

⚠️ It should be noted that the make test target was modifed to ensure the coverage files for the 2 respective invocations of go test wrote to unique output files to prevent information loss. *.out files are already ignored via .gitignore, so no problems there. Also, Codecov claims to support report merging automatically - so this should cause no issues:

Related-to: https://issues.redhat.com/browse/RHOAIENG-11142

How Has This Been Tested?

Testing of the codecov/codecov-action GHA was performed against a repo in my personal organization:

Observations and outcomes were reported in Slack:

make test was ran locally to ensure renaming output files didn't introduce any regressions

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

…controller

This commit integrates Codecov into our testing process by publishing our coverage report generated during `make test` to Codecov via the official `codecov/codecov-action` GHA.

The `CODECOV_TOKEN` was previously uploaded to our GH repository as a secret (and copied to the Dependabot Secrets as well) by a repo admin:
- https://redhat-internal.slack.com/archives/C060A5FJEAD/p1733334721701649

Please be aware of functionality **NOT** included in this initial PR (that may be added in the future):
- unit test results are not being shipped to `Codecov` (even though that capability is supported)
- a `.codecov.yml` file is presently not included.  this could come as a fast-follow after the team gets a "feel" for `Codecov` - but I think its a bit premature to take my personal opinions to create such a file.  Example `odh-dashboard` config can be seen here:
    - https://github.com/opendatahub-io/odh-dashboard/blob/main/.codecov.yml

The `Codecov` integration was added to the `odh_notebook_controller_unit_test.yaml` GHA - which is already configured to execute on `pull_request` and `push` triggers.  As such, no additional modification was necessary.

:warning: It should be noted that the `make test` target was modifed to ensure the coverage files for the 2 respective invocations of `go test` wrote to unique output files to prevent information loss.  `*.out` files are already ignored via `.gitignore`, so no problems there.  Also, Codecov claims to support report merging automatically - so this should cause no issues:
- https://docs.codecov.com/docs/merging-reports

Related-to: https://issues.redhat.com/browse/RHOAIENG-11142
@jiridanek
Copy link
Member

/label tide/merge-method-squash

i like this one, because for these small PRs it will skip creating a merge commit and keeps git history a bit simpler this way

@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@caponetto
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Dec 9, 2024
@jiridanek jiridanek changed the title RHOAIENG-11142: chore(test): Integration Codecov coverage reporting for odh-notebook-controller RHOAIENG-11142: chore(test): Integrate Codecov coverage reporting for odh-notebook-controller Dec 9, 2024
@jiridanek
Copy link
Member

/lgtm

Copy link

openshift-ci bot commented Dec 10, 2024

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 0ed5904 into opendatahub-io:main Dec 10, 2024
12 checks passed
jiridanek pushed a commit to jiridanek/kubeflow that referenced this pull request Jan 23, 2025
…controller (opendatahub-io#492)

This commit integrates Codecov into our testing process by publishing our coverage report generated during `make test` to Codecov via the official `codecov/codecov-action` GHA.

The `CODECOV_TOKEN` was previously uploaded to our GH repository as a secret (and copied to the Dependabot Secrets as well) by a repo admin:
- https://redhat-internal.slack.com/archives/C060A5FJEAD/p1733334721701649

Please be aware of functionality **NOT** included in this initial PR (that may be added in the future):
- unit test results are not being shipped to `Codecov` (even though that capability is supported)
- a `.codecov.yml` file is presently not included.  this could come as a fast-follow after the team gets a "feel" for `Codecov` - but I think its a bit premature to take my personal opinions to create such a file.  Example `odh-dashboard` config can be seen here:
    - https://github.com/opendatahub-io/odh-dashboard/blob/main/.codecov.yml

The `Codecov` integration was added to the `odh_notebook_controller_unit_test.yaml` GHA - which is already configured to execute on `pull_request` and `push` triggers.  As such, no additional modification was necessary.

:warning: It should be noted that the `make test` target was modifed to ensure the coverage files for the 2 respective invocations of `go test` wrote to unique output files to prevent information loss.  `*.out` files are already ignored via `.gitignore`, so no problems there.  Also, Codecov claims to support report merging automatically - so this should cause no issues:
- https://docs.codecov.com/docs/merging-reports

Related-to: https://issues.redhat.com/browse/RHOAIENG-11142
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants