-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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: Change equal to strictEqual and pass delay to setTimeout #9938
Conversation
cb81a4e
to
3c78fb5
Compare
@@ -26,7 +26,7 @@ function err() { | |||
console.error('This should not happen.'); | |||
disposalFailed = true; | |||
process.exit(1); | |||
}); | |||
}, 0); |
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.
@qualquervalor2 Could you check if this works with setImmediate()
instead of setTimeout()
?
If it doesn't work, could you change the 0
to a 1
? The 0 becomes a 1 under the hood so it would be better to be explicit up front. :)
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.
setImediate() did not work. Updated the PR to use 1.
@@ -26,7 +26,7 @@ function err() { | |||
console.error('This should not happen.'); | |||
disposalFailed = true; | |||
process.exit(1); | |||
}); | |||
}, 1); |
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.
This can likely be changed to use common.mustCall()
... for instance:
setTimeout(common.mustCall(() => {}, 0), 1);
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
@Fishrock123 ... are you good with this now? |
change equal to strictEqual, fix setTimeout PR-URL: nodejs#9938 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Landed in 25dfb8e |
Thanks for the contribution! 🎉 |
change equal to strictEqual, fix setTimeout PR-URL: #9938 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
change equal to strictEqual, fix setTimeout PR-URL: nodejs#9938 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
change equal to strictEqual, fix setTimeout PR-URL: #9938 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
change equal to strictEqual, fix setTimeout PR-URL: #9938 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
change equal to strictEqual, fix setTimeout PR-URL: #9938 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Checklist
make -j8 test
(UNIX), orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
test
Description of change