Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make negated `.keys` consider size of sets #924

Merged
merged 1 commit into from Mar 23, 2017

Conversation

@meeber
Copy link
Contributor

meeber commented Jan 30, 2017

When neither the contains nor any flags are set, the .keys
assertion implicitly means .all.keys and thus requires the target
and given sets to be the same size. Therefore, not.keys implicitly
means .not.all.keys, and should thus pass when the target and given
sets aren't the same size, even if the target set is a superset of the
given set. This commit enables this behavior.

Fixes #919

When neither the `contains` nor `any` flags are set, the `.keys`
assertion implicitly means `.all.keys` and thus requires the target
and given sets to be the same size. Therefore, `not.keys` implicitly
means `.not.all.keys`, and should thus pass when the target and given
sets aren't the same size, even if the target set is a superset of the
given set. This commit enables this behavior.
@lucasfcosta
Copy link
Member

lucasfcosta commented Feb 2, 2017

LGTM!

Seems more straightforward this way. Good job!

@meeber
Copy link
Contributor Author

meeber commented Mar 22, 2017

This is a small one. Pinging @keithamus @vieiralucas @shvaikalesh

Copy link
Member

vieiralucas left a comment

LGTM

@keithamus
Copy link
Member

keithamus commented Mar 23, 2017

Okay, happy enough with this given the context.

@keithamus keithamus merged commit 011612c into chaijs:master Mar 23, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@meeber meeber deleted the meeber:fix-not-have-keys branch Aug 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.