-
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: fix flaky test-vm-sigint-existing-handler #7982
test: fix flaky test-vm-sigint-existing-handler #7982
Conversation
Set the `SIGUSR2` handler before spawning the child process to make sure the signal is always handled. Ref: nodejs#7854 Fixes: nodejs#7981
LGTM but thinking that a quick code comment would be helpful also. |
@jasnell Done! CI: https://ci.nodejs.org/job/node-test-commit/4406/ |
CI run on AIX here: https://ci.nodejs.org/job/node-test-commit-aix/nodes=aix61-ppc64/289/console |
LGTM if the AIX CI run is green |
Thank you! LGTM if CI is green. speaking of which... @nodejs/build ... CI seems to be particularly flaky with arm and windows today. |
CI run failed, not sure if it was a one, off, trying master first to make sure still passes there: https://ci.nodejs.org/job/node-test-commit-aix/290/ |
CI re-run was ok so believe it was a fluke - https://ci.nodejs.org/job/node-test-commit-aix/291. |
process.on('SIGUSR2', common.mustCall(() => { | ||
// First kill() breaks the while(true) loop, second one invokes the real | ||
// signal handlers. | ||
process.kill(child.pid, 'SIGINT'); | ||
}, 3)); | ||
|
||
const child = spawn(process.execPath, [ __filename, 'child' ], { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before landing, would you mind removing the spaces inside of the braces?
LGTM |
Landed in be73480 |
Checklist
make -j4 test
(UNIX), orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
test
Description of change
Set the
SIGUSR2
handler before spawning the child process to make sure the signal is always handled.(Basically the same as #7854)
Fixes: #7981