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 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

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 vieiralucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@keithamus
Copy link
Member

Okay, happy enough with this given the context.

@keithamus keithamus merged commit 011612c into chaijs:master Mar 23, 2017
@meeber meeber deleted the fix-not-have-keys branch August 6, 2017 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants