Wait for event loop to drain before exiting workers?

[novemberborn]

I'm considering changing the workers to not automatically exit once the last hook has finished. Ordinarily the event loop will drain and the process will exit on its own. However if some code is still running it could be part of a test. It may even call an assertion that should really cause the test to fail (see #1330).

Changing this behavior will encourage code that cleans up after itself. However I think we'd need an option to revert to the original behavior.

The --timeout option will apply to this to clean up workers that aren't exiting on their own. We should report the offending test files.

Interrupting AVA should do the same (see #583).

Thoughts?

[vadimdemedes]

Could AVA only output some error to tell a user that a certain test suite was still running instead of waiting for all the I/O to finish? If all the tests completed, but something is still running, that test suite should be considered failing right away.

[sindresorhus]

I'm fine with this as long as we display what tests are blocking AVA from finishing on its own.

[novemberborn]

Could AVA only output some error to tell a user that a certain test suite was still running instead of waiting for all the I/O to finish? If all the tests completed, but something is still running, that test suite should be considered failing right away.

@vadimdemedes I like that, but we'd still need to give the test files a little bit of time to exit on their own. And then we may have to let users configure that…

I'm fine with this as long as we display what tests are blocking AVA from finishing on its own.

@sindresorhus it'd be test files, but yes we should show this.

I think if we land #583 we could set a default timeout (perhaps 10 seconds?). It'd automatically apply to this use case since eventually the worker pool gets filled up with workers that won't exit, and then the timeout is triggered. This feels to me like the cleanest approach.

[sindresorhus]

And then we may have to let users configure that…

Configure the time, or whether to exit right away or wait to drain?

[novemberborn]

And then we may have to let users configure that…

Configure the time, or whether to exit right away or wait to drain?

We need an option so users can make AVA exit right away. Or an API so they can do it from a test.after.always() hook but I'd rather make it an option.

But the wait-to-drain period also needs to be customizable. There may be a complex shutdown sequence or a remote database teardown that takes seconds. I don't really want an option for that, hence my suggestion of using a default (but long) timeout value and applying that to the entire test run.

[sindresorhus]

For inspiration, here's how Jest solved it:

https://facebook.github.io/jest/docs/en/cli.html#detectopenhandles

I think we should do something similar.

[cancerberoSgx]

I'm having an issue in another scenario but 99% sure is caused by this - and which in jest is solved by --detectOpenHandles. I'm running ava tests with a customized node.js distribution that substitute the default event loop with a custom one, concretly https://github.com/nodegui/qode which uses Qt event loop.

Do you know how to workaround this problem? - tests run - test.after() callback run, but the node.js process won't exit. If I run the same code without ava then it does. any tip is most appreciated - if you want I could open a new issue to track this but I'm sure is the same cause.