chaijs / chai-spies

Spies for Chai Assertion Library.
MIT License
132 stars 29 forks source link

Incompatibility with chai@4 #71

Closed aqrln closed 7 years ago

aqrln commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@meeber yep, it works. Thanks!

tindn commented 7 years ago

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 commented 7 years ago

@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).

interlock commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

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

heath-guidewire commented 7 years ago

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?!

max-degterev commented 7 years ago

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 commented 7 years ago

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.

kontrollanten commented 7 years ago

You're a hero, @shellscape

hypescript commented 6 years ago

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 commented 6 years ago

@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.

LukasBombach commented 6 years ago

At least it works for now, thanks @shellscape