-
Notifications
You must be signed in to change notification settings - Fork 30k
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-net-write-slow #7555
Conversation
Increase socket timeout so that there is enough time to reliably run the test on FreeBSD. Fixes: nodejs#7516
(Still kind of wonder if the test shouldn't be removed, though. See #7516 for archeological and other details.) |
CI is green. |
@nodejs/testing |
What about changing the timeout value based on |
@mscdex That's an option, but I don't think it's preferable in this situation. The arbitrarily large value doesn't make the test take any longer to pass. If it affected the test duration, I'd be inclined to tune the value based on platform. But since it doesn't affect execution time on successful tests, I'd prefer to avoid the complexity of different values on different platforms. |
This FreeBSD failure on CI is starting to get a little out of control, so I'm going to try to raise the profile of this fix and argue for an expedited landing (small change, big positive impact, easy to revert, doesn't affect userland code). Raising profile: @nodejs/collaborators |
LGTM |
LGTM |
Increase socket timeout so that there is enough time to reliably run the test on FreeBSD. Fixes: nodejs#7516 PR-URL: nodejs#7555 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Landed in 9d654a3. Goodbye, red FreeBSD CI. |
Checklist
make -j4 test
(UNIX), orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
test net
Description of change
Increase socket timeout so that there is enough time to reliably run the
test on FreeBSD.
Fixes: #7516