Conversation
`resume` is not necessary since `pause` is never called
mscdex
added
the
child_process
|
@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? |
|
@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 |
|
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. |
|
@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! |
|
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 ? |
|
LGTM with or without stress test |
|
Stress test on all platforms (running the test 100 times): https://ci.nodejs.org/job/node-stress-single-test/930/ |
|
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 |
|
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) |
|
CI still has failures, but looks like they are either known infra issues or issues having some other reason (mostly V8 related failures) |
|
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 <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
|
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 <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
|
thoughts re lts? |
|
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 <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
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 <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
8 participants
Checklist
make -j4 test(UNIX), orvcbuild test nosign(Windows) passesDescription of change
resumeis not necessary sincepauseis never calledRef: #4640