-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use V8 builtin coverage reporting #7062
Comments
@aaronabramov you had tried this, or just looked at it? |
👋 I'd love to help with this as best I can, there are a few outstanding issues that make the experience a little bit clunky, but shouldn't block a proof of concept: A strategy for handling source-maps also needs to be developed. I've been chatting with @jdalton about some of these issues, as they relate to ESM -- perhaps we can aim for a community effort. |
We talked a tiny bit internally last week, and it was the sourcemaps that we figured would be hard part. How much of that is handled in devtools? Or, do you have a write-up about it somewhere? |
@SimenB in nyc, we handle source-maps with a fairly tiny abstraction on top of Mozilla's source-map library. Some of this might be able to be torn out of nyc ... it might be better to start over with fresh eyes, especially since source-map has switched to an async API (nyc is pinned on the old sync API). |
For coverage I suppose the async api works, but jest is also stuck on the sync api because of test errors which must be collected synchronously. Although I have hope for mozilla/source-map#331 |
for mozilla/source-map#331, I have created POC as discussion mozilla/source-map#369 - while I have low expectation if it could be accepted due to some hacky workaround to achieve sync interfaces. /cc @SimenB for fyi. |
How can we get the ball rolling on this? Using V8 coverage and using native ES modules would remove the need for Jest to use babel out-of-the box, right? |
You'd still need it to get mock hoisting, but I suppose that's something people can live without. This was actually unblocked by bcoe/c8#85, so if anyone wants to work on this, that'd be awesome! 🙂 |
@SimenB 👋 one issue we still have is source-maps that are injected dynamically, because c8 doesn't hook module.require; I'd love to get the ball rolling on implementing source-maps in V8/Node.js itself. |
Jest controls the source code and manually executes it in a sandbox, so we should be able to inject it still, right? Built-in sourcemap support in the engine(s) would be absolutely amazing! |
@SimenB I think with some fiddling we should be able to figure out a way to provide the source-map, might take some work in v8-to-istanbul; the main limitation faced by c8, is that it doesn't intercept source-code requires itself (which is a deprecated API any ways). |
I'm in your slack, happy to try to figure out a way forwards in the not too distant future (swamped at work for the last few months and for a couple more). I think this might be quite revolutionary if we're able to make it work |
Could you expand a bit on this? Intercept how? In Jest's case, we inject our own |
For anyone following along: I've experimented a bit with this, but the current approach is very FS based - you have to set an env variable before running node (we could wrap it a spawning node process, so not a biggie), but the report is only output to when the process exits. That makes it hard to properly deal with sourcemaps (since we manage them in memory, although they do exist on disk). Which means watch mode and coverage is a no-go (and we also have to do more manual process management). Open issue request for a programmatic approach: nodejs/node#28283 |
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
🚀 Feature Proposal
V8 provides a native way of getting code coverage. Recently a c8 project by @bcoe was released to use this feature in Node land. Would be cool to experiment with this edgy tech.
Here's a blog post about it.
Motivation
Make coverage 2-3x faster and babel-free.
The text was updated successfully, but these errors were encountered: