Split test-with-coverage.yml workflow in two pieces: ci-pr (untrusted…#1229
Conversation
…) and ci-coverage (trusted)
WalkthroughTest workflow runs on pull_request (master) and push (master), produces a coverage artifact; a new workflow (workflow_run) downloads that artifact and uploads coverage to Codecov using codecov-action@v5. Changes
Sequence DiagramsequenceDiagram
participant PR as Pull Request
participant GH as GitHub Actions
participant Artifact as Artifact Storage
participant Codecov as Codecov
PR->>GH: trigger test workflow (pull_request or push on master)
GH->>GH: checkout PR code, setup Go, run generate & test
GH->>Artifact: upload "coverage" artifact (./build/_output/coverage/coverage.txt)
GH->>GH: complete test-with-coverage run
GH->>GH: workflow_run triggers upload-coverage workflow
GH->>Artifact: download "coverage" artifact from triggering run
GH->>Codecov: upload coverage via codecov/codecov-action@v5 with CODECOV_TOKEN
Codecov-->>GH: return status (success/fail)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/ci-coverage.yml:
- Around line 14-18: The "Download coverage artifact" step uses
actions/download-artifact@v4 to fetch artifacts by run-id but is missing the
required github-token input; update that step (the step named "Download coverage
artifact" which calls actions/download-artifact@v4) to include a with entry for
github-token set to ${{ secrets.GITHUB_TOKEN }} so the action has permission to
download artifacts across workflow runs.
|
/retest suspicious of flakiness, i will investigate it |
| - name: Upload coverage artifact | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: coverage | ||
| path: ./build/_output/coverage/coverage.txt |
There was a problem hiding this comment.
I'm confused, why do we need this step when there is the other worklow?
There was a problem hiding this comment.
in order to pass the artifact to the second workflow which is the one that uploads the file to codecov
There was a problem hiding this comment.
ah, right 🤦♂️ Thanks
There was a problem hiding this comment.
but I guess that you need to update the other workflow to reflect the location - see the comment https://github.com/codeready-toolchain/host-operator/pull/1229/files#r2713481794
Change names of workflows
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/upload-coverage.yml:
- Around line 20-25: The Codecov upload step currently points to root
"coverage.txt" which won't match the downloaded artifact path; update the
"Upload to Codecov" step's files parameter (in the step using
codecov/codecov-action@v5) to "build/_output/coverage/coverage.txt" so the
action uploads the extracted artifact at build/_output/coverage/coverage.txt
instead of coverage.txt.
♻️ Duplicate comments (1)
.github/workflows/upload-coverage.yml (1)
14-18: Missinggithub-tokenfor cross-workflow artifact download remains unresolved.This issue was flagged in a previous review. The
actions/download-artifact@v4action requires explicitgithub-tokenwhen usingrun-idto download artifacts from a different workflow run. Without it, the download will fail due to insufficient permissions.Note: The response mentioned changing "version back to v6", but the code still shows v4, and the commit message references
upload-artifact(notdownload-artifact). The fix requires addinggithub-token, not changing versions.🔧 Proposed fix
- name: Download coverage artifact uses: actions/download-artifact@v4 with: name: coverage + github-token: ${{ secrets.GITHUB_TOKEN }} run-id: ${{ github.event.workflow_run.id }}
MatousJobanek
left a comment
MatousJobanek
left a comment
There was a problem hiding this comment.
Looks good, but please check the file path if it's correct or not
| uses: codecov/codecov-action@v5 | ||
| with: | ||
| token: ${{ secrets.CODECOV_TOKEN }} | ||
| files: build/output/coverage/coverage.txt |
There was a problem hiding this comment.
shouldn't this be?
| files: build/output/coverage/coverage.txt | |
| files: build/_output/coverage/coverage.txt |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jrosental, MatousJobanek The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/upload-coverage.yml:
- Around line 21-26: Add an explicit checkout step before the codecov upload:
run actions/checkout (e.g., actions/checkout@v4) and set the ref to the
triggering workflow's commit SHA (use github.event.workflow_run.head_commit.id
or github.event.workflow_run.head_commit.sha) with fetch-depth: 0 so the repo is
checked out at the PR head commit; place this checkout step before the existing
codecov/codecov-action@v5 step to ensure the uploaded coverage is associated
with the correct commit.
| - name: Upload to Codecov | ||
| uses: codecov/codecov-action@v5 | ||
| with: | ||
| token: ${{ secrets.CODECOV_TOKEN }} | ||
| files: build/_output/coverage/coverage.txt | ||
| fail_ci_if_error: true |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
codecov-action v5 workflow_run github actions context commit SHA
💡 Result:
When your upload job runs on on: workflow_run, github.sha / GITHUB_SHA is not the triggering commit—it’s the latest commit on the default branch for that event type. [1]
To get the commit SHA that the triggering workflow actually ran for, use the workflow_run payload:
- Triggering commit SHA:
${{ github.event.workflow_run.head_sha }}[2] - Triggering branch:
${{ github.event.workflow_run.head_branch }}[2]
For codecov/codecov-action@v5, make sure you check out that exact SHA (Codecov requires actions/checkout first). [3]
on:
workflow_run:
workflows: ["CI"]
types: [completed]
jobs:
codecov:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event.workflow_run.head_sha }}
- uses: codecov/codecov-action@v5
with:
token: ${{ secrets.CODECOV_TOKEN }}Sources: GitHub Actions workflow_run event behavior [1], workflow run fields (head_sha, head_branch) [2], Codecov Action v5 requirement to run actions/checkout before upload [3].
🏁 Script executed:
cat .github/workflows/upload-coverage.ymlRepository: codeready-toolchain/host-operator
Length of output: 763
Add actions/checkout with the correct ref before uploading to Codecov.
When using workflow_run triggers, github.sha refers to the default branch commit, not the PR head commit. codecov/codecov-action@v5 requires actions/checkout to run first with the triggering workflow's commit SHA to properly associate coverage reports:
Required fix
steps:
- name: Download coverage artifact
uses: actions/download-artifact@v4
with:
name: coverage
github-token: ${{ secrets.GITHUB_TOKEN }}
run-id: ${{ github.event.workflow_run.id }}
+ - name: Checkout triggering workflow commit
+ uses: actions/checkout@v4
+ with:
+ ref: ${{ github.event.workflow_run.head_sha }}
- name: Upload to Codecov
uses: codecov/codecov-action@v5
with:
token: ${{ secrets.CODECOV_TOKEN }}
files: build/_output/coverage/coverage.txt
fail_ci_if_error: true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Upload to Codecov | |
| uses: codecov/codecov-action@v5 | |
| with: | |
| token: ${{ secrets.CODECOV_TOKEN }} | |
| files: build/_output/coverage/coverage.txt | |
| fail_ci_if_error: true | |
| steps: | |
| - name: Download coverage artifact | |
| uses: actions/download-artifact@v4 | |
| with: | |
| name: coverage | |
| github-token: ${{ secrets.GITHUB_TOKEN }} | |
| run-id: ${{ github.event.workflow_run.id }} | |
| - name: Checkout triggering workflow commit | |
| uses: actions/checkout@v4 | |
| with: | |
| ref: ${{ github.event.workflow_run.head_sha }} | |
| - name: Upload to Codecov | |
| uses: codecov/codecov-action@v5 | |
| with: | |
| token: ${{ secrets.CODECOV_TOKEN }} | |
| files: build/_output/coverage/coverage.txt | |
| fail_ci_if_error: true |
🤖 Prompt for AI Agents
In @.github/workflows/upload-coverage.yml around lines 21 - 26, Add an explicit
checkout step before the codecov upload: run actions/checkout (e.g.,
actions/checkout@v4) and set the ref to the triggering workflow's commit SHA
(use github.event.workflow_run.head_commit.id or
github.event.workflow_run.head_commit.sha) with fetch-depth: 0 so the repo is
checked out at the PR head commit; place this checkout step before the existing
codecov/codecov-action@v5 step to ensure the uploaded coverage is associated
with the correct commit.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/test-with-coverage.yml:
- Around line 34-38: Remove the trailing whitespace on the "Upload coverage
artifact" step and make the uploaded artifact path match what the codecov upload
expects: update the artifact "path: ./build/_output/coverage/coverage.txt" (in
the step named "Upload coverage artifact") to "path: coverage/coverage.txt" so
the extracted file layout matches the downstream `upload-coverage.yml` usage;
alternatively, if you prefer keeping the original upload location, instead add a
`path: coverage` to the corresponding `actions/download-artifact@v4` download
step or change the codecov `files` entry to
"./build/_output/coverage/coverage.txt" — pick one consistent option and apply
it to the steps referenced above.
| - name: Upload coverage artifact | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: coverage | ||
| path: ./build/_output/coverage/coverage.txt |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the exact lines from the workflow file to check for trailing whitespace
cat -n .github/workflows/test-with-coverage.yml | sed -n '34,38p'Repository: codeready-toolchain/host-operator
Length of output: 288
🏁 Script executed:
# Search for coverage.txt references in Makefile
rg -n "coverage\.txt|_output/coverage" MakefileRepository: codeready-toolchain/host-operator
Length of output: 59
🏁 Script executed:
# Find the downstream workflow that consumes this artifact
fd "upload-coverage" .github/workflows/Repository: codeready-toolchain/host-operator
Length of output: 114
🏁 Script executed:
# Read the upload-coverage.yml workflow to see what artifact name it expects
cat -n .github/workflows/upload-coverage.ymlRepository: codeready-toolchain/host-operator
Length of output: 931
🏁 Script executed:
# Search more broadly for coverage generation - check various config files
rg -n "coverage" Makefile --max-count 20Repository: codeready-toolchain/host-operator
Length of output: 59
🏁 Script executed:
# Search for references to coverage in shell scripts or other config
find . -type f \( -name "*.sh" -o -name "*.yml" -o -name "*.yaml" \) -exec grep -l "coverage" {} \; 2>/dev/null | head -20Repository: codeready-toolchain/host-operator
Length of output: 175
🏁 Script executed:
# Check what the build/_output directory structure looks like or where it's created
fd "_output" . --type d 2>/dev/null | head -20Repository: codeready-toolchain/host-operator
Length of output: 59
🏁 Script executed:
# Search for test-related configuration and coverage generation
rg -n "test-with-coverage|coverage" --type yaml .github/workflows/ -A 2 -B 2Repository: codeready-toolchain/host-operator
Length of output: 2371
🏁 Script executed:
# Check if there's documentation about the coverage path
rg -n "coverage.txt" .Repository: codeready-toolchain/host-operator
Length of output: 380
🏁 Script executed:
# Look for go test or testing commands that generate coverage
rg -n "go test|cover|coverage" --type sh .Repository: codeready-toolchain/host-operator
Length of output: 59
🏁 Script executed:
# Check for scripts directory
fd "script" . --type d | head -10Repository: codeready-toolchain/host-operator
Length of output: 59
🏁 Script executed:
# Find COV_DIR definition in Makefiles
rg -n "COV_DIR\s*=" make/Repository: codeready-toolchain/host-operator
Length of output: 122
🏁 Script executed:
# Also check if there's a main Makefile
cat -n Makefile | head -50Repository: codeready-toolchain/host-operator
Length of output: 251
🏁 Script executed:
# Find OUT_DIR definition
rg -n "OUT_DIR\s*=" make/Repository: codeready-toolchain/host-operator
Length of output: 59
🏁 Script executed:
# Also check main Makefile for OUT_DIR
rg -n "OUT_DIR" Makefile | head -20Repository: codeready-toolchain/host-operator
Length of output: 59
🏁 Script executed:
# List all make files and search for OUT_DIR
ls -la make/*.mkRepository: codeready-toolchain/host-operator
Length of output: 840
🏁 Script executed:
# Search OUT_DIR in all make files
rg -n "OUT_DIR" make/Repository: codeready-toolchain/host-operator
Length of output: 503
🏁 Script executed:
# Check if OUT_DIR is from an environment variable or specific file
cat make/common.mk | head -100Repository: codeready-toolchain/host-operator
Length of output: 123
🏁 Script executed:
# Re-read upload-coverage.yml to check if there's a path parameter for download-artifact
cat -n .github/workflows/upload-coverage.yml | sed -n '14,20p'Repository: codeready-toolchain/host-operator
Length of output: 355
🏁 Script executed:
# Check GitHub Actions documentation references or any comments about artifact paths
rg -n "artifact|path" .github/workflows/upload-coverage.yml -B 1 -A 1Repository: codeready-toolchain/host-operator
Length of output: 197
🏁 Script executed:
# Check if there are any other references to artifact handling or codecov configuration
rg -n "codecov|artifact" . --type yaml --type toml --type json 2>/dev/null | head -30Repository: codeready-toolchain/host-operator
Length of output: 641
🏁 Script executed:
# Check .codecov.yaml to see if there's any path configuration there
cat -n .codecov.yamlRepository: codeready-toolchain/host-operator
Length of output: 1896
🌐 Web query:
actions/download-artifact@v4 default path behavior GitHub Actions
💡 Result:
In actions/download-artifact@v4, if you don’t set with: path, the action downloads/extracts artifacts into $GITHUB_WORKSPACE (the job’s workspace directory). [1]
If you download all artifacts (omit name), the action will (by default) create a subdirectory per artifact name under that destination path. [1][2]
If you want multiple matched artifacts extracted into the same destination directory, set merge-multiple: true (otherwise it stays false). [1]
Sources: [1] actions/download-artifact v4 README (inputs/examples) (github.com), [2] GitHub Docs on downloading all artifacts (per-artifact directories) (docs.github.com)
Citations:
- 1: https://github.com/jsoref/actions-download-artifact?utm_source=openai
- 2: https://docs.github.com/actions/using-workflows/storing-workflow-data-as-artifacts?utm_source=openai
Remove trailing whitespace from line 38 and fix the codecov file path mismatch.
Line 38 has trailing whitespace that should be removed.
More critically, the artifact path is misaligned with the downstream workflow. When actions/download-artifact@v4 downloads the artifact without specifying a path parameter, it preserves the original directory structure. The file will be extracted to ./build/_output/coverage/coverage.txt, but the codecov action in upload-coverage.yml (line 25) expects coverage/coverage.txt.
Either:
- Change the codecov
filesparameter to./build/_output/coverage/coverage.txt, or - Add
path: coverageto the download step to extract files into acoverage/directory, or - Change the upload path to just upload the file (e.g., use the directory
./build/_output/coverageif directory traversal is acceptable).
🤖 Prompt for AI Agents
In @.github/workflows/test-with-coverage.yml around lines 34 - 38, Remove the
trailing whitespace on the "Upload coverage artifact" step and make the uploaded
artifact path match what the codecov upload expects: update the artifact "path:
./build/_output/coverage/coverage.txt" (in the step named "Upload coverage
artifact") to "path: coverage/coverage.txt" so the extracted file layout matches
the downstream `upload-coverage.yml` usage; alternatively, if you prefer keeping
the original upload location, instead add a `path: coverage` to the
corresponding `actions/download-artifact@v4` download step or change the codecov
`files` entry to "./build/_output/coverage/coverage.txt" — pick one consistent
option and apply it to the steps referenced above.
| fail_ci_if_error: true # optional (default = false) | ||
| verbose: true # optional (default = false) | ||
| - name: Upload coverage artifact | ||
| uses: actions/upload-artifact@v4 |
There was a problem hiding this comment.
codeready-toolchain/registration-service@468f6e9
| uses: actions/upload-artifact@v4 | |
| uses: actions/upload-artifact@v6 |
|
|
||
| steps: | ||
| - name: Download coverage artifact | ||
| uses: actions/download-artifact@v4 |
There was a problem hiding this comment.
codeready-toolchain/registration-service@ddd8eab
| uses: actions/download-artifact@v4 | |
| uses: actions/download-artifact@v7 |
|
|
/retest |
|
govulncheck is failing because there is a newer go stable version with security fixes. it here is the pr: #1230 e2e tests are failing very likely due to flakiness. I will hit retest and investigate it |
|
/retest |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1229 +/- ##
=======================================
Coverage 70.15% 70.15%
=======================================
Files 84 84
Lines 6565 6565
=======================================
Hits 4606 4606
Misses 1747 1747
Partials 212 212 🚀 New features to boost your workflow:
|
3 participants
…) and ci-coverage (trusted)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.