Plugin issues with Chai v4

[meeber]

I went through the popular Chai plugins to see how their test suites fair with Chai v4. (Actually, I used the #884 build instead of the current canary release). Overall, we're in good shape. Here are my findings:

1. chai-immutable
2. chai-jquery
3. chai-react

Major issues

• chai-immutable

  • Code had to be updated because of breaking change of deep.property to nested.property. Also because of change in capitalization in values returned by type-detect. Also because .not.keys was being treated implicitly as .not.any.keys instead of .not.all.keys. Further changes are likely needed due to change in not.property.
  • Updated code only works in Chai v4.0; therefore, these are breaking changes for end users
  • Tests had to be updated accordingly to change deep.property to nested.property, and to remove some of the .not.keys tests since they no longer made sense
  • Updated tests only work in Chai v4.0
  • PR submitted (Update for Chai v4.0 astorije/chai-immutable#69) but shouldn't be merged because there's more work to be done, and it shouldn't be merged until Chai v4 is released; package.json needs updating

• chai-things

  • Code was already broken prior to v4 due to any flag in Chai core
  • Tests had to be updated because of changed wording in failed property assertion
  • Updated tests only work in Chai v4.0; still one broken test because of any flag in core
  • No PR submitted yet because of major any issue
  • Independently of this, I have an open issue (Issues with contains, any, and all #881) about possibly deprecating any in core, which would happen to resolve this problem

Minor issues

• chai-as-promised

  • Code had to be updated because in Chai v4, assertions return new Assertion objects with flags transferred over, and the promise was getting lost as a result
  • Updated code still works in Chai v3.5
  • One test had to be updated to change deep.property to nested.property
  • Updated test only works in Chai v4.0
  • PR submitted (Support Chai@4 - Fix assertions broken by Chai 4.0 chai-as-promised#157) but shouldn't be merged until Chai v4 is released; package.json needs updating

• chai-jquery

  • Code had to be updated because in Chai v4, if undefined is explicitly passed as the second parameter to the property assertion, it asserts that the value value of the property equals undefined.
  • Updated code still works in Chai v3.5
  • Four tests had to be updated due to small change in failed assertion message from "have a property" to "have property"
  • PR submitted (Update for Chai v4.0 chai-jquery#105) and can be merged now

• chai-http

  • No code had to be updated
  • Tests had to be updated to change deep.property to nested.property
  • Updated tests only work in Chai v4.0
  • PR submitted (Update for Chai v4.0 chai-http#131) but shouldn't be merged until Chai v4 is released; package.json needs updating

• dirty-chai

  • No code had to be updated
  • Three tests had to be updated because they were asserting on Chai Assertion objects in a way that caused problems
  • Updated tests still work in Chai v3.5
  • PR submitted (Update for Chai v4 prodatakey/dirty-chai#22) but shouldn't be merged until Chai v4 is released; package.json needs updating

• chai-timers

  • No code had to be updated
  • One test had to be fixed because above no longer accepts strings.
  • Updated tests still work in Chai v3.5
  • PR submitted (Fix broken test in Chai v4 chai-timers#14) and can be merged now

No issues

• sinon-chai
• chai-enzyme
• chai-webdriver
• chai-spies
• chai-json-schema
• chai-react
• chai-backbone
• chai-subset
• jsx-chai
• chai-fs
• chai-stats
• chai-null
• chai-factories
• chai-change

[jeromemacias]

Can you also include https://github.com/producthunt/chai-enzyme please ?

[meeber]

@jeromemacias Thanks for the heads up! Just ran the test suite with Chai v4; no issues.

[lucasfcosta]

Hey friends! So, is anything left here @meeber?
We were thinking about releasing 4.0.0 and it would be good to avoid issues with any plugins.

All we've gotta do is make sure those PRs are merged when 4.0.0 gets released right?

[meeber]

There have been a number of commits in master since I tested these plugins. And we have a couple more on the way. I don't anticipate any new issues, but will need to test them again with the updated code for due diligence.

Assuming there aren't any additional issues: The plugins listed under "minor issues" already either have a PR ready to merge, or a PR that just needs the version updated once 4.0 is release, and then it can be merged. The plugins listed under "major issues" need more work. @astorije is already aware of some changes that impact chai-immutable. chai-things is broken since before v3.5 due to a conflict with Chai core regarding the any flag; I don't think it's essential that we fix this prior to v4.0 release, but it's something we should look into going forward.

[keithamus]

Development on chai-things has mostly stalled - in the current situation the featureset that chai-things really needs is not well catered for with chai's current plugin system. The proposed new system in #585 takes plugins like chai-things into account (hopefully).

[meeber]

@keithamus @vieiralucas @lucasfcosta @shvaikalesh @astorije I ran the test suites for all of these plugins using the second canary release. There were a couple of new small issues in dirty-chai and chai-query; I updated the PRs for those already. There was also a new breaking change in chai-immutable due to #924; I updated the PR for that as well.

Finally, there's a new issue related to #900 that's causing three test suites to fail. Haven't had a chance to look into it in-depth, but it appears that the issue is related to mocha-phantomjs choking on these lines. Specifically, Object.getOwnPropertyNames(testFn) is including callee in the result set, but then Object.getOwnPropertyDescriptor(testFn, name) is returning undefined when name is callee. That seems very odd; more investigation is needed. The affected repos are:

1. chai-immutable
2. chai-react
3. chai-jquery

[meeber]

@keithamus @vieiralucas @lucasfcosta @shvaikalesh As mentioned above, several plugin test suites are failing in PhantomJS because of these lines:

var excludeNames = Object.getOwnPropertyNames(testFn).filter(function(name) {
  return !Object.getOwnPropertyDescriptor(testFn, name).configurable;
});

I won't have enough cycles to troubleshoot this in depth for a couple weeks. If no one else does either, then I suggest we hack-fix it by checking that the return value of Object.getOwnPropertyDescriptor(testFn, name) is defined before accessing its .configurable property. Normally this shouldn't be necessary but something funky is clearly going on here; this hack should at least let the tests pass until we have a chance to investigate further.

[meeber]

Per #966, the PhantomJS 1.x issue is now resolved.

[meeber]

• I updated the PRs for chai-immutable, chai-as-promised, chai-jquery, and chai-http.
• Due to dev dependency conflicts, the PR for dirty-chai can't be updated until chai-as-promised releases a new version.
• The PR for chai-timers was previously merged.
• chai-things is still broken due to major issues that predate v4.0.

[keithamus]

I feel as though chai-things will pretty much have to wait until #585 before it can get the abilities it needs from chai (and also the attention).

[meeber]

When checking for compatibility with Chai v4, I was using each plugin's master branch. Per chaijs/chai-spies#71, the current npm release of chai-spies is incompatible with Chai v4; we need to prioritize releasing a new version of that.

[lucasfcosta]

@meeber but according to what I've seen at that issue, chai-spies master branch is already compatible with Chai v4, right?

Also, I think that we should focus on releasing chai-as-promised too. I've been getting lots of requests to do that and I think that plugin is pretty important for our users.

I'm planning on releasing the v4 compatible versions of our plugins this sunday. If anyone wants to schedule a hangouts or a skype call so that we can get these done it would be awesome.

Also, I remember @keithamus sent us a link with instructions for releasing modules but I can't find it. It would be great if we could have that in order to get this going.

[meeber]

@lucasfcosta Right!

[sverweij]

There is a (easily solvable, but annoying) issue with chai-json-schema using chai@4 on node@4 - see chaijs/chai-json-schema#47