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

Incompatibility with chai@4 #71

Closed
aqrln opened this issue May 27, 2017 · 18 comments
Closed

Incompatibility with chai@4 #71

aqrln opened this issue May 27, 2017 · 18 comments

Comments

@aqrln
Copy link

aqrln commented May 27, 2017

chai version: 4.0.0
chai-spies version: 0.7.1
Node.js version: 7.10.0

expect(spy).to.be.called.with(...) fails with:

Error: Invalid Chai property: _his. Did you mean "is"?
 at Object.proxyGetter [as get] (node_modules/chai/lib/chai/utils/proxify.js:63:17)
 at Proxy.assertWith (node_modules/chai-spies/lib/spy.js:283:29)
 at Proxy.chainableMethodWrapper (node_modules/chai/lib/chai/utils/addChainableMethod.js:113:49)

Besides that, if this is a breaking change in the public API for plugin authors, it should probably have been mentioned in the release notes for chai@4.0.0.

Thanks!

@meeber
Copy link
Contributor

meeber commented May 27, 2017

@aqrln I think this is just a case of the browser build needing to be updated. The _his thing is a bug that was fixed in #56, but the browser build hasn't been updated since then. In versions of Chai prior to v4, this bug would've just resulted in nonsensical failed assertion messages for certain types of negated assertions, but in Chai v4 it's causing a louder error due to the new proxy protection catching invalid properties being chained off of assertions.

Pinging @keithamus

@aqrln
Copy link
Author

aqrln commented May 27, 2017

@meeber oh, sorry, I forgot to mention the environment. It is Node, not a browser. I've updated the first comment to avoid further confusion.

@meeber
Copy link
Contributor

meeber commented May 27, 2017

@aqrln It looks like the last npm release was also before the fix. Does it break when installing this plugin from the github master branch?

@aqrln
Copy link
Author

aqrln commented May 27, 2017

@meeber

It looks like the last npm release was also before the fix.

Yeah, I see now. I was about to write this, but you beat me to it :)

Does it break when installing this plugin from the github master branch?

I'll try it in a minute.

@aqrln
Copy link
Author

aqrln commented May 27, 2017

@meeber yep, it works. Thanks!

@tindn
Copy link

tindn commented Jun 1, 2017

chai version: 4.0.1
chai-spies version: 0.7.1 (from github master branch)
Node.js version: 6.10.3

I had the same error as @aqrln, and installing from github fixed it. Now i ran to the following error

expect(spy).to.have.been.called.once() fails with:

Unhandled rejection TypeError: expect(...).to.have.been.called.once is not a function

Would appreciate any help or direction on how to figure this issue out.

Thanks

@aqrln
Copy link
Author

aqrln commented Jun 2, 2017

@tindn regarding this error, I believe we should be able to just update the package from the npm registry on Sunday, yay.

I'd open a new issue for the second one, let's keep this thread on-topic. Such thing tend to become lost if they aren't in an appropriate place, especially given that I'm going to go ahead and close this issue as it is actually resolved now (well, not yet completely, but the release is scheduled, so there's no point in keeping this on the backlog).

@aqrln aqrln closed this as completed Jun 2, 2017
@interlock
Copy link

interlock commented Jun 5, 2017

So according to the linked issue, only the master branch of this plugin is working. The tagged npm release 0.7.1 is not. Can we get a release to bump or are we all forced to tag this package to master?

"chai-spies": "git://github.com/chaijs/chai-spies.git#master"

or if you like to lock it to current HEAD:

"chai-spies": "git://github.com/chaijs/chai-spies.git#b11aeebf"

@meeber
Copy link
Contributor

meeber commented Jun 9, 2017

@keithamus What's the release process for this module? Manually bump version numbers in files, open PR, merge PR, tag commit, let travis do its thing?

@keithamus
Copy link
Member

@meeber right now manually release. I'm working on a PR to get it in-line with deep-eql, check-error etc - so that it'll be an auto-release with semver. Same for chai-http and others. Hope to put something up before Monday

@max-degterev
Copy link

max-degterev commented Jun 13, 2017

can we get a quick bump to 0.7.2 with current master? locking package at certain commit/branch breaks our deploy 😞

@heath-guidewire
Copy link

heath-guidewire commented Jul 22, 2017

Same here in our world... could really use a bump to 0.7.2... Actually I'm surprised there hasn't been a version release since this issue has been known for almost 2 months and there has been talk more than a month ago that the release was scheduled... Does it really take that long to push a patch version?!

whatwewant added a commit to whatwewant/dva-socket.io that referenced this issue Jul 23, 2017
@max-degterev
Copy link

max-degterev commented Jul 26, 2017

Could probably switch to SinonJS easily enough. API seems similar. Anyone tried this?

Update: https://github.com/domenic/sinon-chai
Update: For anyone interested in migrating to sinon - it was pretty straightforward, can probably even be done with autoreplace. It's fully compatible, no issues with chai v4.

@shellscape
Copy link

I've published chai-spies-next for those who want a dep not linked to master (in case of breaking changes). Toss me into the pile of users that are surprised with slight historical mismanagement of this repo/module.

timja pushed a commit to hmcts/cmc-citizen-frontend that referenced this issue Aug 20, 2017
Remove chai-spies as its not being maintained properly
chaijs/chai-spies#71
@kontrollanten
Copy link

You're a hero, @shellscape

@hypescript
Copy link

Ping @keithamus
Just wanted to remind the maintainers that despite this bug being marked as fixed back in June there is still no published version that includes the fix.

@shellscape
Copy link

@kontrollanten @hypescript I've moved on from chai in general - the mismanagement of the projects in the org was just too brutal - so I'm probably not going to be maintaining chai-spies-next actively, if at all. If either/both of you (or anyone else) would like to be added as a collaborator, please ping me on the twitters @shellscape and I'll get you setup.

hypescript pushed a commit to hypescript/authmosphere that referenced this issue Dec 31, 2017
Please note, that:
 * cannot use chai-spies due to chaijs/chai-spies#71
 * async calls to next won't be detected, just as they were not detected before
@LukasBombach
Copy link

At least it works for now, thanks @shellscape

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests