-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add string support to throws #125
Conversation
|
It's not. We only use the core assert so not to duplicate code. If anything can be done better, we'll do it. Also the core Node.js people have said 👍 from me. |
One of my problems with the Node team. /offtopic |
Things can always be done better! |
@Qix- In their defense it was a mistake to expose it as it was only really meant to be used to test core, from what I've read. Also why ideally almost everything should be in user-land so it can be individually versioned and evolved. |
That makes sense. |
}, msg); | ||
} else { | ||
assert.throws(fn, err, msg); | ||
} |
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.
if (typeof err === 'string') {
err = function (e) {
return e.message === err;
};
}
assert.throws(fn, err, msg);
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.
Definitely cleaner, but then I guess it should be like this.
if (typeof err === 'string') {
var e = err;
err = function (err) {
return err.message === e;
};
}
assert.throws(fn, err, msg);
In your example, at the time the function is executed, err
is not the string anymore but it is the function. So it will check if err.message
is equal to the function instead of to the original string.
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.
Yup
👍 Thanks Sam :) |
No problem, always happy to contribute ;). Any idea when these latest changes ( |
@SamVerschueren Hopefully today. Just want to get in the two open PRs. Any help reviewing #120 welcome ;) |
That's not an easy one :). Will have a look at it later today. |
I added the possibility to pass in the error message as string to the
throws
method.Example
Benefit
If you want to test the returned error message you either should use a regex or a validation function. The validation function spoils the test file in my opinion, and the regex is, well yeah, it's a regex. People have to escape some characters and a string just looks cleaner.
Downside
Not sure if it is a downside, but it's not standard
assert
stuff which might confuse people. Although, I don't really see it as downside, it's more an improvement.// @sindresorhus @vdemedes @kevva @Qix-