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.

Sign up for GitHub

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

[WIP] Verify types on within and related assertions #692

Merged
merged 2 commits into from
May 4, 2016
Merged

[WIP] Verify types on within and related assertions #692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
78c5763
Verify types on within and related assertions
vieiralucas Apr 30, 2016
de86590
Documentation update to indicate type requirements
vieiralucas May 1, 2016
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
105 changes: 85 additions & 20 deletions lib/chai/core/assertions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,14 +504,15 @@ module.exports = function (chai, _) {
/**
* ### .above(value)
*
* Asserts that the target is greater than `value`.
* Asserts that the number target is greater than `value`.
Copy link
Member

Choose a reason for hiding this comment

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

Just being nitpicky, but this should be:

Asserts that the target number is greater than value

*
* expect(10).to.be.above(5);
*
* Can also be used in conjunction with `length` to
* assert a minimum length. The benefit being a
* more informative error message than if the length
* was supplied directly.
* was supplied directly. In this case the target must
* have a length property.
*
* expect('foo').to.have.length.above(2);
* expect([ 1, 2, 3 ]).to.have.length.above(2);
Expand All @@ -521,14 +522,26 @@ module.exports = function (chai, _) {
* @alias greaterThan
* @param {Number} value
* @param {String} message _optional_
* @namespace BDD
* @api public
*/

function assertAbove (n, msg) {
if (msg) flag(this, 'message', msg);
var obj = flag(this, 'object');
if (flag(this, 'doLength')) {
var obj = flag(this, 'object')
, doLength = flag(this, 'doLength');

if (doLength) {
new Assertion(obj, msg).to.have.property('length');
} else {
new Assertion(obj, msg).is.a('number');
}

if (_.type(n) !== 'number') {
throw new Error('the argument to above must be a number');
Copy link
Member

Choose a reason for hiding this comment

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

@meeber I'm not sure this sentence is clear enough, however, I'm not a native english speaker, would you mind suggesting a new message for this?
What do you think about: The .above assertion must receive a Number as argument?

Copy link
Member Author

@vieiralucas vieiralucas May 1, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I copied the idea from here

Copy link
Member

Choose a reason for hiding this comment

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

While this isn't the best message, we can leave it for now as it mimicks the others.

Copy link
Member

Choose a reason for hiding this comment

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

I'm kinda extremely late to the party, but could these make use of utils.expectTypes? Same above at L537.

}

if (doLength) {
var len = obj.length;
this.assert(
len > n
Expand All @@ -553,14 +566,15 @@ module.exports = function (chai, _) {
/**
* ### .least(value)
*
* Asserts that the target is greater than or equal to `value`.
* Asserts that the number target is greater than or equal to `value`.
Copy link
Member

Choose a reason for hiding this comment

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

"target number" here would be better than "number target"

*
* expect(10).to.be.at.least(10);
*
* Can also be used in conjunction with `length` to
* assert a minimum length. The benefit being a
* more informative error message than if the length
* was supplied directly.
* was supplied directly. In this case the target must
* have a length property.
*
* expect('foo').to.have.length.of.at.least(2);
* expect([ 1, 2, 3 ]).to.have.length.of.at.least(3);
Expand All @@ -569,14 +583,26 @@ module.exports = function (chai, _) {
* @alias gte
* @param {Number} value
* @param {String} message _optional_
* @namespace BDD
* @api public
*/

function assertLeast (n, msg) {
if (msg) flag(this, 'message', msg);
var obj = flag(this, 'object');
if (flag(this, 'doLength')) {
var obj = flag(this, 'object')
, doLength = flag(this, 'doLength');

if (doLength) {
new Assertion(obj, msg).to.have.property('length');
} else {
new Assertion(obj, msg).is.a('number');
}

if (_.type(n) !== 'number') {
throw new Error('the argument to least must be a number');
}

if (doLength) {
var len = obj.length;
this.assert(
len >= n
Expand All @@ -600,14 +626,15 @@ module.exports = function (chai, _) {
/**
* ### .below(value)
*
* Asserts that the target is less than `value`.
* Asserts that the number target is less than `value`.
*
* expect(5).to.be.below(10);
*
* Can also be used in conjunction with `length` to
* assert a maximum length. The benefit being a
* more informative error message than if the length
* was supplied directly.
* was supplied directly. In this case the target must
* have a length property.
*
* expect('foo').to.have.length.below(4);
* expect([ 1, 2, 3 ]).to.have.length.below(4);
Expand All @@ -617,14 +644,26 @@ module.exports = function (chai, _) {
* @alias lessThan
* @param {Number} value
* @param {String} message _optional_
* @namespace BDD
* @api public
*/

function assertBelow (n, msg) {
if (msg) flag(this, 'message', msg);
var obj = flag(this, 'object');
if (flag(this, 'doLength')) {
var obj = flag(this, 'object')
, doLength = flag(this, 'doLength');

if (doLength) {
new Assertion(obj, msg).to.have.property('length');
} else {
new Assertion(obj, msg).is.a('number');
}

if (_.type(n) !== 'number') {
throw new Error('the argument to below must be a number');
}

if (doLength) {
var len = obj.length;
this.assert(
len < n
Expand All @@ -649,14 +688,15 @@ module.exports = function (chai, _) {
/**
* ### .most(value)
*
* Asserts that the target is less than or equal to `value`.
* Asserts that the number target is less than or equal to `value`.
Copy link
Member

Choose a reason for hiding this comment

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

Again "target number"

*
* expect(5).to.be.at.most(5);
*
* Can also be used in conjunction with `length` to
* assert a maximum length. The benefit being a
* more informative error message than if the length
* was supplied directly.
* was supplied directly. In this case the target must
* have a length property.
*
* expect('foo').to.have.length.of.at.most(4);
* expect([ 1, 2, 3 ]).to.have.length.of.at.most(3);
Expand All @@ -665,14 +705,26 @@ module.exports = function (chai, _) {
* @alias lte
* @param {Number} value
* @param {String} message _optional_
* @namespace BDD
* @api public
*/

function assertMost (n, msg) {
if (msg) flag(this, 'message', msg);
var obj = flag(this, 'object');
if (flag(this, 'doLength')) {
var obj = flag(this, 'object')
, doLength = flag(this, 'doLength');

if (doLength) {
new Assertion(obj, msg).to.have.property('length');
} else {
new Assertion(obj, msg).is.a('number');
}

if (_.type(n) !== 'number') {
throw new Error('the argument to most must be a number');
}

if (doLength) {
var len = obj.length;
this.assert(
len <= n
Expand All @@ -696,14 +748,15 @@ module.exports = function (chai, _) {
/**
* ### .within(start, finish)
*
* Asserts that the target is within a range.
* Asserts that the number target is within a range of numbers.
Copy link
Member

Choose a reason for hiding this comment

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

Here again "target number"

*
* expect(7).to.be.within(5,10);
*
* Can also be used in conjunction with `length` to
* assert a length range. The benefit being a
* more informative error message than if the length
* was supplied directly.
* was supplied directly. In this case the target must
* have a length property.
*
* expect('foo').to.have.length.within(2,4);
* expect([ 1, 2, 3 ]).to.have.length.within(2,4);
Expand All @@ -712,15 +765,27 @@ module.exports = function (chai, _) {
* @param {Number} start lowerbound inclusive
* @param {Number} finish upperbound inclusive
* @param {String} message _optional_
* @namespace BDD
* @api public
*/

Assertion.addMethod('within', function (start, finish, msg) {
if (msg) flag(this, 'message', msg);
var obj = flag(this, 'object')
, range = start + '..' + finish;
if (flag(this, 'doLength')) {
, range = start + '..' + finish
, doLength = flag(this, 'doLength');

if (doLength) {
new Assertion(obj, msg).to.have.property('length');
} else {
new Assertion(obj, msg).is.a('number');
}

if (_.type(start) !== 'number' || _.type(finish) !== 'number') {
throw new Error('the arguments to within must be numbers');
}

if (doLength) {
var len = obj.length;
this.assert(
len >= start && len <= finish
Expand Down
15 changes: 15 additions & 0 deletions test/assert.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,14 @@ describe('assert', function () {
err(function() {
assert.isAbove(1, 1);
}, 'expected 1 to be above 1');

err(function() {
assert.isAbove(null, 1);
}, 'expected null to be a number');

err(function() {
assert.isAbove(1, null);
}, 'the argument to above must be a number');
});

it('below', function() {
Expand All @@ -802,6 +810,13 @@ describe('assert', function () {
assert.isBelow(1, 1);
}, 'expected 1 to be below 1');

err(function() {
assert.isBelow(null, 1);
}, 'expected null to be a number');

err(function() {
assert.isBelow(1, null);
}, 'the argument to below must be a number');
});

it('memberDeepEquals', function() {
Expand Down
Loading