Skip to content

CLI-373 Fix uncovered lines in SQ caused by Bun's inaccurate LCOV reporter#232

Open
nquinquenel wants to merge 2 commits intomasterfrom
task/nq/CLI-373-lcov-bun
Open

CLI-373 Fix uncovered lines in SQ caused by Bun's inaccurate LCOV reporter#232
nquinquenel wants to merge 2 commits intomasterfrom
task/nq/CLI-373-lcov-bun

Conversation

@nquinquenel
Copy link
Copy Markdown
Member

No description provided.

@nquinquenel nquinquenel marked this pull request as ready for review April 30, 2026 10:01
@hashicorp-vault-sonar-prod
Copy link
Copy Markdown

hashicorp-vault-sonar-prod Bot commented Apr 30, 2026

CLI-373

@sonar-review-alpha
Copy link
Copy Markdown
Contributor

sonar-review-alpha Bot commented Apr 30, 2026

Summary

Problem: Bun's native --coverage and --coverage-reporter=lcov flags produce inaccurate LCOV reports that cause false "uncovered lines" in SonarQube.

Solution: Replace Bun's native coverage with Istanbul-based instrumentation for unit tests, mirroring the existing integration test approach.

Changes:

  • Unit tests: Switched from bun test --coverage to bun test --preload ./tests/coverage/preload-instrumenter.ts (new Bun plugin that instruments src/**/*.ts with Istanbul before loading)
  • Report generation: build-scripts/report-coverage.ts now merges both unit and integration raw coverage data and generates separate LCOV files for each
  • Coverage cleanup: Updated to remove both unit (raw-unit/) and integration (raw/) raw coverage directories before fresh runs
  • Shared utilities: Extracted coverage serialization logic into tests/coverage/utils.ts to reduce duplication
  • CI workflow: Updated GitHub Actions to call the new report-coverage script after unit tests complete
  • Documentation: Added detailed explanation in CLAUDE.md with a warning never to revert to Bun's native coverage

The fix ensures accurate code coverage metrics in SonarQube without breaking existing integration test coverage.

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues
0 New dependency risks

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

What reviewers should know

Where to start reading:

  1. Begin with the new tests/coverage/preload-instrumenter.ts — this is the core change that instruments unit test files with Istanbul
  2. Review tests/coverage/utils.ts — shared serialization helper used by both unit and integration flows
  3. Check build-scripts/report-coverage.ts — now handles both unit and integration coverage with flexible processing for CI/local workflows
  4. Review CLAUDE.md additions for the coverage pipeline explanation and important gotcha

Key points:

  • Process isolation: Bun runs each test file in a separate worker; the preload generates unique filenames (coverage-<timestamp>-<pid>.json) to prevent race conditions
  • Flexible reporting: The report script works when only unit data exists (CI unit job), only integration data exists (CI integration job), or both (local test:coverage run)
  • No-op on errors: Instrumentation failures are logged as warnings; tests continue to run to avoid failures due to coverage tooling
  • Critical gotcha: The CLAUDE.md warning states unit tests must never switch back to bun test --coverage — that would reintroduce false uncovered line reports in SonarQube

Review focus areas:

  • Verify the Bun plugin correctly filters to src/**/*.ts only and instruments those files
  • Confirm the report script correctly handles partial coverage data (missing raw dirs)
  • Check that the GitHub Actions workflow correctly chains unit tests → report generation before upload
  • Ensure package.json test:coverage script matches the new flow (clear → unit tests with preload → integration tests → report generation)

  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

sonar-review-alpha[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@sonar-review-alpha sonar-review-alpha Bot left a comment

Choose a reason for hiding this comment

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

LGTM! ✅

🗣️ Give feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant