Update to latest version of expect.js #400
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -6,6 +6,7 @@ var vegetables = require('./fixtures').vegetables; | |||
| var food = require('./fixtures').food; | |||
| var chocolates = require('./fixtures').chocolates; | |||
| var inputs = require('./fixtures').inputs; | |||
| var toArray = Array.prototype.slice.call.bind(Array.prototype.slice); | |||
There was a problem hiding this comment.
The first part could be replaced for example with Function.call.bind….
|
LGTM |
In version 0.3, expect.js corrected the order of its `eql` assertion. This exposed several bugs in Cheerio's unit tests, where `aguments` objects were being tested for deep equality with Array instances.
fb55
added a commit
that referenced
this pull request
Feb 28, 2014
Update to latest version of expect.js
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #350, @fb55 and I discovered that the latest version of
expect.js(0.3.1) caused false positives in Cheerio's unit test suite. Worse, the generated error was causing Mocha to crash, so TravisCI was unaware that the build was failing. (Actually, you might say this was a good thing because the errors were false positives anyway. But I would argue with you :P)This problem was introduced when
expect.jscorrected the parameter order of itseqlcomparator. This was a legitimate bug fix--it corrected semantics where (functionally speaking) order has no value.Or it should have no value. Actually, the comparison internal logic has a flaw which makes it non-reflexive. I've submitted a patch against
expect.jsto remedy this and a separate patch to Node.js (sinceexpect.js's implementation is derived from there)....but that has little significance to the problem in our test suite--an
argumentsobject should never be considered equivalent to an Array instance. That's why I'm updating the tests to convert allargumentsinto Arrays before comparison.As for Mocha: it was crashing while rendering string diffs for the errors generated by
expect.js. I've submitted a patch for that, too, but it shouldn't have any bearing on this change.Commit message: