test: change assert message arg from literal to template literal#16843
test: change assert message arg from literal to template literal#16843markmcnelis wants to merge 4 commits intonodejs:masterfrom
Conversation
markmcnelis
changed the title
mscdex
added
v8 engine
Trott
left a comment
Trott
left a comment
There was a problem hiding this comment.
Hi @markmcnelis! Welcome, and thanks for the PR! I have some small changes I hope you can make and then push to your branch (so it will update this PR). Thanks!
test/parallel/test-global.js
Outdated
| assert.strictEqual(global.baseFoo, 'foo', | ||
| 'x -> global.x in base level not working'); | ||
| // eslint-disable-next-line no-undef | ||
| `${baseFoo} -> global.baseFoo in base level not |
There was a problem hiding this comment.
This would result in very confusing message, IMO. I think you might be misunderstanding the original error message text.
Try removing the eslint-disable-line comment that was added and using something like this for the message instead:
`x -> global.x failed: global.baseDir = ${global.baseDir}`
test/parallel/test-global.js
Outdated
| 'bar', | ||
| 'global.x -> x in base level not working'); | ||
| // eslint-disable-next-line no-undef | ||
| `${global.baseBar} -> baseBar in base level not |
There was a problem hiding this comment.
This too will be very confusing: "bar -> baseBar in base level not working"....huh?
Try something like this instead:
`global.x -> x failed: baseBar = ${baseBar}`
Trott
added
the
code-and-learn
|
Hi @Trott! Thanks for your input. I've made some changes based on your feedback. Please review if you get a chance. Thanks |
Trott
dismissed
their stale review
Looks good to me if the linter doesn't complain and CI is green. Thanks!
PR-URL: nodejs#16843 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
|
Landed in 9468108. |
PR-URL: #16843 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #16843 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
7 participants
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test