-
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
improved assert message on cluster-worker-disconnect #16021
Conversation
assert.strictEqual(w.voluntaryMode, true, | ||
'Voluntary exit mode was not set'); | ||
'Voluntary exit mode was not set, current voluntary mode is ${w.voluntaryMode}'); |
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.
The line length is to long. It would actually be best to just remove the message overall. The message from strictEqual
should be better than the current situation. The same applies to w.state
.
ping @Hadis-Fard |
@@ -58,7 +58,7 @@ if (cluster.isWorker) { | |||
// Disconnect worker when it is ready | |||
worker.once('listening', common.mustCall(() => { | |||
const w = worker.disconnect(); | |||
assert.strictEqual(worker, w, 'did not return a reference'); | |||
assert.strictEqual(worker, w, '${worker.id} did not return a reference'); |
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.
nit: This needs backticks on the string literal rather than single quote marks.
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
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
Improve assert messages in test-cluster-worker-disconnect.js. PR-URL: nodejs#16021 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Landed in eefee3e. Thanks for the contribution! 🎉 |
Improve assert messages in test-cluster-worker-disconnect.js. PR-URL: #16021 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Improve assert messages in test-cluster-worker-disconnect.js. PR-URL: #16021 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Improve assert messages in test-cluster-worker-disconnect.js. PR-URL: #16021 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Improve assert messages in test-cluster-worker-disconnect.js. PR-URL: nodejs/node#16021 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Improve assert messages in test-cluster-worker-disconnect.js. PR-URL: nodejs/node#16021 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Improve assert messages in test-cluster-worker-disconnect.js. PR-URL: #16021 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Improve assert messages in test-cluster-worker-disconnect.js. PR-URL: #16021 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Improve assert messages in test-cluster-worker-disconnect.js. PR-URL: #16021 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Improve assert messages in test-cluster-worker-disconnect.js. PR-URL: nodejs/node#16021 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)