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

Update for Chai v4 #22

Closed
wants to merge 1 commit into from
Closed

Update for Chai v4 #22

wants to merge 1 commit into from

Conversation

meeber
Copy link
Contributor

@meeber meeber commented Dec 18, 2016

DO NOT MERGE. Chai v4 isn't released yet, version numbers in package.json still need updating.

I'm just going through some popular Chai plugins to see how much breakage there is when used with the upcoming Chai v4 release. It doesn't look like it breaks any of dirty-chai's functionality, but it does cause two tests to fail. I don't think these failed tests will impact end users of dirty-chai; they are specific to how assertions were being performed on actual Chai Assertion objects.

The test on line 127 fails for two reasons:

  1. The property assertion is actually triggering the getter for chai.Assertion.prototype['neverFail'], which is causing a __flag property to be created directly on chai.Assertion.prototype when setting the defer flag, which is in turn wreaking havoc on all future assertions. (This was happening in v3.5 too, just not with the same consequences).
  2. in Chai v4, assertions return a new Assertion object with the flags transferred over. Therefore, the second should switches the context to the new Assertion object instead of the neverFail function, causing the assertion to fail.

The test on line 129 fails because the new chai.Assertion({}) that gets passed to the neverFail getter via call is actually a proxified Assertion object, which then gets double-wrapped inside another proxy inside of the addChainableMethod function. Therefore, when should is called, it gets called twice (the second time is from within the proxy getter of the first call), and the end result is that the context is the Assertion object instead of the neverFail function.

This PR refactors the tests slightly so that they work in both chai v3.5 and chai v4.0

@joshperry
Copy link
Contributor

Thank you for taking the time to go through this, @meeber! I'll track this with the chai v4 release.

@meeber
Copy link
Contributor Author

meeber commented Apr 23, 2017

DO NOT MERGE. Chai v4 isn't released yet, version numbers in package.json still need updating.

The second canary release of Chai v4 caused an additional test to fail. The test failed because of a bad interaction between .bind and the new Chai "Length Guard". The Length Guard protects developers against accidentally chaining .length off of an uninvoked method or chainable method assertion, since doing so references the function's built-in .length property instead of Chai's .length assertion. Unfortunately, .bind calls .length internally on the bind target, so if .bind is used with a Chai Assertion object, it's going to trigger the Length Guard.

In the failing test, .bind was used with a non-invoked method Assertion object (specifically, .equal). I refactored the test to use .call instead, and the test now passes.

@layershifter
Copy link

chai@4.0.0 was released 👍

@meeber
Copy link
Contributor Author

meeber commented May 26, 2017

I just took a look at this. I believe chaijs/chai-as-promised#157 needs to get merged (and a new version of chai-as-promised released) before the Chai@4 devdependency jungle can be navigated for this lib.

@bkw
Copy link

bkw commented Jun 12, 2017

Version 7.0 of domenic/chai-as-promised with support for chai@4 has been released.

@joshperry
Copy link
Contributor

Update for chai 4 has been pushed in version 2.0.0. Thank you for the help and prodding!

@joshperry joshperry closed this Jun 12, 2017
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

Successfully merging this pull request may close these issues.

4 participants