debitoor / chai-subset

"containSubset" object properties matcher for Chai
http://chaijs.com/plugins/chai-subset/
MIT License
82 stars 20 forks source link

containSubset should fail for dates that are not equal #21

Closed sgilroy closed 8 years ago

sgilroy commented 8 years ago

The way that objects are evaluated for equality fails for dates, because Object.keys(new Date()) returns an empty array (a Date has no keys).

Test to demonstrate the failure:

describe('date', function() {
    var testedObject = new Date('2015-11-30');
    it('should pass for the same date', function() {
        expect(testedObject).to.containSubset(new Date('2015-11-30'));
    });

    it('should fail for a different date', function() {
        expect(testedObject).to.not.containSubset(new Date('2012-02-22'));
    });
});
eagleeye commented 8 years ago

Yup, I've put your tests in PR #22 , will try to fix that in next few days.

eagleeye commented 8 years ago

@sgilroy please review) #22

sgilroy commented 8 years ago

Looks like an effective solution to me, though you may also want to consider revising the way to you test for equality to resolve other potential failures. The algorithm behind isEqual in lodash might be useful. Perhaps you could use _.isEqual() for cases where an expected value is an object that has no keys. You may want to also (or instead) update the documentation to explain how the containsSubset test is implemented, and how it could fail.

ebdrup commented 8 years ago

I like the solution, not a fan of _.isEqual. I personally prefer the simple, self-contained solution.

eagleeye commented 8 years ago

fixed in #22