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
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 35 additions & 16 deletions lib/chai-things.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,19 @@

// The assertion's object for `something` should be array-like
var object = utils.flag(this, "object");
expect(object).to.have.property("length");
expect(object.length).to.be.a("number", "something object length");

// The object should contain something
this.assert(object.length > 0,
"expected #{this} to contain something",
"expected #{this} not to contain something"
);
}
if (object.hasOwnProperty("length")) {
expect(object.length).to.be.a("number", "something object length");
this.assert(object.length,
"expected #{this} to contain something");
} else {
expect(object).is.an("object", "since length property unavailable");
this.assert(Object.keys(object).length,
"expected #{this} to contain something");
}

// The object should contain something

}

// Handles the `all` chain property
function chainAll() {
Expand Down Expand Up @@ -100,8 +104,15 @@

// The assertion's object for `something` should be array-like
var arrayObject = utils.flag(this, "object");
expect(arrayObject).to.have.property("length");
var length = arrayObject.length;

var length;
if (!arrayObject.hasOwnProperty("length")) {
expect(arrayObject).is.an("object", "since length property unavailable");
length = Object.keys(arrayObject).length;
} else {
length = arrayObject.length;
}

expect(length).to.be.a("number", "something object length");

// In the negative case, an empty array means success
Expand All @@ -114,7 +125,7 @@

// Try the assertion on every array element
var assertionError;
for (var i = 0; i < length; i++) {
for (var i in arrayObject) {
// Test whether the element satisfies the assertion
var item = arrayObject[i],
itemAssertion = copyAssertion(this, item, somethingAssert);
Expand Down Expand Up @@ -161,25 +172,32 @@
return function allMethod() {
// Return if no `all` has been used in the chain
var allFlags = utils.flag(this, "all");
if (!allFlags)
if (!allFlags || methodName === "keys")
return _super.apply(this, arguments);
// Use the nested `all` flag as the new `all` flag for this.
utils.flag(this, "all", utils.flag(allFlags, "all"));

// The assertion's object for `all` should be array-like
var arrayObject = utils.flag(this, "object");
expect(arrayObject).to.have.property("length");
var length = arrayObject.length;
var length;
if (!arrayObject.hasOwnProperty("length")) {
expect(arrayObject).is.an("object", "since length property unavailable");
length = Object.keys(arrayObject).length;
} else {
expect(arrayObject).to.have.property("length");
length = arrayObject.length;
}
expect(length).to.be.a("number", "all object length");


// In the positive case, an empty array means success
var negate = utils.flag(allFlags, "negate");
if (!negate && !length)
return;

// Try the assertion on every array element
var assertionError, item, itemAssertion;
for (var i = 0; i < length; i++) {
for (var i in arrayObject) {
// Test whether the element satisfies the assertion
item = arrayObject[i];
itemAssertion = copyAssertion(this, item, allAssert);
Expand Down Expand Up @@ -240,4 +258,5 @@
});
// Define the `all` chainable assertion method
Assertion.addChainableMethod("all", null, chainAll);
Assertion.addChainableMethod("each", null, chainAll);
}));
54 changes: 46 additions & 8 deletions test/all.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,14 @@
describe "using all()", ->

describe "an object without length", ->
nonLengthObject = {}
nonLengthObject = 1

it "fails to have all elements equal to 1", ->
(() -> nonLengthObject.should.all.equal 1).
should.throw "expected {} to have a property 'length'"

it "fails not to have all elements equal to 1", ->
it "fails to have all elements equal to assertion", ->
(() -> nonLengthObject.should.all.equal 1).
should.throw "expected {} to have a property 'length'"

should.throw "since length property unavailable: expected 1 to be an object"
it "fails not to have all elements equal to assertion", ->
(() -> nonLengthObject.should.not.all.equal 1).
should.throw "since length property unavailable: expected 1 to be an object"

describe "an object without numeric length", ->
nonNumLengthObject = { length: 'a' }
Expand All @@ -35,6 +33,15 @@ describe "an empty array's elements", ->
it "should trivially all not equal 1", ->
emptyArray.should.all.not.equal(1)

describe "an empty object's elements", ->
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.


it "should trivially all not equal 1", ->
emptyObject.should.all.not.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.

Again, this should fail before asserting equality on any keys. It's empty.



describe "the array [1, 1]'s elements", ->
array = [1, 1]
Expand Down Expand Up @@ -67,6 +74,37 @@ describe "the array [1, 1]'s elements", ->
(() -> array.should.not.all.not.equal 2).
should.throw "expected not all elements of [ 1, 1 ] to not equal 2"

describe "the object {first: 1, second: 1}'s elements", ->
obj = [1, 1]
Copy link
Member

Choose a reason for hiding this comment

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

Is this meant to be an object literal?

Copy link
Author

Choose a reason for hiding this comment

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

dang, yes


it "should all equal 1", ->
obj.should.all.equal 1

it "should all not equal 2", ->
obj.should.all.not.equal 2

it "should not all equal 2", ->
obj.should.not.all.equal 2

it "should not all not equal 1", ->
obj.should.not.all.not.equal 1

it "do not all equal 2", ->
(() -> obj.should.all.equal 2).
should.throw "expected all elements of [ 1, 1 ] to equal 2"

it "do not all *not* equal 1", ->
(() -> obj.should.all.not.equal 1).
should.throw "expected all elements of [ 1, 1 ] to not equal 1"

it "do not *not* all equal 1", ->
(() -> obj.should.not.all.equal 1).
should.throw "expected not all elements of [ 1, 1 ] to equal 1"

it "do not *not* all not equal 2", ->
(() -> obj.should.not.all.not.equal 2).
should.throw "expected not all elements of [ 1, 1 ] to not equal 2"


describe "the array [1, 2]'s elements", ->
array = [1, 2]
Expand Down
51 changes: 46 additions & 5 deletions test/something.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,23 @@ describe "using something()", ->


describe "an object without length", ->
nonLengthObject = {}
nonLengthObject = 1

it "fails to include something", ->
(() -> nonLengthObject.should.include.something()).
should.throw "expected {} to have a property 'length'"
should.throw "since length property unavailable: expected 1 to be an object"

it "fails not to include something", ->
(() -> nonLengthObject.should.not.include.something()).
should.throw "expected {} to have a property 'length'"
should.throw "since length property unavailable: expected 1 to be an object"

it "fails to include something that equals 1", ->
(() -> nonLengthObject.should.include.something.that.equals 1).
should.throw "expected {} to have a property 'length'"
should.throw "since length property unavailable: expected 1 to be an object"

it "fails not to include something that equals 1", ->
(() -> nonLengthObject.should.not.include.something.that.equals 1).
should.throw "expected {} to have a property 'length'"
should.throw "since length property unavailable: expected 1 to be an object"


describe "an object without numeric length", ->
Expand Down Expand Up @@ -115,6 +115,47 @@ describe "the array [{ a: 1 }, { b: 2 }]", ->
it "should not include something with a property b of 3", ->
array.should.not.include.something.with.property('b', 3)

describe "the object {primary: { a: 1 }, secondary: { b: 2 }}", ->
obj = { primary: { a: 1 }, secondary: { b: 2 }}

it "should include something", ->
obj.should.include.something()

it "does not *not* include something", ->
(() -> obj.should.not.include.something()).
should.throw "expected [ { a: 1 }, { b: 2 } ] not to contain something"

it "should include something that deep equals { b: 2 }", ->
obj.should.include.something.that.deep.equals { b: 2 }

it "should include something that not deep equals { b: 2 }", ->
obj.should.include.something.that.not.deep.equals { b: 2 }

it "does not include something that deep equals { c: 3 }", ->
(() -> obj.should.include.something.that.deep.equals { c: 3 }).
should.throw "expected an element of { Object (primary, secondary) } to deeply equal { c: 3 }"

it "should not include something that deep equals { c : 3 }", ->
obj.should.not.include.something.that.deep.equals { c: 3 }

it "should include something that not deep equals { c: 3 }", ->
obj.should.include.something.that.not.deep.equals { c: 3 }

it "does not *not* include something that deep equals { b: 2 }", ->
(() -> obj.should.not.include.something.that.deep.equals { b: 2 }).
should.throw "expected no element of { Object (primary, secondary) } to deeply equal { b: 2 }"

it "should include something with a property b of 2", ->
obj.should.include.something.with.property('b', 2)

it "does not include something with a property b of 3", ->
(() -> obj.should.include.something.with.property('b', 3)).
should.throw "expected an element of { Object (primary, secondary) }" +
" to have a property 'b' of 3, but got 2"

it "should not include something with a property b of 3", ->
obj.should.not.include.something.with.property('b', 3)


describe "the array [{ a: 1 }, { a: 1 }]", ->
array = [{ a: 1 }, { a: 1 }]
Expand Down