ci: surface code coverage on pull requests#771
Merged
Conversation
Three small infra changes; no production code touched. - .github/workflows/build_debug.yml: add `pull_request:` trigger so the existing code-coverage job runs on PRs targeting develop. Gate the Docker `build` and `package` jobs with `if: github.event_name != 'pull_request'` so they remain push-only (we don't want PR runs pushing debug images to Docker Hub). - .codecov.yml: new file. Configure both project and patch status as informational so the Codecov bot posts a PR comment with the diff but never blocks merge. Once project coverage hits 85% we flip both to `informational: false`. Ignores 3rd-party, build artefacts, generated protobuf, and test sources themselves. - CMakeLists.txt: drop `cmd/*` from the lcov EXCLUDE list so the pktvisord/pktvisor-reader entry-point logic shows up in the report (per discussion). `3rd/*`, `libs/*`, and the build dir stay excluded. The first PR run will give us the baseline coverage number, which informs the Phase 2 push toward 85%. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Mirror the comment-on-PR pattern used in orb-discovery's device-discovery-lint-tests workflow. Codecov continues to receive uploads (long-term dashboard) but romeovs/lcov-reporter-action now also posts a self-contained markdown summary directly into the PR, showing per-file coverage and diff for changed files. Pinned to v0.4.0 (commit 87a815f) per repo convention. Adds pull-requests:write to the code-coverage job's permissions so the action can post/update the comment with the default GITHUB_TOKEN. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Belt-and-suspenders: pull-requests:write should cover PR comment posting per GitHub's docs, but some comment-posting actions hit the issues.createComment endpoint directly (since PR conversation comments share the issue API path) and only check the issues:write scope. Granting both avoids a 403 surprise. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Coverage after merging chore/coverage-on-prs into develop will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
The previous run leaked /usr/include/c++/13/* and similar system paths into the coverage report, making the comment dominated by STL noise. Add /usr/*, /Library/*, */c++/*, and */.conan2/* to the lcov EXCLUDE list so the report only covers our own source tree. Also strip explanatory comments from the workflow file.
The previous run hit lcov v2's strict-by-default behaviour on unused EXCLUDE patterns: '/Library/*' and '*/.conan2/*' don't match any file on the Ubuntu runner, so lcov bailed with a non-zero exit at the filter step. - Drop the two non-Linux patterns from setup_target_for_coverage_lcov. - Pass `LCOV_ARGS --ignore-errors unused` so any future pattern drift becomes a warning rather than a hard failure. - Picks up the user's CodeCoverage.cmake refresh (Lars Bilke's Jan 2026 version) which adds -fprofile-update=atomic for concurrent ctest runs and proper lcov-version detection — drop-in compatible with our existing call site.
|
Coverage after merging chore/coverage-on-prs into develop will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
romeovs/lcov-reporter-action emits an HTML table row per source file. With ~99 .gcda files in pktvisor (every handler/input compilation unit plus their tests), the rendered comment hit ~65402 bytes — right at GitHub's 65536-char limit. Files alphabetically past src/Metrics.h (which is exactly where src/handlers/ and src/inputs/ start) got silently truncated. Replace the action with a two-step approach: 1. `lcov --summary` + `lcov --list` produce a compact text table (~80 chars/file flat) that holds all 99 files comfortably. 2. marocchino/sticky-pull-request-comment posts/updates a single comment per PR keyed on the `code-coverage` header. Codecov upload remains for the dashboard.
Better fit than the lcov-list+sticky-comment combo: - Single step instead of two (render + post) - Uploads a browsable HTML report as a PR artifact, so the full per-directory drill-down is one click away even if the inline comment truncates anything - update-comment: true keeps PR clean across re-runs
Code CoveragePer-file breakdown |
LCOV of commit
|
mfiedorowicz
approved these changes
May 9, 2026
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70a573fff2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+77
to
+83
| if: github.event_name == 'pull_request' | ||
| uses: zgosalvez/github-actions-report-lcov@7d72c57ce4bc101a4a0bf9d726b6c435abde8439 #v7.0.10 | ||
| with: | ||
| coverage-files: build/coverage.info | ||
| artifact-name: code-coverage-report | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| update-comment: true |
There was a problem hiding this comment.
This step unconditionally runs on every pull_request, but for forked PRs (and Dependabot PRs) GitHub downgrades GITHUB_TOKEN to read-only, so github-actions-report-lcov cannot update the PR comment and will fail with a 403. That turns the new coverage workflow into a red check for external contributions even when tests and coverage generation succeed; gate this step to non-fork PRs (or make it non-blocking) to avoid breaking contributor PRs.
Useful? React with 👍 / 👎.
3 tasks
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
2 participants
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.
Phase 1 of the coverage initiative — make coverage visible in PRs without enforcing anything yet. Pure infra; no production code touched.
Changes
`.github/workflows/build_debug.yml`: add `pull_request:` trigger to the workflow so the existing `code-coverage` job runs on PRs targeting `develop`. The `build` and `package` Docker-pushing jobs stay push-only via `if: github.event_name != 'pull_request'` — we don't want PRs pushing debug images to Docker Hub.
`.codecov.yml` (new): both project and patch status are `informational: true`, so the Codecov bot posts a comment in the PR with diff/flags/files but never blocks merge. Phase 3 of the rollout flips both flags to `false` once project coverage reaches 85%. `ignore` covers vendored 3rd-party code, generated protobuf, build artefacts, and test sources themselves.
`CMakeLists.txt`: drop `cmd/` from the `setup_target_for_coverage_lcov` EXCLUDE list. The pktvisord/pktvisor-reader entry points were excluded but contain real logic worth tracking. `3rd/`, `libs/*`, and the build directory stay excluded.
What this gives us
Test plan