feat: force exit the process when completed

[popomore]

Checklist

• npm test, tests passing
• npm run test:snap (to update the snapshot)
• tests and/or benchmarks are included
• documentation is changed or added

[coveralls]

Pull Request Test Coverage Report for Build 183

• 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.009%) to 95.861%

---

Totals
Change from base Build 162:	0.009%
Covered Lines:	392
Relevant Lines:	402

---

💛 - Coveralls

[shinnn]

Elaborate on what kind of problem this patch solves in #82 (comment).

[addaleax]

Calling process.exit() without needing to is generally discouraged; for example, it may not allow scheduled writes to the disk or to stdout/stderr to finish. So, like @shinnn said, it would be helpful to know what issue this solves.

[popomore]

It's unnecessary to call process.exit directly, foreground-child will do this. I have pushed the new code.

[shinnn]

Not needed. Even if no callback function is provided, foreground-child itself will do what you do here. https://github.com/tapjs/foreground-child/blob/v1.5.6/index.js#L22-L24

[popomore]

But c8 has passed ths cb argument without done.

[shinnn]

Indeed.

[shinnn]

Again #82 (review),

Elaborate on what kind of problem this patch solves in #82 (comment).

Otherwise LGTM.

[bcoe]

@popomore (like @shinnn mentioned too) mind providing an example of the bug this fixes for you? it would be good to know so that we can write a test for it.

[popomore]

Can you check this repo https://github.com/popomore/c8-mocha?

If you run npm run cov, the process will not exit. It seems that if ipc channel is opened, the listener should be cleaned as foreground-child done.

[bcoe]

@popomore 👍 gotcha, thanks for clarifying; and simply ensuring that we call done() addresses the issue?

[popomore]

Yes, foreground-child will remove the listener after calling done

[shinnn]

I think there is no need to add a regression test here.

Instead, foreground-child should validate its last argument by checking Function#length; If the Function#length of the callback function is less than 1, foreground-child should throw an error.

[popomore]

@shinnn Thank for your accepting

@bcoe when will be released? I want to test c8 in my code, comparing the performance with nyc.

[bcoe]

@popomore please try npm i c8@next, I'll have the official release out a bit later this week.

[popomore]

Thanks