Skip to content

Conversation

@takyyon
Copy link
Contributor

@takyyon takyyon commented Jan 9, 2026

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

The PR coverage check workflow was not accurately detecting uncovered files due to several issues:

  1. Files in apps/Standalone/ were being checked but this dev environment has no tests
  2. Using basename for file matching could match wrong files with the same name in different directories
  3. Packages without coverage data were silently skipped instead of failing
  4. Manifest and swagger files (configuration/data) were being checked for coverage

This fix improves the accuracy of the coverage check so PRs with untested code are properly blocked.

Impact of Change

  • Users: No user-facing changes
  • Developers: Coverage check will now properly fail when files in testable packages lack coverage
  • System: More accurate CI coverage reporting

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in: GitHub Actions workflow run

Manual Testing Evidence

Issue identified in PR #8690: The coverage check was incorrectly flagging manifest files and Standalone app files that don't have test coverage.

Changes made:

  1. Added exclusions for apps/Standalone/**, apps/docs/**
  2. Added exclusions for **/manifest/**, **/manifests/**, **/swagger/**
  3. Improved file path matching (full relative path instead of basename)
  4. Added explicit package coverage requirements

Verification:

Why automated tests are not feasible:
This is a GitHub Actions workflow file that can only be fully tested by running the workflow itself in CI. The bash script logic operates on lcov coverage reports which are generated during the CI run.

Contributors

@krrishmittal

Screenshots/Videos

N/A - CI workflow changes only

- Add exclusions for apps/Standalone and apps/docs (no tests)
- Add exclusions for manifest and swagger directories (config/data files)
- Use full relative path matching instead of basename for accurate file matching in lcov reports
- Add explicit list of packages that require coverage
- Fail when files in covered packages have no coverage data
- Report skipped files for packages not configured for coverage

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

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings January 9, 2026 01:23
@github-actions
Copy link

github-actions bot commented Jan 9, 2026

🤖 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: fix(ci): Improve PR coverage check workflow accuracy
  • Issue: None — title follows the conventional commit style and accurately describes the change.
  • Recommendation: (Optional) This is good as-is.

Commit Type

  • Properly selected (fix).
  • Only one item was selected, which is correct for this change.

Risk Level

  • The PR body selects Low and the PR has the risk:low label. These match and are appropriate given the change touches only CI workflow configuration.

What & Why

  • Current: The What & Why section is present and explains the problems (incorrect coverage checks for Standalone/docs, basename matching issues, packages without coverage being skipped, manifest/swagger being checked) and the reasons for the changes.
  • Issue: None.
  • Recommendation: (Optional) The explanation is clear — no change required.

Impact of Change

  • Impact section is provided and readable:
    • Users: No user-facing changes
    • Developers: Coverage check will now properly fail for files in testable packages that lack coverage
    • System: More accurate CI coverage reporting
  • Recommendation: Consider listing the exact package directories impacted by the new COVERED_PACKAGES list (already present in the workflow) in the Impact section for quick reference to reviewers.

Test Plan

  • The author selected Manual testing and provided an explanation why: workflow changes require running in CI. They also provided a GitHub Actions run verifying the behavior.
  • Assessment: Acceptable for a workflow change. The manual/CICD evidence is included.

Contributors


Screenshots/Videos

  • Screenshots not applicable for CI workflow changes — N/A is appropriate.

Summary Table

Section Status Recommendation
Title No change needed
Commit Type No change needed
Risk Level No change needed
What & Why No change needed
Impact of Change (Optional) Mention the exact package list from the workflow for clarity
Test Plan Manual/CICD evidence provided — acceptable for workflow changes
Contributors No change needed
Screenshots/Videos N/A is appropriate

Final Notes

  • The change modifies only the GitHub Actions workflow (.github/workflows/pr-coverage.yml) and adjusts exclusions and coverage-matching logic. From the diff, this is a non-production code change (CI config) and the advised risk level is risk:low, which matches the PR's declared risk.
  • One small operational item: I see the needs-pr-update label still attached to the PR. If you believe the PR is ready, please remove that label so automated triage/maintainers know it no longer needs updates.

Please update/remove the needs-pr-update label if you consider this PR final. Otherwise, no further edits to the PR title/body are required — this passes the PR title and body compliance check. Thank you for the clear description and for including CI verification evidence!


Last updated: Fri, 09 Jan 2026 01:29:33 GMT

@takyyon takyyon added the risk:low Low risk change with minimal impact label Jan 9, 2026
@github-actions
Copy link

github-actions bot commented Jan 9, 2026

📊 Coverage check completed. See workflow run for details.

@github-actions
Copy link

github-actions bot commented Jan 9, 2026

📊 Coverage check completed. See workflow run for details.

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 fixes accuracy issues in the PR coverage check workflow that were causing it to incorrectly report coverage status. The workflow was matching files incorrectly using basename (which could match wrong files with the same name), checking files in packages without tests (like Standalone), and silently skipping packages that should have coverage.

Key Changes:

  • Fixed file path matching to use relative paths with anchors instead of basename matching
  • Added explicit list of packages requiring coverage with validation
  • Excluded non-testable directories (Standalone app, docs, config files) from coverage checks

@takyyon takyyon merged commit b39aa61 into main Jan 9, 2026
17 of 18 checks passed
@takyyon takyyon deleted the fix/pr-coverage-workflow-improvements branch January 9, 2026 01:34
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