ci: connect features folder to SonarCloud coverage#14908
Conversation
❌ Action Required: Monitored Files ChangedThe following files in monitored folders have been modified:
Action Required: Please rebase your branch against git rebase origin/develop |
There was a problem hiding this comment.
Pull request overview
Connects the root-level features/ workspace packages to the existing SonarCloud coverage + test execution reporting pipeline, aligning them with how apps/ and libs/ are already handled.
Changes:
- Extend Sonar configuration to include
features/sources and TS config paths. - Add
jest-sonarreporting for@features/market-bannerand aggregate feature coverage artifacts in CI. - Introduce and wire a new reusable workflow/job (
test-features) into PR and scheduled Sonar scan flows.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
sonar-project.properties |
Adds features/ to Sonar sources, test exclusions, and TS config discovery. |
features/market-banner/jest.config.js |
Generates Sonar test execution XML via jest-sonar. |
.github/workflows/test-features-reusable.yml |
New reusable workflow to run feature coverage and upload aggregated artifacts. |
.github/workflows/sonar.yml |
Scheduled scan now installs feature deps and merges feature coverage into the scan inputs. |
.github/workflows/scan-sonar-reusable.yml |
Optionally downloads/merges features coverage artifacts for PR sonar scans. |
.github/workflows/build-and-test-pr.yml |
Adds test-features job and feeds its coverage into the Sonar scan workflow. |
.changeset/wild-moose-buy.md |
Adds a changeset entry related to the features Sonar coverage wiring. |
| description: | | ||
| If you run this manually, and want to run on a PR, the correct ref should be refs/pull/{PR_NUMBER}/merge to | ||
| have the "normal" scenario involving checking out a merge commit between your branch and the base branch. | ||
| If you want to run only on a branch or specific commit, you can use either the sha or the branch name instead (prefer the first verion for PRs). |
There was a problem hiding this comment.
Typo in the workflow_dispatch ref input description: “verion” should be “version” (helps avoid copying this typo into other workflow files).
| If you want to run only on a branch or specific commit, you can use either the sha or the branch name instead (prefer the first verion for PRs). | |
| If you want to run only on a branch or specific commit, you can use either the sha or the branch name instead (prefer the first version for PRs). |
mcayuelas-ledger
force-pushed
the
chore/connect-features-sonarcloud
branch
from
b6f4cf4 to
ea1388a
Compare
mcayuelas-ledger
force-pushed
the
chore/connect-features-sonarcloud
branch
from
ea1388a to
7e244cd
Compare
| if [ -d ./coverage-features ]; then | ||
| echo "Merging features coverage files" | ||
| cat ./coverage-features/lcov.info >> ./lcov.info | ||
| cat ./coverage-features/sonar-executionTests-report.xml >> ./sonar-executionTests-report.xml | ||
| fi |
There was a problem hiding this comment.
In the Sonar scan reusable workflow, the merge step appends the features sonar-executionTests-report.xml into a single file via cat >>. If that XML file already contains its own <testExecutions> root (common for Jest Sonar reporters), appending multiple reports will make the combined file non-well-formed XML and may break Sonar test execution import. Consider switching to a merge strategy that preserves a single root element, or pass multiple report paths to Sonar.
| test-features: | ||
| name: "Test Features" | ||
| uses: LedgerHQ/ledger-live/.github/workflows/test-features-reusable.yml@develop | ||
| secrets: inherit | ||
|
|
There was a problem hiding this comment.
This workflow adds a test-features job, but the final ok job (used as the single required check) does not include test-features in its needs. That means the required OK check can succeed even if test-features fails, and it may also complete before test-features finishes. Add test-features to the ok.needs list (and ensure failures are reflected).
| mkdir -p ${{ github.workspace }}/coverage | ||
| touch ${{ github.workspace }}/coverage/lcov.info | ||
| touch ${{ github.workspace }}/coverage/sonar-executionTests-report.xml | ||
|
|
||
| pnpm turbo run coverage --api="http://127.0.0.1:${{ steps.setup-caches.outputs.port }}" --token="${{ secrets.TURBOREPO_SERVER_TOKEN }}" --team="foo" --filter="./features/**" --concurrency=4 -- --runInBand=1 | ||
|
|
||
| - name: Process coverage files | ||
| id: process-coverage | ||
| run: | | ||
| for package_json in $(find ./features -name "package.json" -not -path "*/node_modules/*"); do | ||
| pkg=$(dirname "$package_json") | ||
| echo "Processing feature at path: $pkg" | ||
| if [ -f "$package_json" ] && jq -e '.scripts.coverage' "$package_json" > /dev/null 2>&1; then | ||
| if [ -f "$pkg/coverage/lcov.info" ] && [ -f "$pkg/coverage/sonar-executionTests-report.xml" ]; then | ||
| echo "Appending coverage files for $pkg" | ||
| cat "$pkg/coverage/lcov.info" >> ${{ github.workspace }}/coverage/lcov.info | ||
| cat "$pkg/coverage/sonar-executionTests-report.xml" >> ${{ github.workspace }}/coverage/sonar-executionTests-report.xml | ||
| fi |
There was a problem hiding this comment.
The feature coverage aggregation builds sonar-executionTests-report.xml by concatenating per-package XML files. If more than one feature package produces a Jest Sonar report, this will typically result in an invalid XML document (multiple <testExecutions> roots), which can break Sonar test execution parsing. Consider merging only the inner <file> nodes under a single root, or avoid merging and instead pass multiple report paths to Sonar.
| - name: Generate Unit test coverage for features | ||
| run: | | ||
| mkdir -p features/coverage | ||
| touch features/coverage/lcov.info | ||
| touch features/coverage/sonar-executionTests-report.xml | ||
|
|
||
| pnpm turbo run coverage --filter="./features/**" --concurrency=4 -- --runInBand=1 | ||
|
|
||
| for package_json in $(find ./features -name "package.json" -not -path "*/node_modules/*"); do | ||
| pkg=$(dirname "$package_json") | ||
| echo "Processing feature at path: $pkg" | ||
|
|
||
| if [ -f "$package_json" ] && jq -e '.scripts.coverage' "$package_json" > /dev/null 2>&1; then | ||
| if [ -f "$pkg/coverage/lcov.info" ] && [ -f "$pkg/coverage/sonar-executionTests-report.xml" ]; then | ||
| echo "Appending coverage files for $pkg" | ||
| cat "$pkg/coverage/lcov.info" >> features/coverage/lcov.info | ||
| cat "$pkg/coverage/sonar-executionTests-report.xml" >> features/coverage/sonar-executionTests-report.xml | ||
| fi | ||
| else | ||
| echo "No coverage script found for $pkg, skipping." | ||
| fi | ||
| done |
There was a problem hiding this comment.
This scheduled Sonar workflow aggregates features/**/coverage/sonar-executionTests-report.xml by concatenating XML files. Once multiple feature packages exist, this can produce an invalid XML document (multiple <testExecutions> roots) and make the Sonar scan fail to parse unit test execution. Prefer merging XML properly (single root + combined <file> elements) or configuring Sonar to read multiple report files rather than concatenating.
mcayuelas-ledger
force-pushed
the
chore/connect-features-sonarcloud
branch
from
6a20cfc to
b4cc7b3
Compare
|
| name: "Test Features" | ||
| needs: determine-affected | ||
| if: ${{contains(needs.determine-affected.outputs.paths, 'features') && !github.event.pull_request.head.repo.fork}} | ||
| uses: LedgerHQ/ledger-live/.github/workflows/test-features-reusable.yml@chore/connect-features-sonarcloud |
There was a problem hiding this comment.
The new test-features job calls a reusable workflow at LedgerHQ/ledger-live/.github/workflows/test-features-reusable.yml@develop, but that workflow file is introduced by this PR (so it won’t exist on the develop ref until after merge). This will cause the PR workflow to fail when GitHub Actions tries to resolve the reusable workflow. Consider either splitting this into two PRs (add reusable workflow first, then wire it in), or referencing a ref that includes this change (e.g., a pinned commit SHA or an in-repo reference if supported).
| uses: LedgerHQ/ledger-live/.github/workflows/test-features-reusable.yml@chore/connect-features-sonarcloud | |
| uses: ./.github/workflows/test-features-reusable.yml |
|
lewisd5
force-pushed
the
chore/connect-features-sonarcloud
branch
from
5686e3c to
b4cc7b3
Compare
✅ Checklist
npx changesetwas attached.jest-sonargenerates the expectedsonar-executionTests-report.xmlandlcov.infofiles.features/foldertest-featuresjob in PR pipeline📝 Description
The
features/folder at the root of the monorepo was not connected to SonarCloud. Code infeatures/market-banner(and any future feature packages) was invisible to coverage analysis and quality gates.This PR wires the
features/folder into the SonarCloud pipeline by:featurestosonar.sourcesandsonar.typescript.tsconfigPathsinsonar-project.propertiesjest-sonarreporter tofeatures/market-banner/jest.config.jsto generatesonar-executionTests-report.xmltest-features-reusable.yml(following the same pattern astest-libs-reusable.yml)test-featuresjob into the PR workflow (build-and-test-pr.yml), the sonar scan reusable workflow (scan-sonar-reusable.yml), and the scheduled scan (sonar.yml)❓ Context
🧐 Checklist for the PR Reviewers