-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Wait for event loop to drain before exiting workers? #1718
Comments
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. |
I'm fine with this as long as we display what tests are blocking AVA from finishing on its own. |
@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…
@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. |
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 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. |
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. |
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. |
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?
The text was updated successfully, but these errors were encountered: