isEqual behavior given two Errors is inconsistent with documentation

[fvgs]

Do you want to request a feature or report a bug?

Bug (in either the codebase or the documentation)

What is the current behavior?

const a = new Error ('foo')
const b = new Error ('foo')
// This succeeds
expect (a) . toEqual (b)

What is the expected behavior?

https://facebook.github.io/jest/docs/en/expect.html#toequalvalue

According to the documentation, toEqual should perform a "deep equal". However, the implementation is inconsistent with the documentation when the the two objects are Errors:

https://github.com/facebook/jest/blob/master/packages/jest-matchers/src/jasmine-utils.js#L72

if (a instanceof Error && b instanceof Error) {
  return a.message == b.message;
}

With the current implementation, toEqual will only check the message properties of the two Errors for equality instead of checking all properties of the two objects recursively. This is inconsistent with the documentation which does not mention this special behavior. Either the documentation should be updated or the implementation should be fixed.

On one hand, keeping the current implementation may be convenient because, more often than not when testing for an error, one is really just concerned with equality of the error messages as opposed to equality of other properties such as Error.stack. That said, the current behavior illustrated above can already be achieved more directly by comparing the Error.message strings. The current behavior also has the downside of being unexpected (even if it were documented) and ultimately being less flexible and limiting functionality. For instance, if one's goal were to compare two or more errors to see if they are the same error and occurred on the same line, the current implementation of toEqual might unexpectedly give a false positive because it only checks the messages for equality.

I believe the better implementation is the one which offers more flexibility, does not limit functionality, and is consistent with the behavior expected by someone with a reasonable understanding of how an Error works.

If we consider the API as defined by the documentation, such a fix would be a patch update. Though, given the nuance involved, fixing this behavior would more likely merit a major update.

Jest Version
Jest 20.0.0

[thymikee]

Would you like to improve the docs for that? :)
Until the next major release we can discuss if we'd like to change that behavior.

[fvgs]

@cpojer @thymikee reopen for discussion?

[thymikee]

What are your concerns?

[fvgs]

@thymikee The concern is treating Errors as a special case when using isEqual to me seems like unexpected behavior. It also limits functionality for someone who may want to rely on the "regular" definition of deep equality.

It's simple(r) enough to compare the message strings if that is the behavior that is desired. That's my subjective opinion. I opened this issue because it's something I found unexpected and had to look into the first time I compared Errors for equality (the behavior was undocumented at the time).