test: fix test-vm-sigint flakiness#7854
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Set the `SIGUSR2` handler before spawning the child process to make sure the signal is always handled.
santigimeno commented Jul 23, 2016
Maybe fixes #7767 ? |
addaleax commented Jul 23, 2016
LGTM CI: https://ci.nodejs.org/job/node-test-commit/4238/ |
Trott commented Jul 24, 2016
Unfortunately, I don't think the stress test means #7767 is fixed because the stress test doesn't fail on current master either. (Or at least it hasn't yet. 1000 runs and counting: https://ci.nodejs.org/job/node-stress-single-test/815/nodes=centos5-32/console) So there's probably some weird interaction with a previous test or a specific build peculiarity or something like that going on. Regardless, if this is an improvement to the test, then awesome. I'm OK with a "maybe fixes". |
santigimeno commented Jul 24, 2016
@Trott I couldn't reproduce it running the test multiple times sequentially in my |
santigimeno commented Jul 30, 2016
@Trott does this LGTY? |
Trott commented Jul 30, 2016
@santigimeno Yes, LGTM. If it fixes it (which, judging from what you say, it does) then great. If it turns out that it doesn't fix it, then it's a harmless change. |
Trott commented Jul 31, 2016
@santigimeno Great! (I don't think there's anything holding up #7859 if you want to land it, is there?) |
santigimeno commented Jul 31, 2016
You're right. Just landed it. |
cjihrig commented Aug 1, 2016
LGTM |
1 similar comment
jasnell commented Aug 1, 2016
LGTM |
Trott commented Aug 2, 2016
Now that there's code added to print the signal, it fails like this: https://ci.nodejs.org/job/node-test-commit-linux/4427/nodes=centos5-64/console |
Trott commented Aug 2, 2016
|
addaleax commented Aug 2, 2016
Yup, sounds good. I don’t think there’s anything speaking against landing this (and adding a |
addaleax commented Aug 2, 2016
Landed in 93ac2ea, thanks! |
Set the `SIGUSR2` handler before spawning the child process to make sure the signal is always handled. Fixes: #7767 PR-URL: #7854 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Set the `SIGUSR2` handler before spawning the child process to make sure the signal is always handled. Ref: nodejs#7854Fixes: nodejs#7981
Set the `SIGUSR2` handler before spawning the child process to make sure the signal is always handled. Ref: #7854Fixes: #7981 PR-URL: #7982 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Set the `SIGUSR2` handler before spawning the child process to make sure the signal is always handled. Fixes: #7767 PR-URL: #7854 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Set the `SIGUSR2` handler before spawning the child process to make sure the signal is always handled. Ref: #7854Fixes: #7981 PR-URL: #7982 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
6 participants
Checklist
make -j4 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
test
Description of change
Set the
SIGUSR2handler before spawning the child process to make surethe signal is always handled.
The test was sometimes crashing in my
OS Xbox when running the test suite because ofSIGUSR2not being handled.