avajs / ava

Node.js test runner that lets you develop with confidence 🚀
MIT License
20.74k stars 1.41k forks source link

Shared worker teardown #2734

Open novemberborn opened 3 years ago

novemberborn commented 3 years ago

Based on this discussion it'd be great to have a teardown method within the shared worker. E.g. after negotiating the protocol, you can call .teardown() to register a callback.

Under normal use, after a test run, AVA can explicitly message the shared workers to begin their teardown. They must then respond when complete. The worker may then exit on its own or be terminated explicitly.

In watch mode, AVA should keep the workers alive until it's made to exit. It should then tear down the workers gracefully, as per above.

We should no longer unreference the worker:

https://github.com/avajs/ava/blob/bdf2cf07e8010820a600cbc8f005be3715239e07/lib/plugin-support/shared-workers.js#L115

Looking at this now, the whole deregistration bit here makes no sense:

https://github.com/avajs/ava/blob/bdf2cf07e8010820a600cbc8f005be3715239e07/lib/plugin-support/shared-workers.js#L54-L58

The shared worker may end up in a "deregistered" state if ever there is a gap between test workers using it.

The API instance should probably track shared worker instances, not in a per-run variable like now, but on the instance itself. This will require some rewiring:

https://github.com/avajs/ava/blob/bdf2cf07e8010820a600cbc8f005be3715239e07/lib/api.js#L236

Then, perhaps, we could add a teardown() method to the API itself, which we can call after its run completes:

https://github.com/avajs/ava/blob/bdf2cf07e8010820a600cbc8f005be3715239e07/lib/cli.js#L467

Watch mode requires the process to be "killed" using a SIGINT signal. This is currently handled internally be the API instance:

https://github.com/avajs/ava/blob/d7426720ad55ba387d86aaae3a3af06a8020c761/lib/api.js#L55

And then per run:

https://github.com/avajs/ava/blob/d7426720ad55ba387d86aaae3a3af06a8020c761/lib/api.js#L96

It might make more sense to register for SIGINT in the lib/cli.js file and then call an interrupt() method on the API instance, after which we can kick off the teardown.

When SIGINT is received a second time we should forcibly exit the process, without waiting for a graceful teardown of the shared workers.

novemberborn commented 3 years ago

@timdp so I had a look, and this is quite involved. Let me know if you're still keen, or perhaps I should do some refactoring first so just the teardown work itself remains?

timdp commented 3 years ago

I'm of course eager to have the teardown behavior but it's not my decision. :wink: Like I said earlier, happy to contribute if at all possible.