-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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 assert message arg from literal to template literal #16843
Conversation
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.
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
@@ -34,17 +34,21 @@ baseFoo = 'foo'; // eslint-disable-line no-undef | |||
global.baseBar = 'bar'; | |||
|
|||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
|
||
assert.strictEqual(baseBar, // eslint-disable-line no-undef | ||
'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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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}`
Hi @Trott! Thanks for your input. I've made some changes based on your feedback. Please review if you get a chance. Thanks |
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>
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
test