-
-
Notifications
You must be signed in to change notification settings - Fork 306
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 a Function.constructor check to throws. #130
Conversation
@wilmoore Please use the |
@Raynos - thanks; good call. I've updated the guard to use |
@@ -417,6 +418,11 @@ Test.prototype['throws'] = function (fn, expected, msg, extra) { | |||
expected = String(expected); | |||
} | |||
|
|||
if (expected instanceof Function) { | |||
passed = caught.error.constructor === expected; |
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.
caught.error instanceof expected
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.
caught.error instanceof expected
Are we 100% sure that we want to allow lose inheritance-based matching here? For example, if a TypeError
is thrown, if I assert the type is Error
, that would pass.
I can see cases where I'd want either of those behaviors (though, for my current use-case, I need the strict test). Perhaps we should consider a completely separate assertion for one or the other.
What do you think @Raynos?
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.
@wilmoore that's the semantics of require('assert')
in node core on purpose.
Having different semantics from require('assert')
is confusing and not worth it, even if they are "technically" better.
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.
Having different semantics from require('assert') is confusing and not worth it...
Indeed. I agree with that. Update coming.
lgtm :) |
Add a Function.constructor check to throws.
Allows you to also write: