ci: plumb CODECOV_TOKEN into the reusable test workflow - #3274
Open
vpetersson-bot wants to merge 2 commits into
Open
ci: plumb CODECOV_TOKEN into the reusable test workflow#3274vpetersson-bot wants to merge 2 commits into
vpetersson-bot wants to merge 2 commits into
Conversation
Coverage has never been uploaded from master. The master run for 82c5def logged: CODECOV_TOKEN: -> Token length: 0 error -- Upload queued for processing failed: {"message":"Token required because branch is protected"} ##[end-action outcome=success] so no baseline was ever stored and every PR reports "Please upload report for BASE". The step still passed because codecov-action defaults to fail_ci_if_error: false. Two independent causes, both needed: - `workflow_call` does NOT inherit secrets, so `secrets.CODECOV_TOKEN` inside test-runner.yml was empty however the repo was configured. Declared as an optional workflow_call secret and passed explicitly from the python job only — least privilege, and it documents the dependency at both ends rather than relying on `secrets: inherit`. - the repo has no CODECOV_TOKEN secret at all (only BALENA_TOKEN, DOCKER_PASSWORD, DOCKER_USERNAME). That has to be added separately; this change is inert until it is. Also stop the failure being silent: warn via a GitHub annotation when the token is empty on a push. fail_ci_if_error deliberately stays false so a Codecov outage cannot block a merge — but a misconfiguration should not be invisible for months either. Left optional so tokenless uploads from fork PRs keep working.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3274 +/- ##
=========================================
Coverage ? 90.71%
=========================================
Files ? 76
Lines ? 8440
Branches ? 896
=========================================
Hits ? 7656
Misses ? 563
Partials ? 221 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes Codecov baseline uploads from the protected master branch by explicitly plumbing CODECOV_TOKEN into the reusable CI test workflow, so Codecov can accept coverage uploads and compute PR deltas correctly.
Changes:
- Declares
CODECOV_TOKENas an optionalworkflow_callsecret in the reusabletest-runner.yml. - Passes
CODECOV_TOKENexplicitly from therun-python-testsjob indocker-test.yaml(instead of relying on implicit secret inheritance). - Adds a “missing token” warning step and provides the token to
codecov/codecov-actionvia both thetoken:input andCODECOV_TOKENenv var.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.github/workflows/test-runner.yml |
Accepts an optional Codecov token secret, adds a (currently mis-gated) warning step, and uses the token in the Codecov upload step. |
.github/workflows/docker-test.yaml |
Passes CODECOV_TOKEN into the reusable workflow for the Python test job. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- pass the caller's event name as an explicit workflow_call input rather than reading github.event_name inside the called workflow. I believe the context does carry the caller's event through, but the failure mode of being wrong is that the token warning silently never fires — which is precisely the bug class this change exists to stop, so it is not worth betting on. An explicit input cannot be wrong. - set fail_ci_if_error: false explicitly instead of relying on the action's default. The comment already claimed we depend on that behaviour; defaults can change across major versions, so pin it. Also keeps the bind-mount comment attached to the upload step it describes rather than the new warning step.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues Fixed
No associated issue. Surfaced by the Codecov comment on #3270: "Please upload report for BASE (
master@82c5def)".Description
Coverage has never been uploaded from master. The upload is rejected on every master push, and the step reports success anyway, so nothing has stored a baseline and every PR gets that warning.
From the master run for
82c5def(run 31085544289):Master is a protected branch, Codecov requires a token for protected refs, the token was empty, and
codecov-actiondefaults tofail_ci_if_error: false— so the step went green and the failure was invisible.Note what this is not: nothing is wrong with the coverage in #3270 or any other PR. Codecov explicitly reported "All modified and coverable lines are covered by tests" there. The only broken thing is the base-report upload, which makes the coverage delta uncomputable.
Two independent causes — both have to be fixed
workflow_calldoes not inherit secrets.docker-test.yamlcalledtest-runner.ymlwith onlywith:, nosecrets:block, sosecrets.CODECOV_TOKENinside the reusable workflow was empty however the repository was configured. Fixed here.CODECOV_TOKENsecret at all — the configured secrets areBALENA_TOKEN,DOCKER_PASSWORD,DOCKER_USERNAME. This PR is inert until that secret is added, and adding it is the half I cannot do.Fixing either alone achieves nothing, which is presumably how this survived so long.
The change
CODECOV_TOKENas an optionalworkflow_callsecret intest-runner.yml, and pass it explicitly from the python job only indocker-test.yaml. Explicit rather thansecrets: inheritso the test workflow receives just the one secret it needs, and so the dependency is visible at both ends. Optional so tokenless uploads from fork PRs keep working on unprotected refs.token:input, alongside the existing env export (the bundled CLI readsCODECOV_TOKENfrom the environment).push.fail_ci_if_errordeliberately staysfalse— a Codecov outage must never block a merge — but a misconfiguration should not be invisible for months either. This is the part that makes the class of bug self-reporting next time.Verification
Both files parse as valid YAML, and the secret lands on exactly the intended job:
The typescript job does not upload coverage (
if: inputs.test-type == 'python'), so it correctly receives nothing.What I could not verify: the upload actually succeeding, because that needs the secret to exist. Once it is added, the next master push should log a non-zero token length and no "Token required" error, and the following PR should show a real coverage delta instead of the missing-BASE warning.
Checklist
CI-configuration change only — no application code, so the test and device boxes do not apply. The change is validated by YAML parsing and by inspecting which job receives the secret; end-to-end validation is blocked on the repo secret.