-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Do not hard code err instanceof Error checks #1627
Comments
Good idea! We wouldn't be able to use lodash's implementation out of the box because of how pending tests are defined and propagated up to the runner/runnables. https://github.com/mochajs/mocha/blob/master/lib/pending.js If I recall correctly, there's cases where the instance of Pending will have an undefined message. That logic will need to be tweaked a bit. |
Just as another reference, in other parts of Mocha, specifically here: https://github.com/mochajs/mocha/blob/master/lib/runnable.js#L234 Mocha already toStrings() the object as a fallback to the |
See #1758 for related PR. |
@brian-mann, does that pr solve what you're looking for? The object toString method that you mentioned doesn't work because it can still return something other than @danielstjules I don't think checking for error message is a problem here. Pending doesn't appear to be an instance of Error, so it should continue to work as implemented if a Pending object does indeed reach this. |
@boneskull I think we can remove the pr-please label at this point. |
Good point :) Not sure what I was thinking |
Closed by #1758 |
Specifically this line: https://github.com/mochajs/mocha/blob/master/lib/runner.js#L205
Long story short,
err
may be a validError
instance but in situations such asiframes
eachwindow
will have its ownError
object, and this will therefore always always fail.I would suggest taking after what lodash does in its
isError
function.It just ensures its "object like", has a string for message property, and is [object Error]
The text was updated successfully, but these errors were encountered: