Skip to content
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

Detailed coverage #1

Closed
schuay opened this issue Oct 26, 2017 · 5 comments
Closed

Detailed coverage #1

schuay opened this issue Oct 26, 2017 · 5 comments

Comments

@schuay
Copy link

schuay commented Oct 26, 2017

c8/bin/wrap.js

Line 17 in 38dff30

await Profiler.startPreciseCoverage({callCount: true, detailed: true})

Getting detailed coverage in node is currently still a bit tricky, since it requires that the source is reparsed/recompiled after detailed coverage is enabled.

In Chrome DevTools, that's easy. In Node, not so much. We're looking into ways to make this better :)

cc @hashseed @fhinkel

@bcoe
Copy link
Owner

bcoe commented Oct 26, 2017

👋

@schuay thank you for starting the conversation here, this was my motivation for opening this repo.

I work on Istanbul, which is a popular test coverage library on npm (used both for browser coverage and coverage of of files executed with Node.js). I'm really interested in finding a common approach between how V8 folks think about coverage in the browser, and how Node.js folks think about instrumenting source outside the browser.

I'm looping in @bmeck to this conversation, who's been doing a lot of low-level work on Node.js recently to support ES modules -- and might be able to speak to the difficulty this extra parsing step presents.

I will also take the time today to create GitHub issues for the other blockers I mentioned in the README.

@schuay
Copy link
Author

schuay commented Oct 26, 2017

FYI I also opened https://crbug.com/v8/7003 on the V8 tracker. Quoting from there:

An easy approach would be to add flags that enable both features at process start time, before anything is parsed/compiled. It's simple, almost no work, and would be quite easy to use from node. The drawback is that it would need to be passed as a flag when starting node, and could not just be enabled at runtime on a server.

Does that sound like it would cover common use cases? How important is it to be able to enable/disable block coverage / type profiling in a running node process?

@bcoe
Copy link
Owner

bcoe commented Oct 26, 2017

I think this would potentially work great, the tool commonly used for coverage in Node nyc already spawns a subprocess (hiding instrumentation steps from the user) this subprocess could just be spawned with Node flags.

The biggest technical head scratcher I have, is how we'd handle "process.exit()" behavior. We can capture it, but it's synchronous so we can't collect coverage once the process is starting to exit. If we could get detailed coverage with a flag, and figure out this problem, I think the rest of the work is just translating coverage output to something more easily consumed by Istanbuljs reporters (basically just typing).

CC: @isaacs

@schuay
Copy link
Author

schuay commented Oct 27, 2017

Awesome, good to know. Continued discussion for process.exit() in #2.

@bcoe
Copy link
Owner

bcoe commented Nov 22, 2017

in newer versions of v8, we're able to enable block level coverage by starting the inspector with --inspect-brk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants