add failing test demonstrating long running onExit() hook in child process

[jamestalmage]

RE: #134

[vadimdemedes]

I assume this is safe to close since #136 is in master?

[jamestalmage]

I would prefer to see it merged.
It provides a test that mimics the nyc issue and confirms the previous commits are actually doing what we want.

[jamestalmage]

despite the branch name, it does not actually fix anything (you beat me to the punch on that).

it's just a test

[vadimdemedes]

I think this test is kind of complicating it, since we already have a test for this. What situation does this test checks?

[vadimdemedes]

Just not understanding the purpose completely. Could you please explain?

[jamestalmage]

So,

The existing test covers making sure we force close some process that is being kept open due to a perpetual process (i.e. Firebase, express, etc). If that is all that is going on, then the process should exit immediately in once it receives the kill signal (no delay).

That's the setTimeout(15 seconds) in the long-running-test, and then an assertion that we DON'T wait for that setTimeout to finish.

The other scenario is when we send a kill signal to the process, and it intentionally intercepts and delays closure of the process. This is the case with nyc. Nyc specifically uses signal-exit to intercept the kill signal and delay the exit until it is done writing to disk. signal-exit only accepts synchronous callbacks. (I am guessing this is a safety measure to make sure you don't screw things up with a complicated async exit process, and fail to ever let the process exit). This is the the onExit portion.

I run through a 2 second loop

var start = Date.now()
while (Date.now() - start > 2000) {} // just waiting 2 seconds synchronously because I have to be synchronous inside onExit
process.send({..delay-completed-event...})

Before all our changes we were exiting the parent and killing the child before the delay-completed-event. Even though we fixed the problem, I realized my initial test actually didn't cover the full scenario.

I agree, it's not a super straightforward test, but I am simulating a rather convoluted process and I don't know how to make it simpler.

[jamestalmage]

these assertion messages are bad. I should fix them

[vadimdemedes]

Those are error messages, they are displayed when assertion fails.

[vadimdemedes]

Haha, race condition :D

[jamestalmage]

right - I've got them in the affirmative ('did NOT wait for setTimeout(fn, 15000')') -that is actually what we want (do not wait 15 seconds).

[sindresorhus]

@jamestalmage I've added Windows testing with AppVeyor and this assertion fails: https://ci.appveyor.com/project/sindresorhus/ava/build/2/job/w7efao2if7n2g2q9 Could you take a look?

[vadimdemedes]

Ok, let's see what @sindresorhus thinks.

[sindresorhus]

👍