Removes call to net.Socket.resume#8679
Uh oh!
There was an error while loading. Please reload this page.
Conversation
`resume` is not necessary since `pause` is never called
mscdex added the child_process 
Trott commented Sep 21, 2016
ALJCepeda commented Sep 21, 2016
@Trott I'm having trouble understanding why the test is failing. I see that it has 3 failures for pi1-raspbian-wheezy:
I'm confused because I don't know how this applies to the changes I've made or even how to address them. On a related note, Is it possible for me to run these CI tests before I make another pull request? |
gibfahn commented Sep 21, 2016
@ALJCepeda the three failures on the Raspberry Pis look unrelated, we're currently working on fixing infrastructure issues and general flakyness on the Pis. Running |
gibfahn commented Sep 21, 2016
Incidentally, if you look at https://ci.nodejs.org/job/node-test-pull-request/ you can see that the last ~30 runs of that job have failed, suggesting an infrastructure issue. I'll try the CI again, but I suspect we'll have to wait until the underlying issues are fixed. |
ALJCepeda commented Sep 21, 2016
@gibfahn I must have looked at the wrong history because I saw other builds were passing and assumed the fail had to be specific to my changes. Now I know where to check, Thank you! |
gibfahn commented Sep 21, 2016
New CI has the same problem, but as the failures were only on ARM, and the test that was changed is actually still passing on ARM (link), I think we can safely say that the CI passed. However it might be worth doing a stress test just to make sure there aren't any intermittent failures. Thoughts @Trott ? |
gibfahn commented Sep 21, 2016
LGTM with or without stress test |
Trott commented Sep 21, 2016
Stress test on all platforms (running the test 100 times): https://ci.nodejs.org/job/node-stress-single-test/930/ |
imyller commented Sep 22, 2016
New CI (with V8 5.4 now in master): https://ci.nodejs.org/job/node-test-pull-request/4230/ |
Update still holding for ~3 more hours for full 48 hour PR window |
gibfahn commented Sep 23, 2016
Yet another CI (build failures): https://ci.nodejs.org/job/node-test-pull-request/4235/ This should be good to land anyway (previous CI runs all look good except for infra issues) |
imyller commented Sep 23, 2016
CI still has failures, but looks like they are either known infra issues or issues having some other reason (mostly V8 related failures) |
imyller commented Sep 23, 2016
I'll start landing this:
|
In test parallel/child-process-fork-net2 `net.Socket.resume()` is not necessary since `net.Socket.pause()` is never called. PR-URL: nodejs#8679 Refs: nodejs#4640 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ilkka Myller <[email protected]>
imyller commented Sep 23, 2016
landed in d9a6d4a Thank you for your effort and contribution, @ALJCepeda |
In test parallel/child-process-fork-net2 `net.Socket.resume()` is not necessary since `net.Socket.pause()` is never called. PR-URL: #8679 Refs: #4640 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ilkka Myller <[email protected]>
MylesBorins commented Oct 6, 2016
thoughts re lts? |
jasnell commented Oct 7, 2016
should be fine for v4 I think. |
In test parallel/child-process-fork-net2 `net.Socket.resume()` is not necessary since `net.Socket.pause()` is never called. PR-URL: #8679 Refs: #4640 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ilkka Myller <[email protected]>
In test parallel/child-process-fork-net2 `net.Socket.resume()` is not necessary since `net.Socket.pause()` is never called. PR-URL: #8679 Refs: #4640 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ilkka Myller <[email protected]>
8 participants
Checklist
make -j4 test(UNIX), orvcbuild test nosign(Windows) passesDescription of change
resumeis not necessary sincepauseis never calledRef: #4640