-
Notifications
You must be signed in to change notification settings - Fork 16
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
Fix broken assert.equal assertion when not applied on a Collection #18
Changes from all commits
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 |
---|---|---|
|
@@ -416,6 +416,7 @@ module.exports = function (chai, utils) { | |
*/ | ||
|
||
var assert = chai.assert; | ||
var originalEqual = assert.equal; | ||
|
||
/** | ||
* ### .equal(actual, expected) | ||
|
@@ -437,10 +438,15 @@ module.exports = function (chai, utils) { | |
*/ | ||
|
||
assert.equal = function (actual, expected) { | ||
/* | ||
* It seems like we shouldn't actually need this check, however, | ||
* `assert.equal` actually behaves differently than its BDD counterpart! | ||
* Namely, the BDD version is strict while the "assert" one isn't. | ||
*/ | ||
if (actual instanceof Collection) { | ||
return new Assertion(actual).equal(expected); | ||
} | ||
else return assert.equal; | ||
else return originalEqual(actual, expected); | ||
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. You nailed it, actually! The issue isn't so much that I didn't call the original assertion, but that I was returning a function, which never throws anything. And of course, not storing the original assertion somewhere triggers an infinite loop. That's one more proof we need more tests... 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. (: |
||
}; | ||
|
||
/** | ||
|
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.
Good point to add that comment, I was actually just thinking "wait, why do we just and only call
equal
in the equal assertion?!".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.
Yeah /:
Not too happy that chai does that but oh well. Incidentally, I wasn't sure what comment style to use here…this was the only thing that passed the linter!
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.
Mmh, not cool re: linter. I'll check, edit the rules and replace with
//
s instead, makes more sense here. Sorry about that.