Conversation
Collect V8 coverage from the Storybook process spawned during E2E tests and convert it to lcov via c8, so Codecov can merge it with unit test coverage. Source maps are now generated in the build but excluded from npm via .npmignore.
|
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #156 +/- ##
=======================================
Coverage ? 74.96%
=======================================
Files ? 31
Lines ? 819
Branches ? 204
=======================================
Hits ? 614
Misses ? 158
Partials ? 47 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull request overview
This PR adds E2E code coverage collection to complement existing unit test coverage. The implementation enables V8 coverage collection from the spawned Storybook process during E2E tests, then uses c8 to convert the raw V8 coverage data to lcov format with source-mapped TypeScript paths.
Changes:
- Enable source map generation in tsdown builds for V8-to-source mapping
- Set
NODE_V8_COVERAGEenvironment variable during CI test runs to collect V8 coverage from spawned processes - Add c8 dependency and script to convert V8 coverage to lcov format
- Upload both unit test and E2E coverage reports to Codecov
- Exclude source maps from published npm packages
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tsdown-shared.config.ts | Enables sourcemap generation for all packages to support V8 coverage source mapping |
| turbo.json | Adds e2e-coverage/** to test:ci task outputs for turbo caching |
| package.json | Adds c8 dependency, sets NODE_V8_COVERAGE in test:ci, adds test:ci:report-e2e-coverage script |
| packages/mcp/.npmignore | Excludes .map files from npm package |
| packages/addon-mcp/.npmignore | Excludes .map files from npm package |
| .gitignore | Ignores e2e-coverage/ and e2e-coverage-report/ directories |
| .github/workflows/check.yml | Adds E2E coverage reporting step and uploads e2e-coverage-report/lcov.info to Codecov |
| pnpm-lock.yaml | Adds c8 and its dependencies; libc annotations from pnpm update |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
kasperpeulen
force-pushed
the
kasper/e2e-coverage
branch
2 times, most recently
from
eb8767c to
dcc11c4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 12 changed files in this pull request and generated 2 comments.
Files not reviewed (4)
- apps/internal-storybook/pnpm-lock.yaml: Language not supported
- eval/pnpm-lock.yaml: Language not supported
- packages/addon-mcp/pnpm-lock.yaml: Language not supported
- pnpm-lock.yaml: Language not supported
| "storybook": "turbo watch storybook --filter=@storybook/mcp-internal-storybook", | ||
| "test": "vitest", | ||
| "test:ci": "vitest run --coverage --reporter=default --reporter=github-actions --reporter=junit --outputFile=test-report.junit.xml", | ||
| "test:ci": "NODE_V8_COVERAGE=./e2e-coverage vitest run --coverage --reporter=default --reporter=github-actions --reporter=junit --outputFile=test-report.junit.xml", |
There was a problem hiding this comment.
NODE_V8_COVERAGE=./e2e-coverage is a relative path, but the E2E tests spawn Storybook with cwd: STORYBOOK_DIR (apps/internal-storybook). That means the Storybook process will write V8 coverage into apps/internal-storybook/e2e-coverage, while the later c8 report --temp-directory ./e2e-coverage step reads from the repo root. This can result in missing/empty E2E coverage (or a failing c8 report). Consider using an absolute path for NODE_V8_COVERAGE (so it’s independent of child process CWD) or explicitly overriding NODE_V8_COVERAGE in the Storybook spawn env to point at the root coverage directory.
| "test:ci": "NODE_V8_COVERAGE=./e2e-coverage vitest run --coverage --reporter=default --reporter=github-actions --reporter=junit --outputFile=test-report.junit.xml", | |
| "test:ci": "NODE_V8_COVERAGE=\"$PWD/e2e-coverage\" vitest run --coverage --reporter=default --reporter=github-actions --reporter=junit --outputFile=test-report.junit.xml", |
There was a problem hiding this comment.
The relative path works because the Storybook process ends up running from the repo root (turbo resets CWD). Verified locally — all V8 coverage files land in ./e2e-coverage/ at the repo root, none in apps/internal-storybook/e2e-coverage/.
package.json
Outdated
| "test": "vitest", | ||
| "test:ci": "vitest run --coverage --reporter=default --reporter=github-actions --reporter=junit --outputFile=test-report.junit.xml", | ||
| "test:ci": "NODE_V8_COVERAGE=./e2e-coverage vitest run --coverage --reporter=default --reporter=github-actions --reporter=junit --outputFile=test-report.junit.xml", | ||
| "test:ci:report-e2e-coverage": "c8 report --src ./packages --all -r lcov --temp-directory ./e2e-coverage -o ./e2e-coverage-report", |
There was a problem hiding this comment.
c8 report is run with --all and --src ./packages, which forces every file under ./packages into the E2E lcov output even if Storybook never executed it. When Codecov merges coverage/lcov.info and e2e-coverage-report/lcov.info, this can introduce lots of 0-hit files and reduce overall coverage unexpectedly. If the goal is to report only what the spawned Storybook process actually covered, drop --all and/or narrow the --src scope to the subset of packages/files Storybook can load.
| "test:ci:report-e2e-coverage": "c8 report --src ./packages --all -r lcov --temp-directory ./e2e-coverage -o ./e2e-coverage-report", | |
| "test:ci:report-e2e-coverage": "c8 report --src ./packages -r lcov --temp-directory ./e2e-coverage -o ./e2e-coverage-report", |
There was a problem hiding this comment.
Investigated this — tried dropping --all but actually Codecov merges by taking the max hit count per line, so --all with --src ./packages is correct. Files at 0% in E2E will be covered by unit tests, and Codecov keeps the higher count. Keeping --all ensures that if we add a new source file and forget to cover it in both unit and E2E, it shows up as 0% rather than being invisible.
kasperpeulen
force-pushed
the
kasper/e2e-coverage
branch
from
70bf67f to
6311970
Compare
kasperpeulen
force-pushed
the
kasper/e2e-coverage
branch
from
6311970 to
c3e5b1d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 12 changed files in this pull request and generated no new comments.
Files not reviewed (4)
- apps/internal-storybook/pnpm-lock.yaml: Language not supported
- eval/pnpm-lock.yaml: Language not supported
- packages/addon-mcp/pnpm-lock.yaml: Language not supported
- pnpm-lock.yaml: Language not supported
… test coverage" This reverts commit c3e5b1d.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 12 changed files in this pull request and generated no new comments.
Files not reviewed (4)
- apps/internal-storybook/pnpm-lock.yaml: Language not supported
- eval/pnpm-lock.yaml: Language not supported
- packages/addon-mcp/pnpm-lock.yaml: Language not supported
- pnpm-lock.yaml: Language not supported
c8's default v8-to-istanbul produces inflated line counts (e.g. 212 vs 57 for the same file) because it maps all source lines including types and imports. monocart-coverage-reports uses AST analysis to count only executable lines, producing line counts within 1-3 of vitest's output. This enables proper Codecov merging of unit + e2e coverage.
kasperpeulen
force-pushed
the
kasper/e2e-coverage
branch
from
c692be7 to
2eadcb9
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 13 changed files in this pull request and generated 1 comment.
Files not reviewed (4)
- apps/internal-storybook/pnpm-lock.yaml: Language not supported
- eval/pnpm-lock.yaml: Language not supported
- packages/addon-mcp/pnpm-lock.yaml: Language not supported
- pnpm-lock.yaml: Language not supported
| const filtered = lcov | ||
| .split('end_of_record\n') | ||
| .filter((section) => { | ||
| const match = section.match(/^SF:(.+)$/m); | ||
| return match && regex.test(match[1]); | ||
| }) | ||
| .join('end_of_record\n'); | ||
|
|
There was a problem hiding this comment.
The filter logic may produce an empty result if no sections match the regex pattern. When all sections are filtered out, the script will write only end_of_record\n to the lcov file, which could cause issues with downstream tools expecting valid coverage data.
Consider adding a check after filtering to verify that at least one section matched, and either log a warning or exit with an error if no matches are found. This would make debugging easier if the pattern is incorrect or if the coverage data structure changes.
| const filtered = lcov | |
| .split('end_of_record\n') | |
| .filter((section) => { | |
| const match = section.match(/^SF:(.+)$/m); | |
| return match && regex.test(match[1]); | |
| }) | |
| .join('end_of_record\n'); | |
| const sections = lcov.split('end_of_record\n'); | |
| const filteredSections = sections.filter((section) => { | |
| const match = section.match(/^SF:(.+)$/m); | |
| return match && regex.test(match[1]); | |
| }); | |
| if (filteredSections.length === 0) { | |
| console.error( | |
| `filter-lcov: no coverage sections matched pattern ${JSON.stringify( | |
| pattern, | |
| )} in file ${file}`, | |
| ); | |
| process.exit(1); | |
| } | |
| const filtered = filteredSections.join('end_of_record\n'); |
Bundle ReportChanges will decrease total bundle size by 39.69kB (-100.0%) ⬇️. This is within the configured threshold ✅ Detailed changes
|
Collect V8 coverage from the Storybook process spawned during E2E tests and upload it to Codecov alongside unit test coverage.
NODE_V8_COVERAGEintest:cito collect coverage from the spawned Storybook processtest:ci:report-e2e-coveragescript usingc8withmonocart-coverage-reportsto convert V8 coverage to lcovsrc/files, matching vitest'sincludepattern.npmignoreto both packages to exclude.mapfiles from published packagesCoverage: 73.90% → 74.96% (+1.06%)