-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
test: ensure callback runs in test-vm-sigint #7768
Conversation
LGTM, thanks |
Minor nit, but perhaps s/insure/ensure/ in the commit message? |
The test failed on FreeBSD with no change in symptoms: https://ci.nodejs.org/job/node-test-commit-freebsd/3322/nodes=freebsd10-64/tapTestReport/test.tap-1066/ |
LGTM even if the test is not ultimately fixed. |
@mscdex s/insure/ensure/ done |
@addaleax Still looks good to you even though it doesn't fix the test? (I still think it's a minor improvement but I don't want to make assumptions that anyone agrees with that.) |
Yep, I never expected this to fix the test in the first place. :) |
PR-URL: nodejs#7768 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Landed in fd02c93 |
PR-URL: #7768 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Checklist
make -j4 test
(UNIX), orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
test vm
Description of change
Make sure the child process
close
callback runs exactly one time./cc @addaleax