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

process.exit breaks coverage #20

Closed
laggingreflex opened this issue Aug 8, 2018 · 4 comments
Closed

process.exit breaks coverage #20

laggingreflex opened this issue Aug 8, 2018 · 4 comments

Comments

@laggingreflex
Copy link
Contributor

If you have process.exit in a file, it doesn't show any coverage.

Repro is simple:

index.js

// process.exit() // uncomment me

Run c8 node index.js

  • Without process.exit()

    ----------|----------|----------|----------|----------|----------------|
    File      |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
    ----------|----------|----------|----------|----------|----------------|
    All files |      100 |      100 |      100 |      100 |                |
     index.js |      100 |      100 |      100 |      100 |                |
    ----------|----------|----------|----------|----------|----------------|
    
  • With process.exit()

    ----------|----------|----------|----------|----------|----------------|
    File      |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
    ----------|----------|----------|----------|----------|----------------|
    All files |  Unknown |  Unknown |  Unknown |  Unknown |                |
    ----------|----------|----------|----------|----------|----------------|
    
@demurgos
Copy link
Contributor

demurgos commented Aug 8, 2018

This is due to the way c8 currently reports its results: it attaches a handler to onExit. If the process stops immediately, the handler is not triggered and no files are written.

The deep cause for this behavior is that there is a mismatch between the c8 architecture and the v8 debugger. In c8, we have a top Node process acting as the server, and many sub-processes reporting its results to it. But V8 is made so the inspected process acts as the server and you can have many inspectors connecting to it.

In practice, this is not an issue when using coverage for tests because an abrupt exit usually means that there is a deeper issue with your tests.

A good solution would be to be allow a spawn-wrapped process to use process.runMain in an unwrapped process but that has its own subprocesses being still wrapped. It should be possible with --require and some env variables.
Longer term, the cleanest solution would be to have a new V8 flag to ask V8 to connect to the inspector.

Edit: I posted a message on V8-users, not sure if it's the right place.

@demurgos
Copy link
Contributor

demurgos commented Aug 9, 2018

I think that this issue depends on: istanbuljs/spawn-wrap#47.

If you were able to set the args dynamically, you maintain process isolation: process.exit in the covered process would not stop its parent.

Edit: I submitted a rough sketch for an event-based spawn-wrap API that would solve the issue, see linked issue.

@bcoe
Copy link
Owner

bcoe commented Sep 10, 2018

@laggingreflex this should now be fixed with the version on master, mind giving it a shot? (you'll need to use Node 10.10.0)

We should add a test for this, so that we can catch any regressions.

@laggingreflex
Copy link
Contributor Author

Yep, it works now.

@bcoe bcoe closed this as completed Sep 12, 2018
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

No branches or pull requests

3 participants