test: add error validation to assert.throws calls - #11253
Conversation
Sorry, something went wrong.
|
I'd be really surprised if make test/vcbuild test didn't fail on this in its current state. Could you please be sure to run make test (or, on Windows, vcbuild test) on code you submit? (In this case, make jslint is enough because pummel tests don't get run by make test so we're really just linting, which make test does after running the tests. But make jslint will save you a few minutes.) |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM if CI is green. Definitely +1 on the regular expressions. Thanks for the contribution!
Sorry, something went wrong.
Sorry, something went wrong.
There was a problem hiding this comment.
Did this pass make test for you locally?
EDIT: Nevermind, I see this is a pummel test.
Sorry, something went wrong.
There was a problem hiding this comment.
I don't think these regular expressions will work. I think you need to include the error type as well. In this case:
/^Error: Unknown group$/
Sorry, something went wrong.
There was a problem hiding this comment.
I don't think this does what you're attempting to do. You'll probably have to use the RegExp() constructor here and build the regex out of strings.
Sorry, something went wrong.
There was a problem hiding this comment.
You can work with strings here. You don't need to pass regular expression literals to the RegExp constructor.
Sorry, something went wrong.
Used regular expressions to validate error messages. Also added messages (third parameter) to the assert.throws calls.
Sorry, something went wrong.
|
I'm not sure why it's saying
When you check the details there are no indications of any tests failing. Could someone explain? |
Sorry, something went wrong.
PR-URL: #11253 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
The arm build bot just reports it's status incorrectly from time to time. All is good |
Sorry, something went wrong.
PR-URL: #11253 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #11253 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #11253 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #11253 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #11253 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
I can't tell why the test was written that way in the first place, but it seems sufficient to check that setPrivateKey and setPublicKey are both undefined. Refs: nodejs/node-v0.x-archive#2638 Refs: nodejs#11253
I can't tell why the test was written that way in the first place, but it seems sufficient to check that setPrivateKey and setPublicKey are both undefined. Refs: nodejs/node-v0.x-archive#2638 Refs: #11253 PR-URL: #49404 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
I can't tell why the test was written that way in the first place, but it seems sufficient to check that setPrivateKey and setPublicKey are both undefined. Refs: nodejs/node-v0.x-archive#2638 Refs: #11253 PR-URL: #49404 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Used regular expressions to validate error messages.
Also added messages (third parameter) to the assert.throws calls.
Checklist
Affected core subsystem(s)
test