Provide friendly error message when deep equality check fails because of types #246

Open
OliverJAsh opened this Issue Feb 16, 2014 · 11 comments

Comments

Projects
None yet
7 participants
function Foo() {}
var foo = new Foo();
expect(foo).to.eql({});

This fails because the objects are not the same type, but the error message doesn’t tell me this:

+ expected - actual

alidd commented Apr 19, 2014

+1

looks like it requires incompatible interface change of deepEqual in deep-eql:

  • return someting like {isEqual: true/false, firstDifference: string?} instead of boolean, or
  • introduce new optional argument "options" of type "object" which could have properties
    like "comparisonDepth", "ignoreTypeDifferece", "returnFirstDifference" etc. and be used to
    return info about differeces, or
  • introduce new optional argument "cb" of type "function" to accept additional info about differeces?

huafu commented Sep 22, 2014

+1, but actually it needs some better diff even if the types are the same:

expect({k: undefined}).to.deep.equal({})

will output the same diff:

+ expected - actual

It's ok for short objects because then you'll see the full list of properties, but for long object it'll list the first properties and then ... so it's hard to know what's happening, just spent 3 hours debugging to find out what was the error lol

Owner

logicalparadox commented Sep 22, 2014

@huafu You can disable the ... truncation of objects through Chai's config object. It will then display the whole object in the error message.

chai.config.truncateThreshold = 0; // disable truncating

This should give you a bit more information for where to look for mysterious mismatched deep equal objects.

As for the visual diff of + expected - actual, that is provided by your test runner. Chai's deep equal test utility operates on a different set of principles than Mocha's (or any other runners) diff display which presents its own set of challenges (and confusion).

huafu commented Sep 22, 2014

@logicalparadox thanks but what I actually would like is to have then a diff showing me that in one side I have a k property set to undefined and the other side I don't have that property on the object.

But anyway, using chai.config.truncateThreshold is a good temporary solution, thanks again1

Owner

keithamus commented Nov 26, 2014

Something could be done about this to make it a little less painful.

@huafu since you commented on the issue, do you have any suggestions that could be done to help highlight these errors more?

huafu commented Nov 26, 2014

well, I guess it could be shown like this:

+someProperty: undefined
-someProperty: --missing--

@huafu There is a separate issue for the undefined property diff issue: #304

If the equality match fails because of the different types, we could log:

expected obj to have type foo, actual type bar

You can type the type like this:

function bar(){}
new bar().constructor.name;
// => "bar"

huafu commented Nov 26, 2014

This is not related to the type, in this case actually they are bare objects. But on any object you could have a property NOT defined on the prototype while you want to expect that this property would be set to undefined on an instance.

UPDATE: ok, I re-read the thread and your message makes more sense @OliverJAsh, my apologizes ;-)

blimmer commented Mar 27, 2015

chai.config.truncateThreshold is very helpful! So nice to see what's different in a deep.equals visually instead of debugging in the browser.

machineghost commented Dec 23, 2016

It's easy to miss chai.config.truncateThreshold . While it is documented on http://chaijs.com/guide/styles/, that page is basically a "read once when you're first learning Chai" page (after that you think "I already understand the whole assert vs. should vs. expect thing" and don't come back). It'd be nice if you could add it somewhere else in the documentation (eg. mention it in the docs for deep.equal) or if you could rename the page to "Assertion Styles and Configuration Options".

Of course, if chai.config.truncateThreshold could default to 0 that'd be even better, but I imagine there's some reason why it isn't already set that way.

Owner

keithamus commented Dec 27, 2016

Ideally truncatethreshold should be deprecated in favour of doing the sensible thing instead. I'd like to not bring it more to the forefront, and instead fix underlying issues

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