Skip to content

Conversation

@avivkeller
Copy link
Member

@avivkeller avivkeller commented Jun 3, 2025

V8 (at least the way we wrote it in the test runner) only provides coverage for files the tests actually import, which leads to a skewed coverage report.

By including all the files via the use of c8, we can see that we actually have a huge lack of coverage.

Copilot AI review requested due to automatic review settings June 3, 2025 23:28
@avivkeller avivkeller requested a review from a team as a code owner June 3, 2025 23:28
@codecov-commenter
Copy link

codecov-commenter commented Jun 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 41.24%. Comparing base (702cb43) to head (53f360c).
Report is 12 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #304       +/-   ##
===========================================
- Coverage   90.58%   41.24%   -49.34%     
===========================================
  Files          63       94       +31     
  Lines        4767     7092     +2325     
  Branches      191      211       +20     
===========================================
- Hits         4318     2925     -1393     
- Misses        446     4164     +3718     
  Partials        3        3               

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

This comment was marked as outdated.

@avivkeller avivkeller force-pushed the feat/accurate-coverage branch from c69f3b4 to 53f360c Compare June 4, 2025 00:03
@avivkeller avivkeller requested a review from Copilot June 4, 2025 00:11
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 enhances test coverage reporting by leveraging c8 to include all project files. Key changes include:

  • Updating test commands in package.json to use c8 instead of experimental Node coverage flags.
  • Modifying the CI workflow to reference the new coverage file location.
  • Adding a .c8rc.json configuration to enable coverage for all files.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

File Description
package.json Updated test scripts to use c8 and added a c8 dependency
.github/workflows/ci.yml Updated the coverage file path to match c8 output
.c8rc.json Introduced configuration to include all files in coverage
Comments suppressed due to low confidence (2)

.github/workflows/ci.yml:73

  • Confirm that the updated coverage file path './coverage/lcov.info' correctly aligns with the output directory generated by c8 to avoid CI mismatches.
"files": "./coverage/lcov.info",

package.json:14

  • Verify that switching to 'c8 npm test' in 'test:coverage' preserves all necessary environment settings and behaviors from the previous Node experimental flags.
"test:coverage": "c8 npm test",

@avivkeller avivkeller mentioned this pull request Jun 4, 2025
4 tasks
@avivkeller avivkeller merged commit 42bbad6 into main Jun 4, 2025
8 checks passed
@avivkeller avivkeller deleted the feat/accurate-coverage branch June 4, 2025 13:23
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.

4 participants