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

Issue24 eachall keys #35

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

drzaal
Copy link

@drzaal drzaal commented Apr 6, 2016

This should resolve the issues with assertion 'keys'
Introduce the more syntactically correct 'each' decorator (synonym with all)
and operate on enumerable objects now

@drzaal drzaal mentioned this pull request Apr 6, 2016
@drzaal
Copy link
Author

drzaal commented Apr 6, 2016

I regret not splitting this into three PRs, but! Here we are.

emptyObject = {}

it "should trivially all equal 1", ->
emptyObject.should.all.equal(1)
Copy link
Member

Choose a reason for hiding this comment

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

This seems a bit broken. I would think that an empty object should fail at .all as it has no keys.

Copy link
Author

Choose a reason for hiding this comment

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

While I would agree...

It is currently the default behaviour, see master at test/all.coffee does:

describe "an empty array's elements", ->
  emptyArray = []

  it "should trivially all equal 1", ->
    emptyArray.should.all.equal(1)

I considered it a breaking change to modify this behaviour.

Copy link
Member

Choose a reason for hiding this comment

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

Okay fair enough. We'll change this in a breaking release later on then 😄

Choose a reason for hiding this comment

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

Just as a note, all should pass if the array is empty.

If it was to fail, then it's negation would have to be true. The negation of all is 'some are not', so an empty array would have to return true if some of its elements are 'not'. However, you cannot find an element in the empty set that is 'not; so it should be false; this is definitely not be behavior you want in a some statement.

Copy link
Author

Choose a reason for hiding this comment

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

james, were you recommending that the some statements were not behaving as expected?

Choose a reason for hiding this comment

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

No, I'm confirming that an empty array should return true if you check to see if all of its elements are equal to 1. This is logic 101.

@keithamus
Copy link
Member

Hey @drzaal awesome work 😄. I've added some comments above, could you please address them?

@drzaal
Copy link
Author

drzaal commented Apr 8, 2016

So, this issue I was dealing with was interoperability with the latest version of chai.

Everything went tops up when I tried running chai 3.5.

I've now got it testing reasonably successfully in both versions 1.4 and 3.5

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.

3 participants