Skip to content

Conversation

@kumo01GitHub
Copy link
Contributor

Platforms affected

n/a

Motivation and Context

nyc doesn't seem to be actively maintained. c8 is an alternative that relies on node itself to provide coverage information rather than trying to inject instrumentation into the code.

Description

Move from nyc to c8

Testing

npm t

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
  • I've updated the documentation if necessary

@kumo01GitHub
Copy link
Contributor Author

I'd like to open this PR after PR #666 would be merged.

@kumo01GitHub kumo01GitHub marked this pull request as ready for review July 19, 2025 10:44
@erisu
Copy link
Member

erisu commented Jul 19, 2025

Can you run npm update and make sure any differences are pushed?

When I run the following:

git clean -fdx
npm i
npm update

I see a change to the package-lock.json.

Basically, node_modules/tsconfig-paths/node_modules/json5 is changed to node_modules/json5.

Previously, with nyc, we can see json5 existed in two dependencies. This would explain the node_modules/tsconfig-paths/node_modules/json5.

├─┬ @cordova/[email protected]
│ └─┬ [email protected]
│   └─┬ [email protected]
│     └── [email protected]
└─┬ [email protected]
  └─┬ [email protected]
    └─┬ @babel/[email protected]
      └── [email protected]

But as nyc is removed in this PR and c8 does not have any sub-dependencies forjson5, it became flattened.

@kumo01GitHub
Copy link
Contributor Author

kumo01GitHub commented Jul 19, 2025

This might be caused by mistake in rebase. I made sure that I got same result npm update and applying same change into master branch. Also you're right. I fixed package-lock.json by running npm update and rerun tests.

@codecov-commenter
Copy link

codecov-commenter commented Jul 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.68%. Comparing base (d37e3cf) to head (66d4243).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #668      +/-   ##
==========================================
+ Coverage   65.53%   72.68%   +7.15%     
==========================================
  Files           3        3              
  Lines         235      615     +380     
==========================================
+ Hits          154      447     +293     
- Misses         81      168      +87     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@erisu erisu left a comment

Choose a reason for hiding this comment

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

LGTM

@erisu erisu merged commit ded64da into apache:master Jul 19, 2025
11 checks passed
@kumo01GitHub kumo01GitHub deleted the chore/c8 branch July 19, 2025 16:07
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.

3 participants