Fixes issue #291, adds assert.sameDeepMembers #312

Merged
merged 2 commits into from Dec 3, 2014

Projects

None yet

3 participants

@cjqed
Contributor
cjqed commented Nov 29, 2014

This should fix #291 and adds the sameDeepMembers function that expect and should already have. If there are any issues please let me know.

Thanks!

@keithamus keithamus commented on an outdated diff Nov 30, 2014
lib/chai/interface/assert.js
@@ -1017,6 +1017,25 @@ module.exports = function (chai, util) {
}
/**
+ * ### .sameDeepMembers(set1, set2, [message])
+ *
+ * Asserts that `set1` and `set2` have deeply the same members.
@keithamus
keithamus Nov 30, 2014 Member

The wording ("have deeply the same members") is a bit funny here. May I suggest "have the same deep members", or "have the same members - using a deep equality check".

@cjqed
Contributor
cjqed commented Nov 30, 2014

Pushed the comment change, I went with "have the same members - using a deep equality check" since it makes it very explicit.

@keithamus
Member

Looks great to me! Thanks for your efforts @cjqed. I'll wait for @logicalparadox to give the ๐Ÿ‘ and merge then ๐Ÿ˜„.

@cjqed
Contributor
cjqed commented Dec 1, 2014

Sounds good @keithamus . I'm probably going to tackle a few more issues that are low hanging fruits and maybe even some tougher ones, I'm just in that kind of mood (and honestly Chai has a very nice codebase!)

@keithamus
Member

@vesln or @logicalparadox, you happy to see this merged in? If so I'll hit the big green button.

@logicalparadox
Member

๐Ÿ‘

@keithamus
Member

Oof, sorry @cjqed - one of the PRs I just merged conflicts with this one. If you could rebase this branch then I can merge it in. Should be an easy fix - some new tests were added and its just conflicting with your diff - make sure to keep both sets of tests.

Thanks for your patience @cjqed :)

@cjqed
Contributor
cjqed commented Dec 3, 2014

This should be ready to merge, I'll work on #313 and try to make it so that it can be automerged as well.

@keithamus
Member

โญ๏ธ

@keithamus keithamus merged commit f06278f into chaijs:master Dec 3, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@keithamus keithamus referenced this pull request Feb 12, 2015
Closed

Get a proper changelog #366

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