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

outputting coverage when process.exit() is called #2

Closed
bcoe opened this issue Oct 26, 2017 · 6 comments
Closed

outputting coverage when process.exit() is called #2

bcoe opened this issue Oct 26, 2017 · 6 comments

Comments

@bcoe
Copy link
Owner

bcoe commented Oct 26, 2017

I think @chrisdickonson came up with a good idea for how to approach this, if we capture an event on process.exit, we'll use spawnSync to output coverage in a subprocess.

const client = await CRI({port: port})
const {Profiler} = client

process.on('exit', () => {
  process.spawnSync('ws-write-coverage.js', ())
  client.close()
})

CC: @schuay, @bmeck

@schuay
Copy link

schuay commented Oct 27, 2017

It'd be awesome to solve this using the current V8 API; but if that turns out to be a bad solution I could imagine emitting an inspector event on Isolate teardown containing all remaining coverage information. Just an idea so far, not sure how happy inspector folks would be about that.

cc @hashseed, @caseq

@schuay schuay mentioned this issue Oct 27, 2017
@bcoe
Copy link
Owner Author

bcoe commented Oct 28, 2017

@schuay looping in @TimothyGu and @eugeneo, who look to have done a chunk of the work on the Inspector binding in Node.js.

The approach I outlined above doesn't quite work. Calling inspector.close() in the context of process.exit() seems to hang the Node process... process.exit is a weird method, in that it can only perform synchronous operations, and the even loop is guaranteed to exit on the next tick (my guess is that inspector.close() steps on the toes of the shutdown process).

Even though this is potentially a bug we can fix in Node, as you point out @schuay, it might be a bit nicer to change around the API a bit so that you can more easily execute an operation as soon as your application has completed execution; maybe I'm just missing something?

@bcoe
Copy link
Owner Author

bcoe commented Oct 28, 2017

digging into this a bit more, I was able to get things to terminate in process.exit() but by the time we've hit process.exit() I believe the inspector session is already terminated so it's not possible to use spawnSync to communicate with the parent process.

@TimothyGu
Copy link
Contributor

How about process.on('beforeExit')?

@bcoe
Copy link
Owner Author

bcoe commented Oct 29, 2017

@TimothyGu good idea, I called unref on the socket connecting to the inspector, and switched to a beforeExit handler:

#4

This seems to work well if the application exits normally, or if the application throws an exception. If the program exits due to a process.exit(0), beforeExit does not fire (am I missing something, haven't used this hook before)?

this is definitely progress!

@bcoe
Copy link
Owner Author

bcoe commented Nov 22, 2017

there's an event that fires when a script finishes execution that we can wire into.

@bcoe bcoe closed this as completed Nov 22, 2017
j03m added a commit to j03m/c8 that referenced this issue Mar 17, 2020
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

3 participants