Add macOS and Windows code coverage builds#163
Add macOS and Windows code coverage builds#163mvandeberg merged 1 commit intocppalliance:developfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe pull request restructures the code coverage CI/CD pipeline from a matrix-based single job into separate per-platform jobs for Linux, macOS, and Windows. Each platform job handles its own build, test, and coverage generation steps. The deployment workflow now aggregates coverage artifacts from all platforms, organizes them into per-OS directories, and creates navigation pages. The README badge table is expanded to display three separate coverage columns (one per platform) replacing the single coverage column. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
An automated preview of the documentation is available at https://163.corosio.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-02-23 20:20:44 UTC |
|
GCOVR code coverage report https://163.corosio.prtest3.cppalliance.org/gcovr/index.html Build time: 2026-02-23 19:38:27 UTC |
mvandeberg
force-pushed
the
pr/win-mac-coverage
branch
from
8d47f14 to
95287eb
Compare
![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
🧹 Nitpick comments (4)
.github/workflows/code-coverage.yml (2)
373-375: Deploy runs even when all builds fail.The
if: ${{ !cancelled() }}condition means deploy executes even if every build job fails (only skipping on workflow cancellation). While the copy step gracefully handles missing artifacts, you could tighten this to require at least one successful build to avoid pointless deploy commits.♻️ Suggested condition
- if: ${{ !cancelled() }} + if: ${{ !cancelled() && (needs.build-linux.result == 'success' || needs.build-macos.result == 'success' || needs.build-windows.result == 'success') }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/code-coverage.yml around lines 373 - 375, The deploy job currently only checks for cancellation (if: ${{ !cancelled() }}) so it runs even when all builds fail; update the deploy job's if condition to require at least one upstream build success and not cancelled by replacing the existing condition with a compound check against the three needs results (e.g., ensure needs.build-linux.result == 'success' || needs.build-macos.result == 'success' || needs.build-windows.result == 'success' combined with !cancelled()), referencing the deploy job and the needs.build-linux/needs.build-macos/needs.build-windows result symbols to locate where to change the condition.
415-433: Stale platform coverage data is preserved when a build fails.When a platform build fails, its artifact won't exist and the
if [ -d ... ]check skips the copy — but the oldgcovr-<platform>directory from a prior run is not removed either. This means the deployed page silently serves stale data for that platform. Ifcarryforward: truein Codecov is the intended model, this is fine, but consider adding a note or logging a warning so it's visible in the deploy output.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/code-coverage.yml around lines 415 - 433, The "Copy coverage results" step currently skips missing platform artifacts and leaves stale gh_pages_dir/${GITHUB_REF_NAME}/gcovr-<platform> directories; update the loop that checks coverage-artifacts/coverage-${platform} so that when the artifact directory is absent you explicitly remove the corresponding gh_pages_dir/${GITHUB_REF_NAME}/gcovr-${platform} (rm -rf) and/or emit a clear warning message (echo) referencing the platform and GITHUB_REF_NAME; ensure the logic around the for loop and the conditional uses the same platform variable names so stale data is removed and a visible log entry is produced when a platform build failed..github/workflows/ci.yml (1)
960-981: Coverage report generation for macOS and Windows looks correct.One subtle difference worth noting: the macOS step (line 964) uses
pip3 install --break-system-packages gcovrwhile the Windows step (line 976) usespip3 install gcovr. This is correct — macOS system Python (managed by Homebrew) requires the--break-system-packagesflag — but consider adding a brief inline comment on eachpip3 installline to explain the difference, to prevent future contributors from "fixing" the inconsistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 960 - 981, Add short inline comments next to the pip3 install lines in the "Generate Coverage Report (macOS)" and "Generate Coverage Report (Windows)" steps explaining why macOS uses --break-system-packages (to override Homebrew/system Python protections) and Windows does not; update the pip3 install in the macOS step (inside the step named "Generate Coverage Report (macOS)") to retain --break-system-packages and add a one-line comment, and add a matching one-line comment to the pip3 install in the step named "Generate Coverage Report (Windows)" clarifying the platform difference so future contributors don't remove the flag.README.md (1)
1-4: Consider using distinct alt text for coverage badges.All six coverage badge images use
Linesas alt text. When rendered without images (e.g., screen readers, slow connections), users see six identical "Lines" links with no way to distinguish them. Consider using platform-specific alt text likeLinux Coverage,macOS Coverage,Windows Coverage.♻️ Suggested diff (master row shown; apply similarly to develop)
-| [](https://cppalliance.org/corosio/master/gcovr-linux/index.html) | [](https://cppalliance.org/corosio/master/gcovr-macos/index.html) | [](https://cppalliance.org/corosio/master/gcovr-windows/index.html) | +| [](https://cppalliance.org/corosio/master/gcovr-linux/index.html) | [](https://cppalliance.org/corosio/master/gcovr-macos/index.html) | [](https://cppalliance.org/corosio/master/gcovr-windows/index.html) |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 1 - 4, The coverage badge images in the README table all share the same alt text "Lines", making them indistinguishable to screen readers; update the markdown image alt text for each coverage badge (the six occurrences of https://cppalliance.org/corosio/.../badges/coverage-lines.svg in both the `master` and `develop` rows) to unique, platform-specific labels such as "Linux Coverage", "macOS Coverage", and "Windows Coverage" so each badge link is descriptive and accessible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/code-coverage.yml:
- Around line 88-228: The macOS (build-macos) and Windows (build-windows)
coverage jobs omit TLS deps, so TLS tests get skipped; add the same
vcpkg/WolfSSL/OpenSSL setup used in ci.yml: create a vcpkg.json with wolfssl,
add a step to bootstrap and install vcpkg packages, ensure OpenSSL is installed
(Homebrew on macOS, MSYS2 on Windows MinGW), and pass CMake variables into the
Build with coverage step (extra-args) to set CMAKE_WOLFSSL_INCLUDE and
CMAKE_WOLFSSL_LIBRARY and OpenSSL include/library paths so CMake can find
WolfSSL/OpenSSL during the Build with coverage job and include TLS code paths in
coverage reports.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 960-981: Add short inline comments next to the pip3 install lines
in the "Generate Coverage Report (macOS)" and "Generate Coverage Report
(Windows)" steps explaining why macOS uses --break-system-packages (to override
Homebrew/system Python protections) and Windows does not; update the pip3
install in the macOS step (inside the step named "Generate Coverage Report
(macOS)") to retain --break-system-packages and add a one-line comment, and add
a matching one-line comment to the pip3 install in the step named "Generate
Coverage Report (Windows)" clarifying the platform difference so future
contributors don't remove the flag.
In @.github/workflows/code-coverage.yml:
- Around line 373-375: The deploy job currently only checks for cancellation
(if: ${{ !cancelled() }}) so it runs even when all builds fail; update the
deploy job's if condition to require at least one upstream build success and not
cancelled by replacing the existing condition with a compound check against the
three needs results (e.g., ensure needs.build-linux.result == 'success' ||
needs.build-macos.result == 'success' || needs.build-windows.result == 'success'
combined with !cancelled()), referencing the deploy job and the
needs.build-linux/needs.build-macos/needs.build-windows result symbols to locate
where to change the condition.
- Around line 415-433: The "Copy coverage results" step currently skips missing
platform artifacts and leaves stale
gh_pages_dir/${GITHUB_REF_NAME}/gcovr-<platform> directories; update the loop
that checks coverage-artifacts/coverage-${platform} so that when the artifact
directory is absent you explicitly remove the corresponding
gh_pages_dir/${GITHUB_REF_NAME}/gcovr-${platform} (rm -rf) and/or emit a clear
warning message (echo) referencing the platform and GITHUB_REF_NAME; ensure the
logic around the for loop and the conditional uses the same platform variable
names so stale data is removed and a visible log entry is produced when a
platform build failed.
In `@README.md`:
- Around line 1-4: The coverage badge images in the README table all share the
same alt text "Lines", making them indistinguishable to screen readers; update
the markdown image alt text for each coverage badge (the six occurrences of
https://cppalliance.org/corosio/.../badges/coverage-lines.svg in both the
`master` and `develop` rows) to unique, platform-specific labels such as "Linux
Coverage", "macOS Coverage", and "Windows Coverage" so each badge link is
descriptive and accessible.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.codecov.yml.github/workflows/ci.yml.github/workflows/code-coverage.ymlREADME.md
mvandeberg
force-pushed
the
pr/win-mac-coverage
branch
from
95287eb to
29f97eb
Compare
mvandeberg
force-pushed
the
pr/win-mac-coverage
branch
from
29f97eb to
8019d73
Compare
2 participants
Summary by CodeRabbit
Release Notes
Chores
Documentation