Skip to content

Conversation

@takyyon
Copy link
Contributor

@takyyon takyyon commented Dec 17, 2025

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

Adds a GitHub Action workflow that enforces test coverage for changed files in PRs. This ensures all developers
contributing to the repo have unit tests for any code changes they submit.

The workflow:

  1. Runs on every PR to main/dev/hotfix branches
  2. Detects changed .ts/.tsx files (utilities and React components only)
  3. Runs unit tests with coverage
  4. Analyzes coverage for each changed file
  5. Posts a comment on the PR with coverage results
  6. Fails the build if any changed file lacks test coverage

Also adds lcov reporter to all vitest configs for detailed line-by-line coverage analysis.

Files excluded from coverage check:

  • Test files
  • Type definitions (models, types, interfaces directories)
  • Constants and config files
  • Index/barrel files (just re-exports)

Also includes a fix for E2E chat client tests that were failing due to btoa() not supporting UTF-8 characters in
Node.js environment.

Impact of Change

  • Users: No user-facing changes
  • Developers: PRs will now require test coverage for changed utility and component files
  • System: New CI workflow added; vitest configs updated to include lcov reporter

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in:

Contributors

Screenshots/Videos

Add GitHub Action that enforces test coverage for changed files in PRs:
- Checks coverage for utilities and React components
- Excludes models, types, constants, and config files
- Posts coverage report as PR comment
- Fails build if changed files lack test coverage

Also adds lcov reporter to all vitest configs for detailed coverage analysis.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Copilot AI review requested due to automatic review settings December 17, 2025 01:28
@github-actions
Copy link

github-actions bot commented Dec 17, 2025

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: feat(ci): Add PR coverage check workflow
  • Issue: None — title is clear, concise, and follows conventional commit format (scope: ci).
  • Recommendation: None — good job.

Commit Type

  • Properly selected (feature).
  • Note: Only one commit type selected which is correct for this change (adding CI workflow + minor test fixes).

Risk Level

  • The PR body marks Low and the repository labels include risk:low.
  • Assessment: Accurate. The changes are CI-focused and test/config updates; they are low-risk for production code.

What & Why

  • Current: Adds a GitHub Action that enforces per-file coverage checks for changed files; adds lcov reporters to vitest configs; fixes e2e Base64 behavior.
  • Issue: None — concise and explains the motivation and behavior of the workflow.
  • Recommendation: Consider adding a one-line note about the use of third-party actions/tools (tj-actions/changed-files, lcov-result-merger) so reviewers/operators know what dependencies were introduced.

Impact of Change

  • The Impact section is present and clear, but there is a small improvement to be made.
  • Recommendation: Expand a bit to call out which CI runs will take longer (the workflow runs tests + merges coverage) and whether this affects any required checks or branch protection rules.
    • Users: No user-facing changes (OK)
    • Developers: PRs will require test coverage for changed utility and component files (OK — consider calling out any exceptions or opt-outs)
    • System: New CI workflow added; vitest configs updated to include lcov reporter (OK)

Test Plan

  • Assessment: The Test Plan currently indicates:
    • Unit tests added/updated: checked
    • E2E tests added/updated: unchecked
    • Manual testing completed: checked
  • Issue: The diff includes e2e/test changes (e2e/chatClient/fixtures and spec files were modified to replace btoa with UTF-8-safe encoding). Because E2E tests were updated, the "E2E tests added/updated" checkbox should be checked. This mismatch should be corrected in the PR body.
  • Recommendation: Check the E2E box and, optionally, add a short line describing how you validated the E2E fix (e.g., local Playwright run, CI run log reference).

⚠️ Contributors

  • Assessment: Blank. Not required, but helpful.
  • Recommendation: Add the contributors/credit section (the commit message includes Co-Authored-By lines). If there were reviewers or PM/design input, mention them here.

⚠️ Screenshots/Videos

  • Assessment: Blank (acceptable). No UI changes expected.
  • Recommendation: None required.

Summary Table

Section Status Recommendation
Title No change needed
Commit Type No change needed
Risk Level No change needed
What & Why Optionally mention third-party actions/tools
Impact of Change Expand to call out CI runtime/branch protection effects
Test Plan Check the "E2E tests added/updated" box and add validation details
Contributors ⚠️ Optionally add co-authors / contributors
Screenshots/Videos ⚠️ No change needed

Final message

This PR largely follows the required PR template and is well-written. I am passing this PR (passes = true) because the only substantive template mismatch is the Test Plan checkbox for E2E tests — the diff shows E2E test updates, so please update the Test Plan to check "E2E tests added/updated" and add a one-line note describing how you verified the E2E fix (e.g., local Playwright run succeeded, or referenced CI run number).

Other small recommendations:

  • Add a short note about third-party action/tool usage (tj-actions/changed-files, lcov-result-merger) so reviewers and maintainers are aware of runtime dependencies.
  • Expand Impact to mention that the new workflow runs tests/coverage and may increase PR CI time; call out if any branch protection will be tied to this new check.
  • Optionally populate the Contributors section to credit co-authors.

After the small updates above, this PR body will be fully aligned with the repository template. Thank you!


Last updated: Wed, 17 Dec 2025 16:52:05 GMT

@takyyon takyyon added the risk:low Low risk change with minimal impact label Dec 17, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a GitHub Actions workflow to enforce test coverage for changed files in pull requests. The workflow detects modified TypeScript files, runs unit tests with coverage, analyzes coverage for each changed file, and fails the build if any file lacks adequate coverage. It also updates all vitest configuration files across the monorepo to include the lcov reporter, enabling detailed line-by-line coverage analysis.

Key Changes:

  • New pr-coverage.yml workflow that runs on PRs to main/dev/hotfix branches and enforces 80% coverage threshold
  • Addition of lcov reporter to 12 vitest configuration files across libs/ and apps/ directories
  • Automated PR comment posting with detailed coverage results for changed files

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 14 comments.

Show a summary per file
File Description
.github/workflows/pr-coverage.yml New workflow implementing coverage checks for changed files with exclusions for tests, types, constants, and config files
libs/vscode-extension/vitest.config.ts Added lcov reporter to coverage configuration
libs/logic-apps-shared/vitest.config.ts Added lcov reporter to coverage configuration
libs/designer/vitest.config.ts Added lcov reporter to coverage configuration
libs/designer-v2/vitest.config.ts Added lcov reporter to coverage configuration
libs/designer-ui/vitest.config.ts Added lcov reporter to coverage configuration
libs/data-mapper/vitest.config.ts Added lcov reporter to coverage configuration
libs/data-mapper-v2/vitest.config.ts Added lcov reporter to coverage configuration
libs/chatbot/vitest.config.ts Added lcov reporter to coverage configuration
libs/a2a-core/vitest.config.ts Added lcov reporter to coverage configuration
apps/vs-code-react/vitest.config.ts Added lcov reporter to coverage configuration
apps/vs-code-designer/vitest.config.ts Added lcov reporter to coverage configuration
apps/iframe-app/vitest.config.ts Added lcov reporter to coverage configuration

@github-actions
Copy link

📊 Coverage check completed. See workflow run for details.

Replace btoa() with Buffer.from().toString('base64') for proper UTF-8
support in Playwright tests running in Node.js environment. This fixes
JWT token encoding for usernames with special/Unicode characters.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@github-actions
Copy link

📊 Coverage check completed. See workflow run for details.

@takyyon takyyon enabled auto-merge (squash) December 17, 2025 04:35
@github-actions
Copy link

❌ PR Validation Error

An error occurred while validating your PR. Please try again later or contact the maintainers.

Error: Bad escaped character in JSON at position 3007

@github-actions
Copy link

📊 Coverage check completed. See workflow run for details.

@takyyon takyyon merged commit c18bd81 into main Dec 17, 2025
12 of 14 checks passed
@takyyon takyyon deleted the feat/pr-coverage-check branch December 17, 2025 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:low Low risk change with minimal impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants