FLY-2591: add Codecov coverage reporting - #207
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
There was a problem hiding this comment.
Pull request overview
Adds Codecov coverage reporting configuration and CI integration so this SDK can publish and track coverage deltas in PRs (and display coverage status in the README), aligning with the setup used in liquidity-provider-server.
Changes:
- Add
codecov.ymlwith project/patch coverage thresholds and ignore patterns. - Update CI to run
npm run test:coverageand uploadcoverage/lcov.infoviacodecov/codecov-action(SHA-pinned). - Add a Codecov badge to
README.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| README.md | Adds Codecov badge alongside existing CI/CodeQL badges. |
| codecov.yml | Defines Codecov status checks, PR comment behavior, and ignored paths. |
| .github/workflows/ci.yml | Switches CI to run coverage and attempts to upload to Codecov. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The upload step ran unconditionally with fail_ci_if_error, so the whole CI job failed whenever CODECOV_TOKEN was absent. That is the case today, and it is permanently the case for pull requests from forks, where Actions secrets are not exposed to the workflow. Gate the upload on a preceding step that checks the token. When it is missing the run emits a warning annotation and a job-summary line, so a skipped upload stays visible instead of passing silently. Keep fail_ci_if_error, so a real upload failure still fails the job once the token exists.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
.github/workflows/ci.yml:45
- The PR description's “Known blocker” says the Codecov upload step will fail CI until onboarding/secret provisioning, but the workflow now explicitly skips the upload when
CODECOV_TOKENis missing (and only runs the action when configured). Please update the PR description accordingly so reviewers/maintainers aren't misled about merge readiness.
- name: Check for Codecov token
id: codecov_token
env:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
run: |
if [ -n "$CODECOV_TOKEN" ]; then
echo "configured=true" >> "$GITHUB_OUTPUT"
else
echo "configured=false" >> "$GITHUB_OUTPUT"
echo "::warning title=Coverage not uploaded::CODECOV_TOKEN is not available, so the Codecov upload was skipped. Expected on pull requests from forks, where Actions secrets are not exposed; otherwise the repository still needs to be onboarded to Codecov and the secret added."
echo ":warning: **Coverage not uploaded** — \`CODECOV_TOKEN\` is not available, so the Codecov upload was skipped." >> "$GITHUB_STEP_SUMMARY"
fi
- name: Upload coverage to Codecov
if: steps.codecov_token.outputs.configured == 'true'
uses: codecov/codecov-action@0fb7174895f61a3b6b78fc075e0cd60383518dac # v5.5.5
.github/workflows/ci.yml:30
npm run test:coveragerunsjest --coverage --collectCoverageFrom="./src/**"(package.json) and this repo contains manysrc/**/*.test.tsfiles, so CI will include test files in the generated lcov report. Since Codecov is configured to ignore**/*.test.ts, the coverage numbers shown by Jest vs. Codecov may diverge, and instrumentation work is wasted on test files. Consider excluding**/*.test.ts(and any other test patterns) in Jest collection (e.g., via jest config or thetest:coveragescript) so the report and Codecov match.
- name: Run unit tests with coverage
run: npm run test:coverage
|
@Luisfc68 sorry, your approval was auto-dismissed — the The only change since you approved is 0494da2, which gates the Codecov upload on whether Why it was needed: the upload ran unconditionally with CI is green now, and the gate is verified rather than assumed. On the latest run the Both review threads are resolved, and the PR description now carries step-by-step QA instructions, including a check that a deliberately failing test still fails CI. |
What
codecov.ymlat the repo root with the same thresholdsliquidity-provider-serveruses: project targetautowith a 1% allowed drop, patch target 80% with a 5% allowed shortfall.npm run test:coverageinstead ofnpm run test, then uploadscoverage/lcov.infowithcodecov/codecov-action(SHA-pinned to v5.5.5, matching how this repo pins every action).CODECOV_TOKENis set. Without a token the upload is skipped and the run emits a warning annotation plus a job-summary line, instead of failing the whole job.No change to jest config was needed.
jest.config.jsalready listslcovincoverageReportersand writes tocoverage/, andcoverage/is already in.gitignore.Why
Integrators consume this SDK directly, so an untested regression reaches third parties instead of stopping inside the squad.
npm run test:coveragealready existed but nothing read its output. With this change each PR gets a coverage delta comment and the README shows the current number, the same setup LPS has today.Why the upload is gated
A step-level
if:cannot read thesecretscontext, so the token is surfaced throughenvon aCheck for Codecov tokenstep that writesconfigured=true|falseto$GITHUB_OUTPUT; the upload reads that output.Two cases need it. The repository is not yet onboarded to Codecov and has no
CODECOV_TOKENsecret, so without the gatefail_ci_if_error: truefails every CI run. And pull requests from forks never receive Actions secrets, so that case does not go away once onboarding lands.fail_ci_if_error: trueis kept deliberately. The gate only skips the step when there is no token at all. Once the secret exists, a genuine upload failure still fails the job.A skipped upload is not silent: it shows as a
Coverage not uploadedwarning on the checks tab and in the run summary.How to verify
npm ci && npm run test:coveragewritescoverage/lcov.info. Verified locally: 39 suites, 357 tests pass, and the file lands at that path.js-yaml).ignore:list incodecov.ymlcovers the generated and non-source paths of this repo:src/api/bindings/**,src/blockchain/bindings/**,src/index.ts,**/*.test.ts,integration-test/**,lib/**,scripts/**. It mirrors the existingcoveragePathIgnorePatternsinjest.config.js.QA
Four checks. The first two are the regression this PR fixes; the last two prove the gate did not simply make CI unconditionally green.
1. Confirm the old behaviour failed
Open the CI run for commit
290cdec(the previous head of this branch).Code integrity validationfails, and the failing step isUpload coverage to Codecov, not a test or lint step.2. Confirm the new behaviour passes, with the skip visible
Open the CI run for the current head. Expect all of:
Code integrity validationpasses.Upload coverage to Codecovis marked skipped, not green. A green upload would mean a token appeared and the gate was never exercised.Coverage not uploaded.3. Exercise the gate logic locally, both branches
Save the gate script exactly as CI runs it:
Run it with no token:
Run it with a token:
configured=trueis what makes the upload step run, so this is the path that will be taken once the real secret is added.4. Confirm CI still fails when it should
The gate must not mask real failures. On a scratch branch, break something and push:
Open a draft PR from that branch.
Code integrity validationmust fail atRun unit tests with coverage, before the gate is ever reached. Delete the branch afterwards.Follow-up, not a blocker
Codecov onboarding for this repository and the
CODECOV_TOKENsecret are tracked separately. Until that lands, coverage is computed on every run but not published, and the README badge will readunknown. Adding the secret is the only step needed to turn uploads on; no further change to this workflow is required.Out of scope, per the ticket: raising the coverage number, and making the Codecov check required for merge.
Related issues
CODECOV_TOKENsecret)