test: increase dgram timeout for armv6 #2808
Conversation
test-dgram-broadcast-multi-process.js and test-dgram-multicast-multi-process.js were failing on Pi 1 because the test was timing out. Changed static 5000ms timeout to a dynamically determined timeout based on the processor using common.platformTimeout().
|
Oh, and you can see the tests failing in prior CI here: https://ci.nodejs.org/job/node-test-commit-arm/591/ |
ChALkeR
added
test
|
If the tests were passing before with 5s, I think doing 35s would be a bit too much. How about something like 2s which would still be 14s on that Pi. |
|
The tests take about 11-13 seconds to run on the arm6 machines, so I'm not so sure 35 seconds is out of line if we're looking to have a timeout large enough that the test won't sometimes be flaky. Each test file has a 60 second timeout (unless that's increased for armv6, but I don't think it is), and for this test on armv6, I'd actually be OK giving it the full 60s before blowing up. |
|
I don't think these tests were ever passing on Raspberry Pi 1 machines. We don't (yet) run these tests on CI. (I had to do modify the |
|
LGTM FWIW |
|
35s seems a bit much, but, eh. |
|
@Trott: The 60s test timeout is actually increased to 180s for ARMv6, which might be a bit much actually, but otherwise, this is LGTM, thought a test that takes this long isn't really optimal. |
|
If it makes anyone feel less conflicted, there are dozens of tests already in the suite that runs on CI that take as long or longer on the Raspberry Pi 1 devices than these tests take. Sometimes, they take far longer. I'm all for efforts to try to speed these tests up, of course. |
test-dgram-broadcast-multi-process.js and test-dgram-multicast-multi-process.js were failing on Pi 1 because the test was timing out. Changed static 5000ms timeout to a dynamically determined timeout based on the processor using common.platformTimeout(). PR-URL: #2808 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
|
Thanks, landed in bcc6d47 (just saw another test failure on this). |
test-dgram-broadcast-multi-process.js and test-dgram-multicast-multi-process.js were failing on Pi 1 because the test was timing out. Changed static 5000ms timeout to a dynamically determined timeout based on the processor using common.platformTimeout(). PR-URL: #2808 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
test-dgram-broadcast-multi-process.js and test-dgram-multicast-multi-process.js were failing on Pi 1 because the test was timing out. Changed static 5000ms timeout to a dynamically determined timeout based on the processor using common.platformTimeout(). PR-URL: #2808 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
test-dgram-broadcast-multi-process.js and
test-dgram-multicast-multi-process.js were failing on Pi 1 because the
test was timing out. Changed static 5000ms timeout to a dynamically
determined timeout based on the processor using
common.platformTimeout().
These tests aren't (yet) currently run on CI because they are in
test/internet. But I altered the Makefile and had them run at https://ci.nodejs.org/job/node-test-commit-arm/593/