-
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: update setTimeout on file test/internet/test-net-connect-unref.js to use common.platformTimeout #21969
Conversation
Even better IMO would be to remove the |
Also: Welcome @conectado! Thanks for the pull request! |
@Trott First and foremost thanks for the feedback! I've started reading the Edit: I've continued reading that file, and found out that on line 1440 all arguments are converted to |
Changes the time used by the last line of the file to normalize the timeout by using the common library.
Removes the setTimeout since if the test were to fail it would time out due to the tools/test.py wrapper
I updated the PR, since the constant |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and welcome!
👍 here to fast-track. |
Landed in 586a7a4. Thanks for the contribution! 🎉 (If you're interested in other possible contributions to Node.js but don't have a good idea of where to start looking, some ideas are posted at https://www.nodetodo.org/next-steps/.) |
Removes the setTimeout since if the test were to fail it would time out due to the tools/test.py wrapper PR-URL: nodejs#21969 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
@richardlau Ugh, I forgot to push it. It's pushed now. (Thanks!) |
Landed in 586a7a4. |
Thanks everyone!! |
Removes the setTimeout since if the test were to fail it would time out due to the tools/test.py wrapper PR-URL: #21969 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Hi!
This is my first PR 😄 I'm sorry if I make any mistake.
This commit updates the time used on the
timeOut
function to confonform to the recomendation given by the guide on writing tests.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes