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

Merge rebased 4.x.x branch into master #783

Merged
merged 19 commits into from
Sep 10, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
3 changes: 2 additions & 1 deletion lib/chai/assertion.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ module.exports = function (_chai, util) {

Assertion.prototype.assert = function (expr, msg, negateMsg, expected, _actual, showDiff) {
var ok = util.test(this, arguments);
if (true !== showDiff) showDiff = false;
if (false !== showDiff) showDiff = true;
if (undefined === expected && undefined === _actual) showDiff = false;
if (true !== config.showDiff) showDiff = false;

if (!ok) {
Expand Down
165 changes: 125 additions & 40 deletions lib/chai/core/assertions.js
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ module.exports = function (chai, _) {
* ### .NaN
* Asserts that the target is `NaN`.
*
* expect('foo').to.be.NaN;
* expect(NaN).to.be.NaN;
* expect(4).not.to.be.NaN;
*
* @name NaN
Expand All @@ -456,7 +456,7 @@ module.exports = function (chai, _) {

Assertion.addProperty('NaN', function () {
this.assert(
isNaN(flag(this, 'object'))
_.isNaN(flag(this, 'object'))
, 'expected #{this} to be NaN'
, 'expected #{this} not to be NaN'
);
Expand Down Expand Up @@ -507,17 +507,10 @@ module.exports = function (chai, _) {
*/

Assertion.addProperty('empty', function () {
var obj = flag(this, 'object')
, expected = obj;

if (Array.isArray(obj) || 'string' === typeof object) {
expected = obj.length;
} else if (typeof obj === 'object') {
expected = Object.keys(obj).length;
}

var obj = flag(this, 'object');
new Assertion(obj).to.exist;
this.assert(
!expected
Object.keys(Object(obj)).length === 0
, 'expected #{this} to be empty'
, 'expected #{this} not to be empty'
);
Expand Down Expand Up @@ -629,14 +622,15 @@ module.exports = function (chai, _) {
/**
* ### .above(value)
*
* Asserts that the target is greater than `value`.
* Asserts that the number target 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 @@ -652,9 +646,20 @@ module.exports = function (chai, _) {

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');
}

if (doLength) {
var len = obj.length;
this.assert(
len > n
Expand All @@ -679,14 +684,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

@lucasfcosta lucasfcosta Sep 10, 2016

Choose a reason for hiding this comment

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

I know this is a merge, but I'll just leave this here as a note:
Shouldn't it be written Asserts that the **target number** ... instead of Asserts that the **number target** ...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the reason it was phrased that way is because that's how it was already phrased elsewhere in the code (see: #692 (comment)). I think the thing for us to do is submit a separate PR that rephrases all of them (both old and new) at once.

Copy link
Member

Choose a reason for hiding this comment

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

Ahhh, I remember that now!
Thanks for refreshing my memory!
I totally agree with you, that would be a great idea, I will open an issue for that just so we won't forget.

*
* 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);
Copy link
Member

Choose a reason for hiding this comment

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

Another side note here:

Currently we can't do:
expect([ 1, 2, 3 ]).to.have.a.length.of.at.least(3);
Because the a word returns a function which has a property called .length and there is no way we can remove that in some environments, but we could use a Proxy in environments that do support it to override what happens when trying to access length on the function returned by addChainableBehavior, what do you think? Is this possible? I didn't dig deep on this one, but I think it might be a valid strategy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, because function.length is configurable in modern environments, it can be intercepted with proxies or manually deleted without the use of proxies. I think one of them is worth doing (in a separate PR) but I'm not sure which approach is best.

Expand All @@ -701,9 +707,20 @@ module.exports = function (chai, _) {

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 @@ -727,14 +744,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 @@ -750,9 +768,20 @@ module.exports = function (chai, _) {

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 @@ -777,14 +806,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`.
*
* 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 @@ -799,9 +829,20 @@ module.exports = function (chai, _) {

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 @@ -825,14 +866,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.
*
* 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 @@ -848,9 +890,20 @@ module.exports = function (chai, _) {
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 Expand Up @@ -1028,28 +1081,59 @@ module.exports = function (chai, _) {


/**
* ### .ownProperty(name)
* ### .ownProperty(name, [value])
*
* Asserts that the target has an own property `name`.
* Asserts that the target has an own property `name` and, optionally, if it has
* (or not, if using `.not`) the desired `value`.
*
* expect('test').to.have.ownProperty('length');
* expect('test').to.haveOwnProperty('length');
* expect('test').to.not.have.ownProperty('foo');
* expect('test').to.not.haveOwnProperty('foo');
* expect({ length: 12 }).to.have.ownProperty('length', 12);
* expect({ length: 1337 }).to.not.have.ownProperty('length', 20);
* expect({ length: 12 }).to.haveOwnProperty('length', 12);
* expect({ length: 1337 }).to.not.haveOwnProperty('length', 20);
*
* @name ownProperty
* @alias haveOwnProperty
* @param {String} name
* @param {Mixed} value (optional)
* @param {String} message _optional_
* @namespace BDD
* @api public
*/

function assertOwnProperty (name, msg) {
function assertOwnProperty (name, value, msg) {
if (msg) flag(this, 'message', msg);
var obj = flag(this, 'object');
this.assert(
Object.prototype.hasOwnProperty.call(obj, name)
, 'expected #{this} to have own property ' + _.inspect(name)
, 'expected #{this} to not have own property ' + _.inspect(name)
);
var negate = flag(this, 'negate');
var objHasProperty = Object.prototype.hasOwnProperty.call(obj, name)
var actualValue = obj[name];

if (negate && value !== undefined) {
if (actualValue === undefined) {
throw new Error(_.inspect(obj) + ' does not have own property ' + _.inspect(name));
}
} else {
this.assert(
objHasProperty
, 'expected #{this} to have own property ' + _.inspect(name)
, 'expected #{this} to not have own property ' + _.inspect(name)
);
}

if (value !== undefined) {
this.assert(
actualValue === value
, 'expected #{this} to have own property ' + _.inspect(name) + ' of #{exp}, but got #{act}'
, 'expected #{this} to not have own property ' + _.inspect(name) + ' of #{act}'
, value
, actualValue
);
}

flag(this, 'object', actualValue);
}

Assertion.addMethod('ownProperty', assertOwnProperty);
Expand Down Expand Up @@ -1774,6 +1858,7 @@ module.exports = function (chai, _) {
, failNegateMsg
, subset
, obj
, true
);
});

Expand Down
Loading