-
-
Notifications
You must be signed in to change notification settings - Fork 698
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
Assertion in 1.9.2 throws JS error in 1.10.0 #302
Comments
I have a similar issue in that assertions that were working in 1.9.2 now fail in 1.10.0 with the same In my case it is the Example:callback.should.have.been.calledWith(elem);
// => TypeError: 'undefined' is not a function (evaluating 'callback.should.have.been.calledWith(elem)') Edit: using [email protected] |
On further investigation, this seems to only be a problem for our in-browser tests run using karma, and not for unit tests run in a Node environment. Karma is running in [email protected], [email protected] and [email protected]. The failures and errors are the same in each case. |
Seems like this is because the new end assertions return no-op functions, which have immutable properties ( When doing > expect([1,2,3]).to.exist()
{ __flags:
{ ssfi: [Function: assert],
object: [ 1, 2, 3 ],
message: undefined } }
> expect([1,2,3]).to.exist() instanceof chai.Assertion
true
> expect([1,2,3]).to.exist
[Function: assert]
> expect([1,2,3]).to.exist instanceof chai.Assertion
true
> @joshperry got any ideas to get around this? |
Thinking out loud: If we make every property/method check to see if it is getting the no-op function - and simply resolve it, then we can potentially fix this issue - having said that, while we might be able to get around The other way around would be to make chainableNoop return a function that does the assertion, rather than doing the assertion in the getter, but this will break code like Of course, another option would be to revert the originating PR and go back to having JSLint unfriendly assertions. I'd personally like to see them kept but not at the price of breaking other code or adding odd quirks. |
Like you said, this will be a problem with any of the properties on a function object that can not be overwritten: I don't think any breaking changes are going to be acceptable, and we aren't going to get this without breaking Another plan could be to create a new assertion called something like |
As I don't want to prevent our users from upgrading in the future, I'm going to go with @joshperry on this and say revert the PR until an acceptable solution can be implemented. As for |
Thanks for the quick investigation / explanation - will remove the version restriction after the reversion. |
PR was reverted. Although 1.10.0 is still the latest release. @logicalparadox we should probably release a new version which reverts the changes - I vote for |
@logicalparadox if you're around - this could do with taking a look at. IMO commit b71b930 should be released as Chai v1.10.1 (it reverts the functionality from #306 - which this issue talks about). After #312 and #313 we could probably also cut a v1.11.0 release, as we've got lots of new stuff in 😄 |
On Nov 11 2014, @lennym wrote..
@lennym - did you ever get this issue sorted? I'm seeing a similar problem where My test suite used to work before upgrading to [email protected]. UPDATE: I did manage to successfully rollback to 1.9.2, by deleting the node_modules folder and reinstalling (ie. At first, I thought it was only This occurs with both the should and expect APIs, but not the assert API. eg.
This behaviour was seen in karma in [email protected], using the following npm modules:
I checked if any of these modules bundled their own version of chai (perhaps one was still using 1.10.0 and ignoring my rolled-back 1.9.2 version?). Only karma-sinon-chai does, but it seems to be using 1.9.2 as well:
|
@scottohara It looks to me like the sinon-chai assertions were not being registered with chai for some reason. This is a great example of some small issue causing assertion properties to produce false-positives because javascript silently ignores non-existent property accesses. |
Closing this - as we have a 2.0.0 tracking PR. 2.0.0 will revert the behaviour that is causing this issue. If anyone disagrees, feel free to comment and I'll re-open it. 2.0.0 tracking issue is #364 |
We have several assertions in our core tests of the form
After upgrading to 1.10.0 from 1.9.2, these assertions now throw an error
I found that changing the assertion to
seems to fix the problem, but I'm not entirely sure why. Any insight would be appreciated, thanks!
The text was updated successfully, but these errors were encountered: