-
-
Notifications
You must be signed in to change notification settings - Fork 693
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
Allow dates for isBelow and isAbove assertions #990
Changes from 13 commits
e277921
1f2b171
c3718ea
4364d1d
90c156a
ce63a96
9d33702
365fa21
a5428a4
ee3cefc
5e6a33a
870a7f3
ccbaf82
fc56c36
5330208
4b5bb3c
6638ef4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1024,7 +1024,7 @@ module.exports = function (chai, _) { | |
/** | ||
* ### .above(n[, msg]) | ||
* | ||
* Asserts that the target is a number greater than the given number `n`. | ||
* Asserts that the target is number or a date greater than the given number or a date `n` respectively. | ||
* However, it's often best to assert that the target is equal to its expected | ||
* value. | ||
* | ||
|
@@ -1069,21 +1069,29 @@ module.exports = function (chai, _) { | |
var obj = flag(this, 'object') | ||
, doLength = flag(this, 'doLength') | ||
, flagMsg = flag(this, 'message') | ||
, ssfi = flag(this, 'ssfi'); | ||
, msgPrefix = ((flagMsg) ? flagMsg + ': ' : '') | ||
, ssfi = flag(this, 'ssfi') | ||
, objType = _.type(obj).toLowerCase() | ||
, nType = _.type(n).toLowerCase() | ||
, shouldThrow = true; | ||
|
||
if (doLength) { | ||
new Assertion(obj, flagMsg, ssfi, true).to.have.property('length'); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It feels like there's a lot of complexity here, for what is essentially 3 use cases:
I feel like L1079-1117 could just be simplified to the following: if (objType === 'date' && nType !== 'date') {
throw ...
} else if (objType === 'number' && nType !== 'number') {
throw ...
} else if (doLength && nType !== 'number') {
throw ...
} else {
throw ...
} |
||
|
||
if (!doLength && (objType === 'date' && nType !== 'date')) { | ||
errorMessage = msgPrefix + 'the argument to above must be a date'; | ||
} else if (nType !== 'number' && (doLength || objType === 'number')) { | ||
errorMessage = msgPrefix + 'the argument to above must be a number'; | ||
} else if (!doLength && (objType !== 'date' && objType !== 'number')) { | ||
var printObj = (objType === 'string') ? "'" + obj + "'" : obj; | ||
errorMessage = msgPrefix + 'expected ' + printObj + ' to be a number or a date'; | ||
} else { | ||
new Assertion(obj, flagMsg, ssfi, true).is.a('number'); | ||
shouldThrow = false; | ||
} | ||
|
||
if (typeof n !== 'number') { | ||
flagMsg = flagMsg ? flagMsg + ': ' : ''; | ||
throw new AssertionError( | ||
flagMsg + 'the argument to above must be a number', | ||
undefined, | ||
ssfi | ||
); | ||
if (shouldThrow) { | ||
throw new AssertionError(errorMessage, undefined, ssfi); | ||
} | ||
|
||
if (doLength) { | ||
|
@@ -1098,8 +1106,9 @@ module.exports = function (chai, _) { | |
} else { | ||
this.assert( | ||
obj > n | ||
, 'expected #{this} to be above ' + n | ||
, 'expected #{this} to be at most ' + n | ||
, 'expected #{this} to be above #{exp}' | ||
, 'expected #{this} to be at most #{exp}' | ||
, n | ||
); | ||
} | ||
} | ||
|
@@ -1111,8 +1120,8 @@ module.exports = function (chai, _) { | |
/** | ||
* ### .least(n[, msg]) | ||
* | ||
* Asserts that the target is a number greater than or equal to the given | ||
* number `n`. However, it's often best to assert that the target is equal to | ||
* Asserts that the target is a number or a date greater than or equal to the given | ||
* number or a date `n` respectively. However, it's often best to assert that the target is equal to | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't read exactly right to me. I think: "Asserts that the target is a number or a date greater than or equal to the given number or date |
||
* its expected value. | ||
* | ||
* expect(2).to.equal(2); // Recommended | ||
|
@@ -1156,21 +1165,29 @@ module.exports = function (chai, _) { | |
var obj = flag(this, 'object') | ||
, doLength = flag(this, 'doLength') | ||
, flagMsg = flag(this, 'message') | ||
, ssfi = flag(this, 'ssfi'); | ||
, msgPrefix = ((flagMsg) ? flagMsg + ': ' : '') | ||
, ssfi = flag(this, 'ssfi') | ||
, objType = _.type(obj).toLowerCase() | ||
, nType = _.type(n).toLowerCase() | ||
, shouldThrow = true; | ||
|
||
if (doLength) { | ||
new Assertion(obj, flagMsg, ssfi, true).to.have.property('length'); | ||
} | ||
|
||
if (!doLength && (objType === 'date' && nType !== 'date')) { | ||
errorMessage = msgPrefix + 'the argument to least must be a date'; | ||
} else if (nType !== 'number' && (doLength || objType === 'number')) { | ||
errorMessage = msgPrefix + 'the argument to least must be a number'; | ||
} else if (!doLength && (objType !== 'date' && objType !== 'number')) { | ||
var printObj = (objType === 'string') ? "'" + obj + "'" : obj; | ||
errorMessage = msgPrefix + 'expected ' + printObj + ' to be a number or a date'; | ||
} else { | ||
new Assertion(obj, flagMsg, ssfi, true).is.a('number'); | ||
shouldThrow = false; | ||
} | ||
|
||
if (typeof n !== 'number') { | ||
flagMsg = flagMsg ? flagMsg + ': ' : ''; | ||
throw new AssertionError( | ||
flagMsg + 'the argument to least must be a number', | ||
undefined, | ||
ssfi | ||
); | ||
if (shouldThrow) { | ||
throw new AssertionError(errorMessage, undefined, ssfi); | ||
} | ||
|
||
if (doLength) { | ||
|
@@ -1185,8 +1202,9 @@ module.exports = function (chai, _) { | |
} else { | ||
this.assert( | ||
obj >= n | ||
, 'expected #{this} to be at least ' + n | ||
, 'expected #{this} to be below ' + n | ||
, 'expected #{this} to be at least #{exp}' | ||
, 'expected #{this} to be below #{exp}' | ||
, n | ||
); | ||
} | ||
} | ||
|
@@ -1197,7 +1215,7 @@ module.exports = function (chai, _) { | |
/** | ||
* ### .below(n[, msg]) | ||
* | ||
* Asserts that the target is a number less than the given number `n`. | ||
* Asserts that the target is a number or a date less than the given number or a date `n` respectively. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't read exactly right to me. I think: "Asserts that the target is a number or a date less than the given number or date There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @meeber I don't see how what you wrote is different from what's already there (green). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @v1adko In almost every case, the suggestion is "given number or date" instead of "given number or a date". In addition to that, the one for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @meeber So no articles then? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Articles for the first half (either "the target is a number or a date", or, "the target is a number or date"). But no articles for the second half ("than the given number or date"). |
||
* However, it's often best to assert that the target is equal to its expected | ||
* value. | ||
* | ||
|
@@ -1242,21 +1260,29 @@ module.exports = function (chai, _) { | |
var obj = flag(this, 'object') | ||
, doLength = flag(this, 'doLength') | ||
, flagMsg = flag(this, 'message') | ||
, ssfi = flag(this, 'ssfi'); | ||
, msgPrefix = ((flagMsg) ? flagMsg + ': ' : '') | ||
, ssfi = flag(this, 'ssfi') | ||
, objType = _.type(obj).toLowerCase() | ||
, nType = _.type(n).toLowerCase() | ||
, shouldThrow = true; | ||
|
||
if (doLength) { | ||
new Assertion(obj, flagMsg, ssfi, true).to.have.property('length'); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And again, as above. |
||
|
||
if (!doLength && (objType === 'date' && nType !== 'date')) { | ||
errorMessage = msgPrefix + 'the argument to below must be a date'; | ||
} else if (nType !== 'number' && (doLength || objType === 'number')) { | ||
errorMessage = msgPrefix + 'the argument to below must be a number'; | ||
} else if (!doLength && (objType !== 'date' && objType !== 'number')) { | ||
var printObj = (objType === 'string') ? "'" + obj + "'" : obj; | ||
errorMessage = msgPrefix + 'expected ' + printObj + ' to be a number or a date'; | ||
} else { | ||
new Assertion(obj, flagMsg, ssfi, true).is.a('number'); | ||
shouldThrow = false; | ||
} | ||
|
||
if (typeof n !== 'number') { | ||
flagMsg = flagMsg ? flagMsg + ': ' : ''; | ||
throw new AssertionError( | ||
flagMsg + 'the argument to below must be a number', | ||
undefined, | ||
ssfi | ||
); | ||
if (shouldThrow) { | ||
throw new AssertionError(errorMessage, undefined, ssfi); | ||
} | ||
|
||
if (doLength) { | ||
|
@@ -1271,8 +1297,9 @@ module.exports = function (chai, _) { | |
} else { | ||
this.assert( | ||
obj < n | ||
, 'expected #{this} to be below ' + n | ||
, 'expected #{this} to be at least ' + n | ||
, 'expected #{this} to be below #{exp}' | ||
, 'expected #{this} to be at least #{exp}' | ||
, n | ||
); | ||
} | ||
} | ||
|
@@ -1284,8 +1311,8 @@ module.exports = function (chai, _) { | |
/** | ||
* ### .most(n[, msg]) | ||
* | ||
* Asserts that the target is a number less than or equal to the given number | ||
* `n`. However, it's often best to assert that the target is equal to its | ||
* Asserts that the target is a number or a date less than or equal to the given number | ||
* or a date `n` respectively. However, it's often best to assert that the target is equal to its | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't read exactly right to me. I think: "Asserts that the target is a number or a date less than or equal to the given number or date |
||
* expected value. | ||
* | ||
* expect(1).to.equal(1); // Recommended | ||
|
@@ -1328,21 +1355,29 @@ module.exports = function (chai, _) { | |
var obj = flag(this, 'object') | ||
, doLength = flag(this, 'doLength') | ||
, flagMsg = flag(this, 'message') | ||
, ssfi = flag(this, 'ssfi'); | ||
, msgPrefix = ((flagMsg) ? flagMsg + ': ' : '') | ||
, ssfi = flag(this, 'ssfi') | ||
, objType = _.type(obj).toLowerCase() | ||
, nType = _.type(n).toLowerCase() | ||
, shouldThrow = true; | ||
|
||
if (doLength) { | ||
new Assertion(obj, flagMsg, ssfi, true).to.have.property('length'); | ||
} | ||
|
||
if (!doLength && (objType === 'date' && nType !== 'date')) { | ||
errorMessage = msgPrefix + 'the argument to most must be a date'; | ||
} else if (nType !== 'number' && (doLength || objType === 'number')) { | ||
errorMessage = msgPrefix + 'the argument to most must be a number'; | ||
} else if (!doLength && (objType !== 'date' && objType !== 'number')) { | ||
var printObj = (objType === 'string') ? "'" + obj + "'" : obj; | ||
errorMessage = msgPrefix + 'expected ' + printObj + ' to be a number or a date'; | ||
} else { | ||
new Assertion(obj, flagMsg, ssfi, true).is.a('number'); | ||
shouldThrow = false; | ||
} | ||
|
||
if (typeof n !== 'number') { | ||
flagMsg = flagMsg ? flagMsg + ': ' : ''; | ||
throw new AssertionError( | ||
flagMsg + 'the argument to most must be a number', | ||
undefined, | ||
ssfi | ||
); | ||
if (shouldThrow) { | ||
throw new AssertionError(errorMessage, undefined, ssfi); | ||
} | ||
|
||
if (doLength) { | ||
|
@@ -1357,8 +1392,9 @@ module.exports = function (chai, _) { | |
} else { | ||
this.assert( | ||
obj <= n | ||
, 'expected #{this} to be at most ' + n | ||
, 'expected #{this} to be above ' + n | ||
, 'expected #{this} to be at most #{exp}' | ||
, 'expected #{this} to be above #{exp}' | ||
, n | ||
); | ||
} | ||
} | ||
|
@@ -1369,8 +1405,8 @@ module.exports = function (chai, _) { | |
/** | ||
* ### .within(start, finish[, msg]) | ||
* | ||
* Asserts that the target is a number greater than or equal to the given | ||
* number `start`, and less than or equal to the given number `finish`. | ||
* Asserts that the target is a number or a date greater than or equal to the given | ||
* number or a date `start`, and less than or equal to the given number or a date `finish` respectively. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't read exactly right to me. I think: "Asserts that the target is a number or a date greater than or equal to the given number or date |
||
* However, it's often best to assert that the target is equal to its expected | ||
* value. | ||
* | ||
|
@@ -1412,24 +1448,35 @@ 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 | ||
, doLength = flag(this, 'doLength') | ||
, flagMsg = flag(this, 'message') | ||
, ssfi = flag(this, 'ssfi'); | ||
, msgPrefix = ((flagMsg) ? flagMsg + ': ' : '') | ||
, ssfi = flag(this, 'ssfi') | ||
, objType = _.type(obj).toLowerCase() | ||
, startType = _.type(start).toLowerCase() | ||
, finishType = _.type(finish).toLowerCase() | ||
, shouldThrow = true | ||
, range = (startType === 'date' && finishType === 'date') | ||
? start.toUTCString() + '..' + finish.toUTCString() | ||
: start + '..' + finish; | ||
|
||
if (doLength) { | ||
new Assertion(obj, flagMsg, ssfi, true).to.have.property('length'); | ||
} | ||
|
||
if (!doLength && (objType === 'date' && (startType !== 'date' || finishType !== 'date'))) { | ||
errorMessage = msgPrefix + 'the arguments to within must be dates'; | ||
} else if ((startType !== 'number' || finishType !== 'number') && (doLength || objType === 'number')) { | ||
errorMessage = msgPrefix + 'the arguments to within must be numbers'; | ||
} else if (!doLength && (objType !== 'date' && objType !== 'number')) { | ||
var printObj = (objType === 'string') ? "'" + obj + "'" : obj; | ||
errorMessage = msgPrefix + 'expected ' + printObj + ' to be a number or a date'; | ||
} else { | ||
new Assertion(obj, flagMsg, ssfi, true).is.a('number'); | ||
shouldThrow = false; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And again, but we need to check both arguments to ensure all 3 (obj, start, finish) are the same type. |
||
|
||
if (typeof start !== 'number' || typeof finish !== 'number') { | ||
flagMsg = flagMsg ? flagMsg + ': ' : ''; | ||
throw new AssertionError( | ||
flagMsg + 'the arguments to within must be numbers', | ||
undefined, | ||
ssfi | ||
); | ||
if (shouldThrow) { | ||
throw new AssertionError(errorMessage, undefined, ssfi); | ||
} | ||
|
||
if (doLength) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't read exactly right to me. I think: "Asserts that the target is a number or a date greater than the given number or date
n
respectively."