Skip to content
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

Provide friendly error message when deep equality check fails because object has property value of undefined #304

Closed
OliverJAsh opened this issue Nov 12, 2014 · 4 comments

Comments

@OliverJAsh
Copy link

expect({ foo: 'bar', baz: { asda: undefined } }).to.eql({ foo: 'bar', baz: {} });
AssertionError: expected { Object (foo, baz) } to deeply equal { foo: 'bar', baz: {} }

That's not helpful. This can be fixed by setting chai.config.truncateThreshold = 0;.

AssertionError: expected { foo: 'bar', baz: { asda: undefined } } to deeply equal { foo: 'bar', baz: {} }

Would it be better if we showed a diff to the user? This way, the error is clear to the user immediately (no Chai config required):

      AssertionError: expected { Object (foo, baz) } to deeply equal { foo: 'bar', baz: {} }
      + expected - actual

       {
         "foo": "bar"
      +  "baz": {
      +    "asda": undefined
      +  }
      -  "baz": {}
       }
@keithamus
Copy link
Member

Thanks @OliverJAsh for the issue. Seems like it is semi related to your issue over in #246? I'd like to see more discussion on this one before prompting for a PR. Maybe @logicalparadox or @vesln have some thoughts?

@ahamid
Copy link

ahamid commented Sep 24, 2015

I have easily lost hours debugging the difference between two objects that appear identical (for some reason Mocha also hangs in this case so it's hard to tell which test is even failing). It would be great if this discrepancy is somehow indicated. Is there a significant semantic difference between an object with a property holding an undefined value, and an object without that property?

@keithamus
Copy link
Member

Thanks for your comments @ahamid, and bringing light to this (quite old) issue. A couple of points on this:

  • Diffing is the responsibility of the assertion library, because it's reporter based. For example a CLI reporter might use color escape codes while an HTML reporter uses HTML tags. Currently Mocha's HTML reporter doesn't show diffs, see No diff in html report mochajs/mocha#1348 for more.
  • The single line error message (AssertionError: expected...) uses loupe (well, it actually doesn't just yet, but we're working on it) to pretty print objects. Without loupe the message would say something like AssertionError: expected [object Object] to equal [object Object]. Loupe converts the [object Object] bits into useful strings about an object - but it is truncated to make sure that the error message is still reasonably a single liner.
  • Loupe has no diffing logic, because it only inspects one object at a time; one separate call for expected, and one for actual.
  • Loupe's truncation logic can be controlled with chai.config.truncateThreshold - this defines how "shallow" or "deep" the pretty-printed value is coming out from loupe; this can be helpful in noticing object discrepancies. Currently if you get stuck, running chai.config.truncateThreshold = Infinity would be a quick way to dive deeper.
  • The real problem here is that loupe doesn't have diffing logic, because it is passed one object at a time.
  • The real solution here would be to provide loupe with enough data that it could point out the discrepancies in an object easier, for example instead of AssertionError: expected { Object (foo, baz) } to deeply equal { foo: 'bar', baz: {} } we could have something like AssertionError: expected { baz: { asda: undefined }... } to deeply equal { baz: {}... }.

If we can get Loupe doing more interesting inspection, by passing it comparison objects, then this would solve this problem without relying on the test runner's diff output, and provide much better error messages.

If anyone wants to take a stab at this I'm happy to work with/mentor them. It'd be reasonably complex, probably involve quite a bit of logic (effectively a diffing algo) but it'd be a huge improvement to error messages 😄

@keithamus
Copy link
Member

Hey @ahamid thanks for bringing this to my attention again. This issue has been raised a few times. I've raised an issue within the loupe project that is similar to this one (chaijs/loupe#1) and better describes the problem. This is also kind of a dupe of #469 so I'm going to close this one.

I'm closing this issue in favour of chaijs/loupe#1 - which will really help us out in part, but also one of things problematic to us is that Mocha's diffing is not the best it could be. mochajs/mocha#1348 exists as part of this, but more investment needs to be made here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants