-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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: Add regex check to error expectation #10038
Conversation
Creating a buffer from a number should throw an error with a message that describes the issue.
@samshull just so you know you can force push over branches on github to update a PR. This will allow you to make changes to a commit without needing to open a new PR |
@thealphanerd, yes, I understand force push, I created a second branch because I had multiple PRs that could not be submitted from the same branch and could not create a branch off a branch because of the fork requirements of github. Edit: I had two PRs, but different code in each. |
@@ -8,7 +8,7 @@ assert.doesNotThrow(function() { | |||
|
|||
assert.throws(function() { | |||
Buffer.from(10, 'hex'); | |||
}); | |||
}, /argument must not be a number/); |
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.
It would be better to test the whole error message:
/^TypeError: "value" argument must not be a number$/
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.
What is the benefit of full text message checking?
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.
@samshull it helps tracking changes in the error message/type as those changes are breaking changes (semver-major).
@jasnell change submitted |
@jasnell It appears that your comment has been addressed. Can you take a look and, if so, please update your review so we can land this? Thanks! |
Landed bae695f |
Creating a buffer from a number should throw an error with a message that describes the issue. PR-URL: #10038 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Creating a buffer from a number should throw an error with a message that describes the issue. PR-URL: #10038 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Creating a buffer from a number should throw an error with a message that describes the issue. PR-URL: #10038 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Creating a buffer from a number should throw an error with a message that describes the issue. PR-URL: #10038 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Checklist
make -j8 test
(UNIX), orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
test
Description of change
Creating a buffer from a number should throw an error with a message
that describes the issue.