Skip to content

Add Helm template coverage CI and Codecov upload - #698

Open
NiallTwomey2 wants to merge 9 commits into
mainfrom
RHIDP-18349-chart-coverage
Open

NiallTwomey2 wants to merge 9 commits into
mainfrom
RHIDP-18349-chart-coverage

Conversation

@NiallTwomey2

@NiallTwomey2 NiallTwomey2 commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Vendor helm-coverage.sh (Apache-2.0, pinned to 18c41243aca54231d36905185f2f65c231b06a84)
  • Run coverage on charts/rhdh, charts/must-gather, and charts/orchestrator-infra using existing ci/ values
  • Rewrite Cobertura paths to charts//templates/... and upload to Codecov
  • Keep Codecov informational so ~28% on rhdh does not fail PRs
  • ct lint/install is unchanged

Test plan

  • Chart Coverage job is green
  • Artifact XML filenames are charts//templates/...
  • Codecov shows those template files (needs CODECOV_TOKEN on this repo)
  • Lint Charts and Test Charts still pass
  • codecov/project and codecov/patch are informational

Issue: RHIDP-18349

Vendor helm-coverage.sh and run it on the three active charts so template branch coverage can be tracked in Codecov without changing ct install.

RHIDP-18349
@NiallTwomey2
NiallTwomey2 requested a review from a team as a code owner October 28, 2026 10:55
@openshift-ci
openshift-ci Bot requested review from rm3l and subhashkhileri October 28, 2026 10:55
Comment thread hack/helm-coverage.sh Fixed
Comment thread hack/helm-coverage.sh Fixed
Comment thread hack/helm-coverage.sh Fixed
Comment thread hack/helm-coverage.sh Fixed
Comment thread hack/helm-coverage.sh Fixed
Comment thread hack/helm-coverage.sh Fixed
Comment thread hack/helm-coverage.sh Fixed
Comment thread hack/helm-coverage.sh Fixed
Comment thread hack/helm-coverage.sh Fixed
Comment thread hack/helm-coverage.sh Fixed
Comment thread hack/helm-coverage.sh Fixed
@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

Track Helm template coverage in CI and Codecov

✨ Enhancement 🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes


AI Description

• Measure template coverage for three charts using their existing CI values files.
• Publish chart-scoped Cobertura reports as artifacts and upload them to Codecov.
• Keep Codecov coverage statuses informational without changing chart lint or install tests.
Diagram

graph TD
  CI["Coverage workflow"] --> Tool["Vendored instrumenter"] --> Helm["Helm rendering"] --> XML[("Cobertura reports")] --> Codecov["Codecov"]
  Values["Chart CI values"] --> Tool
  XML --> Artifacts["Workflow artifacts"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Fetch the pinned upstream script during CI
  • ➕ Avoids committing and reviewing a large vendored file.
  • ➖ Makes coverage runs depend on upstream availability.
  • ➖ Requires download and integrity verification in the workflow.

Recommendation: Keep the pinned vendored script for reproducible, self-contained CI runs rather than downloading it on every run. Reviewers should still validate the upstream provenance and a generated report before relying on the coverage figures.

Files changed (4) +1644 / -0

Other (4) +1644 / -0
chart-coverage.yamlRun and publish coverage for three Helm charts +69/-0

Run and publish coverage for three Helm charts

• Adds a workflow for pull requests to main or release branches and pushes to main. It renders rhdh, must-gather, and orchestrator-infra with their CI values, rewrites Cobertura template paths, archives the XML, and uploads it to Codecov.

.github/workflows/chart-coverage.yaml

.gitignoreIgnore generated coverage reports +1/-0

Ignore generated coverage reports

• Excludes the local coverage-out directory from version control.

.gitignore

codecov.ymlMake Codecov statuses informational +8/-0

Make Codecov statuses informational

• Configures project and patch coverage statuses as informational, so coverage levels do not fail those checks.

codecov.yml

helm-coverage.shVendor a pinned Helm template coverage instrumenter +1566/-0

Vendor a pinned Helm template coverage instrumenter

• Adds the Apache-2.0 upstream script with its source commit recorded. It instruments chart templates, aggregates coverage across values files, and generates Cobertura XML for CI.

hack/helm-coverage.sh

@codecov

codecov Bot commented Oct 28, 2026

Copy link
Copy Markdown

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 ☂️

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Code Review by Qodo


Action required

1. Release coverage is attributed to main 🐞 Bug
Description
The nightly Codecov step supplies neither a commit nor a branch override, even though its checkout
selects matrix.branch. On scheduled runs, release-branch jobs generate reports from release
commits while the workflow's GitHub SHA and ref identify the default branch, so those reports are
uploaded against the wrong revision.
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Nightly release-branch coverage is attributed to the scheduled workflow's default-branch revision instead of the chart revision tested.
## Fix Focus Areas
- .github/workflows/nightly.yaml[150-155]
- .github/workflows/nightly.yaml[205-212]
## Recommended Fix
Capture `git rev-parse HEAD` after checking out each matrix branch and pass it as Codecov's `override_commit`; pass `matrix.branch` as `override_branch`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗


View medium (2)
Remediation recommended
2. Project coverage omits other charts 🐞 Bug
Description
The coverage matrix uses discover-charts, which selects only charts changed by the pull request,
and each matrix job uploads only its own chart's XML. On a pull request that changes one chart,
Codecov receives no report for the other charts, so its project coverage total does not represent
the charts together.
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The coverage matrix runs only for changed charts, making Codecov's project total depend on which chart a pull request changes.
## Fix Focus Areas
- .github/workflows/test.yaml[92-100]
- .github/workflows/test.yaml[141-149]
## Recommended Fix
Run and upload coverage for every supported chart on each applicable pull request, independently of the changed-chart test matrix.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

3. Coverage can mark wrong pull request lines 🐞 Bug
Description
The coverage job checks out the default pull-request merge commit but override_commit attributes
its report to the pull request's head commit. If the base branch has changed template lines absent
from the head commit, the report's line numbers come from a different revision than the one Codecov
is asked to annotate.
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Coverage is generated from the pull-request merge commit but attributed to the head commit, which can have different template line numbers.
## Fix Focus Areas
- .github/workflows/test.yaml[102-105]
- .github/workflows/test.yaml[148-149]
## Recommended Fix
Check out `github.event.pull_request.head.sha` in the coverage job when attributing its report to that SHA, so the measured files and reported commit match.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗


@rhdh-qodo-merge

Copy link
Copy Markdown

Important

The /generate_labels command by Qodo was deprecated on the 1st of October 2026 and is no longer available. We recommend switching to the latest Qodo review capabilities. Learn more

@NiallTwomey2

Copy link
Copy Markdown
Member Author

/agentic_review

@NiallTwomey2 NiallTwomey2 self-assigned this Oct 28, 2026
Comment thread hack/helm-coverage.sh Resolved
Comment thread .github/workflows/chart-coverage.yaml Resolved
Comment thread .github/workflows/chart-coverage.yaml Resolved
Comment thread .github/workflows/chart-coverage.yaml Resolved
Comment thread codecov.yml
@@ -0,0 +1,8 @@
coverage:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Setting codecov.coverage.status.project and codecov.coverage.status.patch informational to true
to pass the GitHub checks

Chart Covered / total branches Rate
rhdh 71 / 258 27.5%
must-gather 47 / 77 61.0%
orchestrator-infra 16 / 19 84.2%
combined 134 / 354 37.9%

If informational is set to false, which is Codecov's default setting, codecov/project and codecov/patch are failing GitHub checks when project coverage is under target, or patch coverage on changed template lines are under Codecov's patch target, ~70%

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This comment is much more useful as context for reviewers ;)

Comment thread .gitignore

# Temporary patch file from sync-upstream-backstage.sh conflict resolution
rhdh-vendored.patch
coverage-out/

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ignores coverage-out/ so local XML files never get committed

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same point, obvious by reading the code. So no need to add a comment for this ;)

Comment thread .github/workflows/scripts-checks.yaml Resolved
Comment thread hack/helm-coverage.sh Resolved
Comment thread .github/workflows/chart-coverage.yaml Resolved
Comment thread .github/workflows/test.yaml Fixed
Comment thread .github/workflows/test.yaml Resolved
Comment on lines +118 to +119
curl -fsSL --proto '=https' --tlsv1.2 -o /tmp/helm-coverage.sh \
"https://raw.githubusercontent.com/jCOTINEAU/chart-coverage/${CHART_COVERAGE_SHA}/helm-coverage.sh"

@NiallTwomey2 NiallTwomey2 Nov 3, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fetch the coverage tool at a pinned SHA (HTTPS-only) - using the curl command

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

no need to add a comment like this, which is already obvious by reading the code ;)
This comment doesn't add much context to the PR and actually does bloat the review UI.

Comment thread .github/workflows/nightly.yaml Resolved
Comment thread .github/workflows/test.yaml Resolved
Comment thread .github/workflows/nightly.yaml Resolved
Comment thread .github/workflows/test.yaml Resolved
Comment thread .github/workflows/test.yaml Resolved
Comment thread .github/workflows/nightly.yaml Resolved
Comment thread .github/workflows/nightly.yaml Resolved
Comment thread .github/workflows/nightly.yaml Resolved
@NiallTwomey2
NiallTwomey2 requested a review from rm3l November 4, 2026 11:37
@sonarqubecloud

sonarqubecloud Bot commented Nov 6, 2026

Copy link
Copy Markdown

Comment on lines +193 to +194
-e "s|filename=\"templates/|filename=\"${chart_dir}/templates/|g" \
-e "s|<source>templates</source>|<source>${chart_dir}</source>|" \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
-e "s|filename=\"templates/|filename=\"${chart_dir}/templates/|g" \
-e "s|<source>templates</source>|<source>${chart_dir}</source>|" \
-e "s|<source>templates</source>|<source>${chart_dir}</source>|" \

Why is this transformation needed?
From a quick testing, filename already contains the right path starting from templates/. So it should be fine as is relative to ${chart_dir}, which should be the <source>.

Comment on lines +130 to +131
-e "s|filename=\"templates/|filename=\"${chart_dir}/templates/|g" \
-e "s|<source>templates</source>|<source>${chart_dir}</source>|" \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
-e "s|filename=\"templates/|filename=\"${chart_dir}/templates/|g" \
-e "s|<source>templates</source>|<source>${chart_dir}</source>|" \
-e "s|<source>templates</source>|<source>${chart_dir}</source>|" \

extra_helm_args: ${{ matrix.chart == 'rhdh' && format('--set image.repository={0} --set image.tag={1} --set image.pullPolicy=Always', steps.image.outputs.repo, steps.image.outputs.tag) || matrix.chart == 'backstage' && format('--set upstream.backstage.image.repository={0} --set upstream.backstage.image.tag={1} --set upstream.backstage.image.pullPolicy=Always', steps.image.outputs.repo, steps.image.outputs.tag) || '' }}
# helm_extra_args: general helm flags (e.g. --values), passed to ct install via --helm-extra-args
helm_extra_args: ${{ matrix.chart == 'rhdh' && '--values /tmp/rhdh-nightly-values.yaml' || matrix.chart == 'backstage' && '--values /tmp/backstage-nightly-values.yaml' || '' }}
coverage:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
coverage:
coverage:

uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
with:
ref: ${{ matrix.branch }}
fetch-depth: 0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
fetch-depth: 0

Full history not needed.

Comment on lines +104 to +105
with:
fetch-depth: 0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
with:
fetch-depth: 0

Comment on lines +118 to +119
curl -fsSL --proto '=https' --tlsv1.2 -o /tmp/helm-coverage.sh \
"https://raw.githubusercontent.com/jCOTINEAU/chart-coverage/${CHART_COVERAGE_SHA}/helm-coverage.sh"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

no need to add a comment like this, which is already obvious by reading the code ;)
This comment doesn't add much context to the PR and actually does bloat the review UI.

fi
echo "All chart tests passed (or no charts to test)."
No newline at end of file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change

Comment thread .gitignore

# Temporary patch file from sync-upstream-backstage.sh conflict resolution
rhdh-vendored.patch
coverage-out/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same point, obvious by reading the code. So no need to add a comment for this ;)

Comment thread codecov.yml
@@ -0,0 +1,8 @@
coverage:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This comment is much more useful as context for reviewers ;)

monitoring_heartbeat: ${{ vars.TEST_MONITORING_HEARTBEAT_ENABLED || 'false' }}
helm_template_values_file: ${{ matrix.chart == 'rhdh' && '/tmp/rhdh-full-plugins-values.yaml' || '' }}

coverage:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This coverage job looks quite similar in both workflows. I would suggest extracting this into a composite action under .github/actions/chart-coverage/action.yml that both workflows call.

This branch has not been deployed

No deployments
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.

3 participants