feat: add test coverage reporting with GitHub Pages deployment #188
Conversation
Add coverage script to package.json and update CI workflow to: - Run CI on all pushes (not just main branch) - Execute tests with coverage on main branch only - Upload coverage reports to GitHub Pages for main branch - Run regular tests without coverage on other branches/PRs This addresses the need for visibility into test coverage metrics while keeping PR builds fast by only generating coverage on main. Closes #184
Update vitest and @vitest/coverage-v8 from v3.2.4 to v4.0.15. Vitest v4 introduces several improvements including: - Stable browser mode - Visual regression testing support - Rewritten pool architecture without Tinypool - Workspace renamed to projects configuration This upgrade prepares the codebase for using v4's projects feature for monorepo test aggregation. Closes #186
Refactor vitest configuration to use v4's projects feature for unified monorepo test execution and coverage reporting. Changes: - Add projects configuration with 'root' and 'examples' workspaces - Simplify package.json test scripts (remove test:submodules, test:root) - Extend coverage.include to capture examples/**/*.ts files - Enable automatic test discovery in examples when tests are added This allows running all tests across the monorepo with a single 'pnpm test' command and generates unified coverage reports that include both src and examples directories.
Add automatic coverage badge SVG generation using jaywcjlove/coverage-badges-cli action. Changes: - Add json-summary reporter to vitest coverage config - Generate badges.svg from coverage-summary.json in CI - Badge will be available at GitHub Pages URL/badges.svg This enables adding a coverage badge to README that automatically updates when coverage changes.
Update coverage-badges-cli action reference to v2.2.0 with correct SHA.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 5 files
Prompt for AI agents (all 2 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name=".github/workflows/ci.yaml">
<violation number="1" location=".github/workflows/ci.yaml:4">
P2: This `push:` trigger without branch filtering will cause duplicate CI runs for PRs to main. Consider adding a branch filter (e.g., `branches: [main]`) or using `pull_request` only for feature branches.
Alternatively, if you want CI to run on all pushes, you could skip the push-triggered run when a PR exists using a condition.</violation>
<violation number="2" location=".github/workflows/ci.yaml:13">
P2: Permissions `pages: write` and `id-token: write` are set at workflow level but only needed by `deploy-coverage` job. Consider moving these to job-level permissions for better security (principle of least privilege).</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| group: ${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| permissions: |
There was a problem hiding this comment.
P2: Permissions pages: write and id-token: write are set at workflow level but only needed by deploy-coverage job. Consider moving these to job-level permissions for better security (principle of least privilege).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yaml, line 13:
<comment>Permissions `pages: write` and `id-token: write` are set at workflow level but only needed by `deploy-coverage` job. Consider moving these to job-level permissions for better security (principle of least privilege).</comment>
<file context>
@@ -9,6 +10,11 @@ concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true
+permissions:
+ contents: read
+ pages: write
</file context>
| name: CI | ||
|
|
||
| on: | ||
| push: |
There was a problem hiding this comment.
P2: This push: trigger without branch filtering will cause duplicate CI runs for PRs to main. Consider adding a branch filter (e.g., branches: [main]) or using pull_request only for feature branches.
Alternatively, if you want CI to run on all pushes, you could skip the push-triggered run when a PR exists using a condition.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yaml, line 4:
<comment>This `push:` trigger without branch filtering will cause duplicate CI runs for PRs to main. Consider adding a branch filter (e.g., `branches: [main]`) or using `pull_request` only for feature branches.
Alternatively, if you want CI to run on all pushes, you could skip the push-triggered run when a PR exists using a condition.</comment>
<file context>
@@ -1,6 +1,7 @@
name: CI
on:
+ push:
pull_request:
branches:
</file context>
| push: | |
| push: | |
| branches: | |
| - main |
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive test coverage reporting to the CI workflow and upgrades Vitest from v3.2.4 to v4.0.15, leveraging Vitest's new projects feature for monorepo support. The changes enable automated coverage report deployment to GitHub Pages for the main branch while maintaining standard test execution for pull requests.
Key Changes
- Vitest v4 upgrade: Migrated from v3.2.4 to v4.0.15 with project-based configuration for monorepo test aggregation
- CI workflow enhancement: Added conditional test execution (standard tests for PRs, coverage generation for main branch) with GitHub Pages deployment
- Simplified test scripts: Consolidated monorepo test execution into a single
pnpm testcommand using Vitest projects
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Restructured configuration to use projects for root and examples workspaces, updated coverage includes/excludes, moved typecheck config to root project |
| pnpm-workspace.yaml | Upgraded @vitest/coverage-v8 and vitest from ^3.2.4 to ^4.0.15 in dev catalog |
| pnpm-lock.yaml | Updated dependency resolution for Vitest v4 ecosystem and related packages |
| package.json | Simplified test scripts by removing separate submodule and root commands, added coverage script |
| .github/workflows/ci.yaml | Added push trigger, GitHub Pages permissions, conditional coverage generation on main branch, and Pages deployment job |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| name: CI | ||
|
|
||
| on: | ||
| push: |
There was a problem hiding this comment.
The workflow now triggers on all pushes to any branch without restrictions. This means the CI will run on pushes to feature branches, tags, and any other refs, which may be more frequent than intended and could consume unnecessary CI resources.
Consider adding branch restrictions to the push trigger to match the existing pull_request configuration:
on:
push:
branches:
- main
pull_request:
branches:
- mainThis ensures CI runs on:
- Pushes to the main branch (for coverage deployment)
- Pull requests targeting main (for PR validation)
| push: | |
| push: | |
| branches: | |
| - main |
| permissions: | ||
| contents: read | ||
| pages: write | ||
| id-token: write |
There was a problem hiding this comment.
The pages: write and id-token: write permissions are defined at the workflow level (lines 15-16), which grants these permissions to all jobs including the ci job. However, these permissions are only needed for the deploy-coverage job.
According to GitHub Actions security best practices, permissions should be scoped as narrowly as possible. Consider moving these permissions to the job level:
permissions:
contents: read
jobs:
ci:
runs-on: ubuntu-latest
# ... steps ...
deploy-coverage:
if: github.ref == 'refs/heads/main'
needs: ci
runs-on: ubuntu-latest
permissions:
pages: write
id-token: write
environment:
name: github-pages
url: ${{ steps.deployment.outputs.page_url }}
# ... steps ...This ensures the ci job only has contents: read permission, reducing the potential impact if that job is compromised.
| - name: Run Tests | ||
| if: github.ref != 'refs/heads/main' | ||
| run: nix develop --command pnpm test | ||
|
|
||
| - name: Run Tests with Coverage | ||
| if: github.ref == 'refs/heads/main' | ||
| run: nix develop --command pnpm run coverage |
There was a problem hiding this comment.
[nitpick] The conditional test execution creates two different test paths: pnpm test for non-main branches (line 42) and pnpm run coverage for main branch (line 46).
The coverage script uses vitest run --coverage which runs tests in non-watch mode. However, the test script runs vitest which may behave differently. This could potentially lead to inconsistent test execution between PRs and main branch deployments.
To ensure consistency, consider:
- Using
vitest run(without coverage) for PRs, andvitest run --coveragefor main branch, OR - Documenting that the
testscript behavior is intentionally different
Looking at package.json line 30, pnpm test runs vitest which will run in non-watch mode in CI due to vitest.config.ts setting watch: false. This is actually consistent, so no change is needed. However, for clarity, consider using vitest run explicitly in the test script to make the intent clear.
| test: { | ||
| name: 'root', | ||
| root: '.', | ||
| include: ['src/**/*.spec.ts', 'scripts/**/*.spec.ts'], |
There was a problem hiding this comment.
[nitpick] The examples project configuration includes scripts/**/*.spec.ts in the root project's test files (line 20), but there is no scripts directory in the repository. This pattern will never match any files and serves no purpose.
Consider removing 'scripts/**/*.spec.ts' from the include array unless you plan to add scripts tests in the future:
include: ['src/**/*.spec.ts'],If scripts tests are planned for the future, this is acceptable but should be documented.
| include: ['src/**/*.spec.ts', 'scripts/**/*.spec.ts'], | |
| include: ['src/**/*.spec.ts'], |
| "lint:biome": "biome check .", | ||
| "lint:knip": "knip", | ||
| "preinstall": "npx only-allow pnpm", | ||
| "prepack": "npm pkg delete scripts.preinstall && pnpm run build", |
There was a problem hiding this comment.
[nitpick] The PR description states "Simplified test scripts - single pnpm test now runs all monorepo tests", but the previous test:submodules script that ran tests in parallel across workspace packages has been removed without replacement.
If the examples workspace package has its own tests (or will have them), the new configuration relies on Vitest's projects feature to run them. However, the examples directory currently has no test files matching the patterns **/*.spec.ts or **/*.test.ts.
This change is acceptable if the intent is to consolidate all test execution through a single Vitest command using projects. Just ensure that when tests are added to the examples directory, they follow the expected naming conventions (*.spec.ts or *.test.ts).
| "prepack": "npm pkg delete scripts.preinstall && pnpm run build", | |
| "prepack": "npm pkg delete scripts.preinstall && pnpm run build", | |
| // Note: Vitest only runs test files matching *.spec.ts or *.test.ts in all workspace packages. |
| name: 'examples', | ||
| root: './examples', | ||
| include: ['**/*.spec.ts', '**/*.test.ts'], | ||
| exclude: ['node_modules', 'dist'], |
There was a problem hiding this comment.
[nitpick] The typecheck configuration is only present in the root project (lines 23-26) but not in the examples project (lines 29-37). This means type-checking for test files will only occur for the root project's tests.
If tests are added to the examples directory in the future and they include type-level tests (e.g., *.test-d.ts files), they won't be type-checked. Consider adding a similar typecheck configuration to the examples project:
{
extends: true,
test: {
name: 'examples',
root: './examples',
include: ['**/*.spec.ts', '**/*.test.ts'],
exclude: ['node_modules', 'dist'],
typecheck: {
enabled: true,
include: ['**/*.spec.ts', '**/*.test-d.ts'],
},
},
}This ensures consistent type-checking across all projects in the monorepo.
| exclude: ['node_modules', 'dist'], | |
| exclude: ['node_modules', 'dist'], | |
| typecheck: { | |
| enabled: true, | |
| include: ['**/*.spec.ts', '**/*.test-d.ts'], | |
| }, |
- Keep vitest v4 for coverage reporting feature - Update zod to v4.1.13 from main - Regenerate pnpm-lock.yaml
Summary
What Changed
projectsfeature for monorepo supportprojectsconfiguration forrootandexamplesworkspacespnpm testnow runs all monorepo testsWhy
Addresses the need for visibility into test coverage metrics. Coverage reports will be available at the GitHub Pages URL after merging.
Notes
Closes #184
Closes #186
Summary by cubic
Add test coverage to CI, publish HTML reports to GitHub Pages on main, and generate a coverage badge. Upgraded Vitest to v4 and switched to projects for unified monorepo testing and coverage.
New Features
Dependencies
Written for commit 57c9daf. Summary will update automatically on new commits.