-
Notifications
You must be signed in to change notification settings - Fork 29k
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
assert: make sure throws is able to handle primitives #20482
Conversation
|
c3026d9
to
6b1f886
Compare
New CI https://ci.nodejs.org/job/node-test-pull-request/14696/ @nodejs/testing PTAL |
As I understand it, this makes |
@addaleax that is indeed an edge case and actually a side effect of this fix. It is meant to fix: assert.throws(() => { throw 4; }, { message: '4' })
TypeError: Cannot use 'in' operator to search for 'message' in 4
at compareExceptionKey (assert.js:374:13) I will work around it to prevent it from being semver-major. |
@nodejs/collaborators PTAL |
This fixes some possible issues with `assert.throws` in combination with an validation object. It will now properly handle primitive values being thrown as error. It also makes sure the `generatedMessage` property is properly set if `assert.throws` is used in combination with an validation object and improves the error performance in such cases by only creating the error once.
ee4907a
to
34b4818
Compare
Rebased due to conflicts. CI before landing: https://ci.nodejs.org/job/node-test-pull-request/14781/ |
This fixes some possible issues with `assert.throws` and `assert.rejects` in combination with an validation object. It will now properly handle primitive values being thrown as error. It also makes sure the `generatedMessage` property is properly set if `assert.throws` or `assert.rejects` is used in combination with an validation object and improves the error performance in such cases by only creating the error once. In addition it will fix detecting regular expressions from a different context such as n-api that are passed through as validator for `assert.throws` or `assert.rejects`. Until now those were not tested. PR-URL: nodejs#20482 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
Landed in 560925f |
This fixes some possible issues with `assert.throws` and `assert.rejects` in combination with an validation object. It will now properly handle primitive values being thrown as error. It also makes sure the `generatedMessage` property is properly set if `assert.throws` or `assert.rejects` is used in combination with an validation object and improves the error performance in such cases by only creating the error once. In addition it will fix detecting regular expressions from a different context such as n-api that are passed through as validator for `assert.throws` or `assert.rejects`. Until now those were not tested. PR-URL: #20482 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
This fixes some possible issues with
assert.throws
in combinationwith an validation object. It will now properly handle primitive
values being thrown as error.
It also makes sure the
generatedMessage
property is properly setif
assert.throws
is used in combination with an validation objectand improves the error performance in such cases by only creating
the error once.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes