chai icon indicating copy to clipboard operation
chai copied to clipboard

fix: accept empty keys when actual object is empty

Open Zirak opened this issue 4 years ago • 1 comments

When comparing object keys, the user may happen upon comparing two empty objects. Before this commit, this would be an error:

// before
expect({}).to.have.all.keys({});
AssertionError: keys required

This commit makes it legal for keys to accept no keys when the actual object is itself empty.

I happened upon this when generating values in property-based testing. Some tests had object comparisons as the above, but failed for the empty input, forcing a dance like this:

if (Object.keys(right).length === 0) {
    expect(left).to.be.an('object').that.is.empty;
} else {
    expect(left).to.have.all.keys(right);
}

There is a negative side to this PR to consider: keys will change its behaviour depending on the object being inspected. That may be fine or it may be not, depending on the project's goals.

Another thing to consider is how now there are "two" ways to check if an object is empty:

  1. expect({}).to.be.empty
  2. expect({}).to.have.all.keys({})

That, I feel, is less of a problem, as there seem to be multiple ways of achieving the same result elsewhere in the library.

Thanks for your time.

Zirak avatar Apr 06 '21 17:04 Zirak

There's something here that I carelessly slipped by. In current chai, assert.hasAllKeys({}) currently throws:

> require('chai').assert.hasAllKeys({})
Uncaught [AssertionError: expected {} to have key 'undefined'] {
  showDiff: true,
  actual: [],
  expected: [ 'undefined' ]
}

In current chai, it could be expected to fail like its siblings, with a keys required message. This is something that can be handled in this commit, or silently accepted and fixed in another PR.

Zirak avatar Apr 06 '21 17:04 Zirak