expect(obj).to.have.all.keys Should List Failing Keys

[machineghost]

First off, it's possible that I'm not understanding keys properly, and if so I apologize. But here's my issue ...

If I do the following:

expect({ foo: 1 }).to.have.all.keys('foo');

it passes; however if I do:

expect({ foo: 1, bar: 1 }).to.have.all.keys('foo');

it fails. I guess this makes sense, although it's confusing as the English interpretation would suggest the above asserts that "the object has all keys" (ie. it has the key foo, since that was "all" of the keys I provided).

What it's really doing is "the object must only have the provided keys (ie.foo)", and as such it would make a lot more sense to have:

expect({ foo: 1, bar: 1 }).to.have.only.keys('foo');

Now, if you fix that, my bug doesn't need to exist, but since I suspect you won't make such a major change it'd be nice if Chai could at least give helpful error output in this situation. Currently it reports:

AssertionError: expected { foo: 1, bar: 1 } to have key 'foo'

But that error is wrong: my object did have a foo; what the error is really trying to tell me is:

AssertionError: expected { foo: 1, bar: 1 } to only have key(s) 'foo', but had key(s) 'bar'

I would imagine (unlike changing .all to .only) this would be a relatively easy fix, and I would similarly imagine that all users of the library would benefit from an improved error message.

Thanks for the consideration.

[meeber]

Yup right now it kinda sucks. See #919 (comment) and #881 for more thoughts on this issue and related. Once we get past the 4.0 release I wanna take a closer look at this. Better error message is definitely an option if it's too destructive to fix the core issue.

[machineghost]

Thanks for the reply (I strongly suspected "we can't break things to fix this" was going to be the issue).

One note on the better error message part: currently if you eschew all and do:

expect(foo).to.contain.keys('foo', 'bar', 'baz', 'qux');

You get a failure message of:

 expect *json representation* to contain keys 'foo', 'bar', 'baz', and 'qux'

again, a really simple (I suspect we're talking < half an hour) fix could make that output:

 expect *json representation* to contain keys 'foo', 'bar', 'baz', and 'qux' but the keys 'foo' and 'qux' were not found

which would be far more helpful to the user. Just a suggestion (I would offer to submit a PR, but unfortunately I know nothing about Chai's underpinnings).

P.S. Sorry just noticed one more thing: keys doesn't appear to handle keys on the __proto__ of the provided object. There might be a technical reason for this, but if not it'd be really great to have some way to also match prototype-inherited keys, eg.

expect(foo).andPrototypeChain.to.contain.keys('foo', 'bar', 'baz', 'qux');

Sorry if this is disjointed; if it would help I can refile as three issues (have=>only, better output when have.keys or contains.keys fails, and checking prototype-inherited keys).

[vieiralucas]

@machineghost Thanks for you issue.

About your problem with keys on the __proto__
keys does not handle properties that come from the prototype because it is build on top of Object.keys

You can overcome that by using the property assertion.

function Foo() {
}

Foo.prototype.bar = 'baz'

const f = new Foo()

expect(f).to.have.keys('bar') // this fails
expect(f).to.have.property('bar') // this works

[keithamus]

Hey @machineghost thanks for this issue!

Thank you for your time in reporting this. During the upcoming chai 5 refactor we're going to be looking at error messages a bunch and I'm sure we'll be implicitly fixing this as part of that. I'm going to close this, because there is nothing for us to do right now, but rest assured this has been noted, and we'll be making sure to keep this in mind in the future.

[sebastianhaberey]

+1

I agree that the .to.have.all.keys notation is not very intuitive and .to.have.only.keys would be significantly more readable and make it easier for everyone not intimately familiar with chai syntax.