test: added in a better template value for a test message #16826
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Commit message is too long. |
gireeshpunathil
approved these changes
Nov 6, 2017
gireeshpunathil
added
the
code-and-learn
apapirovski
reviewed
Nov 6, 2017
| @@ -81,7 +81,8 @@ if (cluster.isWorker) { | |||
| process.once('exit', () => { | |||
| assert.strictEqual(typeof pid, 'number', | |||
| `got ${pid} instead of a worker pid`); | |||
| assert.strictEqual(alive, false, 'worker was alive after master died'); | |||
| assert.strictEqual(alive, false, | |||
| `${alive} worker was alive after master died`); | |||
There was a problem hiding this comment.
I could be misunderstanding something but wouldn't this just result in true worker was alive after master died? That said I'm not certain what the request was at the Code & Learn.
There was a problem hiding this comment.
probably the intent is to print the actual value of alive to aid diagnostics, but in this case looks like it is redundant - as the value is evident (true, as equating to false was resulted in false) and can be inferred statically.
There was a problem hiding this comment.
Theoretically, at least, isAlive() could return any value. Printing it here may be a bit much, though. I guess it could be:
assert.strictEqual(alive, false, `worker was alive after master died (alive = ${alive})`);I'd be 👍 on that change.
jasnell
approved these changes
Nov 8, 2017
Trott
approved these changes
Nov 8, 2017
Trott
pushed a commit
to Trott/io.js
that referenced
this pull request
Nov 11, 2017
Include value that cause failure in error message in test-cluster-master-kill.js. PR-URL: nodejs#16826 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
|
Landed in f7436ba. Thanks for the contribution! 🎉 |
evanlucas
pushed a commit
that referenced
this pull request
Nov 13, 2017
Include value that cause failure in error message in test-cluster-master-kill.js. PR-URL: #16826 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Merged
MylesBorins
pushed a commit
that referenced
this pull request
Nov 17, 2017
Include value that cause failure in error message in test-cluster-master-kill.js. PR-URL: #16826 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins
pushed a commit
that referenced
this pull request
Nov 17, 2017
Include value that cause failure in error message in test-cluster-master-kill.js. PR-URL: #16826 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Merged
Merged
MylesBorins
pushed a commit
that referenced
this pull request
Nov 21, 2017
Include value that cause failure in error message in test-cluster-master-kill.js. PR-URL: #16826 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins
pushed a commit
that referenced
this pull request
Nov 28, 2017
Include value that cause failure in error message in test-cluster-master-kill.js. PR-URL: #16826 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test