Skip to content

feat: add test coverage reporting with GitHub Pages deployment #188

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ryoppippi merged 7 commits into from
Dec 9, 2025
Merged

feat: add test coverage reporting with GitHub Pages deployment #188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b5a03ee
feat(ci): add test coverage reporting with GitHub Pages deployment
ryoppippi Dec 9, 2025
dcb482d
chore(deps): upgrade vitest to v4.0.15
ryoppippi Dec 9, 2025
cc4c412
feat(test): configure vitest projects for monorepo coverage
ryoppippi Dec 9, 2025
0c7bf74
chore: update actionss
ryoppippi Dec 9, 2025
be9259e
feat(ci): add coverage badge generation
ryoppippi Dec 9, 2025
9873b82
chore(ci): update actions
ryoppippi Dec 9, 2025
57c9daf
merge: resolve conflict with main, update to zod v4
ryoppippi Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
name: CI

on:
push:
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
push:
push:
branches:
- main
Fix with Cubic

Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:
      - main

This ensures CI runs on:

  • Pushes to the main branch (for coverage deployment)
  • Pull requests targeting main (for PR validation)
Suggested change
push:
push:
branches:
- main

Copilot uses AI. Check for mistakes.
pull_request:
branches:
- main
Expand All @@ -9,6 +10,11 @@ concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

permissions:
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Fix with Cubic

contents: read
pages: write
id-token: write
Comment on lines +13 to +16
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

jobs:
ci:
runs-on: ubuntu-latest
Expand All @@ -32,4 +38,34 @@ jobs:
run: nix develop --command pnpm run build

- 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
Comment on lines 34 to +46
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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:

  1. Using vitest run (without coverage) for PRs, and vitest run --coverage for main branch, OR
  2. Documenting that the test script 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.

Copilot uses AI. Check for mistakes.

- name: Create Coverage Badge
if: github.ref == 'refs/heads/main'
uses: jaywcjlove/coverage-badges-cli@bd6ccbf422c0ed54c01f283019fd2bc648f58541 # v2.2.0
with:
source: coverage/coverage-summary.json
output: coverage/badges.svg

- name: Upload coverage artifact
if: github.ref == 'refs/heads/main'
uses: actions/upload-pages-artifact@7b1f4a764d45c48632c6b24a0339c27f5614fb0b # v4.0.0
with:
path: coverage

deploy-coverage:
if: github.ref == 'refs/heads/main'
needs: ci
runs-on: ubuntu-latest
environment:
name: github-pages
url: ${{ steps.deployment.outputs.page_url }}
steps:
- name: Deploy to GitHub Pages
id: deployment
uses: actions/deploy-pages@d6db90164ac5ed86f2b6aed7e0febac5b3c0c03e # v4.0.5
5 changes: 2 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,8 @@
"lint:knip": "knip",
"preinstall": "npx only-allow pnpm",
"prepack": "npm pkg delete scripts.preinstall && pnpm run build",
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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).

Suggested change
"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.

Copilot uses AI. Check for mistakes.
"test": "pnpm --aggregate-output run '/^test:/'",
"test:submodules": "pnpm --parallel -r --aggregate-output test",
"test:root": "vitest",
"test": "vitest",
"coverage": "vitest run --coverage",
"typecheck": "pnpm --aggregate-output run '/^typecheck:/'",
"typecheck:submodules": "pnpm --parallel -r --aggregate-output typecheck",
"typecheck:root": "tsgo --noEmit"
Expand Down
Loading
Loading