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

4.x.x Complete Migration Guide #781

Closed
lucasfcosta opened this issue Sep 3, 2016 · 14 comments
Closed

4.x.x Complete Migration Guide #781

lucasfcosta opened this issue Sep 3, 2016 · 14 comments
Assignees
Labels
Milestone

Comments

@lucasfcosta
Copy link
Member

lucasfcosta commented Sep 3, 2016

4.x.x Complete Migration Guide

Hello everyone! I'm writing this to serve as a guide for the 4.x.x version.

To make this as complete as possible I'll include here every single new feature, breaking change and fix and their respective issues and Pull Requests. Bugs introduced and solved after the 3.5.0 released won't be here, since it does not matter for the new version.

I'll also add some examples on how to migrate old code or to use new features when talking about these kinds of changes.

Please let me know if I've forgotten anything 😄


Breaking Changes

// This will not work anymore because there is no benefit to chaining these assertions:
expect(function() {}).to.change.by(2)
expect(function() {}).to.increase.by(2)
expect(function() {}).to.decrease.by(2)

New Features

// You can use `.by` to assert the amount you want something to change
var obj = { val: 10 };
var increaseByTwo = function() { obj.val += 2 };
var decreaseByTwo = function() { obj.val -= 2 };
var increaseByThree = function() { obj.val += 3 };

expect(increaseByThree).to.change(obj, 'val').by(3);
expect(increaseByTwo).to.increase(obj, 'val').by(2);
expect(decreaseByTwo).to.decrease(obj, 'val').by(2);

// Please notice that if you want to assert something did change but not by some amount you need to use `.not` **after** the `change` related assertion
// Take a look at the examples below:
expect(increaseByThree).to.change(obj, 'val').but.not.by(5)
expect(increaseByTwo).to.increase(obj, 'val').but.not.by(1)
expect(decreaseByTwo).to.decrease(obj, 'val').but.not.by(1)
// The `.keys` assertion now works on `map`s and `set`s natively, like the examples below:
expect(new Map([[{objKey: 'value'}, 'value'], [1, 2]])).to.contain.key({objKey: 'value'});
expect(new Map([[{objKey: 'value'}, 'value'], [1, 2]])).to.contain.any.keys([{objKey: 'value'}, {anotherKey: 'anotherValue'}]);
expect(new Map([['firstKey', 'firstValue'], [1, 2]])).to.contain.all.keys('firstKey', 1);
expect(new Set([['foo', 'bar'], ['example', 1]])).to.have.any.keys('foo');

// You can also use `.deep` when asserting agains `Map`s and `Set`s
expect(new Map([[{objKey: 'value'}, 'value'], [1, 2]])).to.contain.any.deep.keys([{objKey: 'value'}, {anotherKey: 'anotherValue'}]);
expect(new Map([['firstKey', 'firstValue'], [1, 2]])).to.contain.all.deep.keys('firstKey', 1);
expect(new Set([['foo', 'bar'], ['example', 1]])).to.have.any.deep.keys('foo');
require('chai/register-assert');  // Using Assert style

Bug Fixes

@lucasfcosta
Copy link
Member Author

lucasfcosta commented Sep 3, 2016

I accidentally sent this issue while editing it.
This is a work in progress.


Current Status: This issue has just been updated (Sept. 6, 2016 - 10:13PM GMT-3) with every commit on the master branch. I'm now going to start adding the changes from the 4.x.x branch.

Since we've got a holiday in Brazil this Wednesday, I may be finishing this before the weekend.
I'm keeping this file on vIM, I'll make sure to edit this issue again with the whole content when I'm done covering every commit.

@lucasfcosta lucasfcosta changed the title 4.x.x Complete Migration Guide [WIP] 4.x.x Complete Migration Guide Sep 3, 2016
@keithamus
Copy link
Member

Don't forget to check out the draft 4.0.0 release notes @lucasfcosta - I've tried to keep them relatively up to date, although lately I've missed quite a few PRs.

@lucasfcosta
Copy link
Member Author

lucasfcosta commented Sep 5, 2016

@keithamus thanks for the link! I'll take a look at it.
I'll try to keep this issue as complete as possible so we will be able to have a detailed guide if users want more information about any of the topics listed and then we can just copy and paste some info from here and add it to the release notes (with some formatting fixes of course).

Currently I'm using gitk and seeing each single commit since the 3.5.0 tag both on the master and 4.x.x branches.

@keithamus keithamus modified the milestone: 4.0 Sep 6, 2016
@lucasfcosta lucasfcosta changed the title [WIP] 4.x.x Complete Migration Guide 4.x.x Complete Migration Guide Oct 2, 2016
@lucasfcosta
Copy link
Member Author

lucasfcosta commented Oct 2, 2016

This is done! 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉
Now the migration guide has every change on the master and 4.x.x branch documented until today (Oct, 2nd, 2016).

Special thanks to @vieiralucas for helping me finish this.

PARTY HARD

@keithamus
Copy link
Member

Thanks for this amazing work @lucasfcosta

@meeber
Copy link
Contributor

meeber commented Oct 4, 2016

Will try to review after work!

@meeber
Copy link
Contributor

meeber commented Oct 4, 2016

@lucasfcosta Thank you for the enormous effort in tracking all of this down and organizing it! Also thank you for the dancing pikachu.

A quick review yielded the following:

  • I think breaking changes should be listed before new features.
  • In the list of new features, I think the proxy stuff should be listed first as I believe it addresses Chai's most complained about issue.
  • Add script that registers should as a side-effect: Given mocha's popularity, it might be worth noting that this change allows you to register should via a mocha option: mocha --require chai/should.
  • The change assertion accepts a function as objec: The "t" is missing in object. Also this needs to be explained a bit more with clearer examples. It also needs to mention that change, increase, and decrease all have this new functionality. Finally, I think we need to update the actual jsdoc for change because, unlike increase and decrease, it doesn't mention this new feature.
  • The you can also assert for a delta using the by assertion alongside change, increase and decrease assertion: Grammar issue going on here.
  • The .keys assertion can now operate on maps and set: Nitpick, but I'd say either don't pluralize "maps" or do pluralize "set". I prefer the first option.
  • Allow use to be imported using new ES6 module syntax: It might be worth noting and exemplifying that this can also be done with require thanks to object destructuring assignment: const {expect, use} = require("chai");
  • This change causes the .include assertion to always use strict equality: Suggest rephrasing this to something like "This change causes the .include assertion to always use strict equality unless the deep flag is set".
  • Suggest changing "While the old one throwed up false positives" to "While the old one threw false positives".
  • Fix stacktraces for overwritten properties and method: Method should be plural.
  • Fix bug when testing Symbol equality with should synta: What happened to that poor "x"??
  • Fix bug when asserting some valid ES6 key: Key should be plural.
  • Fix ownProperty on objects with no prototyp: What happened to that poor "e"??

@lucasfcosta
Copy link
Member Author

lucasfcosta commented Oct 4, 2016

@meeber thanks for the review!
Actually lots of last characters ended up being replaced by a . due to my badly applied regex skills for auto-replace, so lots of those issues come from that.
I'll fix those ASAP!

EDIT: Fixed! Thanks for the help Mr. @meeber, your feedback is always great and constructive!

@meeber
Copy link
Contributor

meeber commented Oct 19, 2016

Doing a last review here, noticed a couple things:

  • "Please notice that the old the old methods" (typo)
  • "The assert.propertyNotVal and assert.deepPropertyNotVal assertions were renamed to assert.notPropertyVal and assert.deepPropertyNotVal" (the final assert.deepPropertyNotVal should be assert.notDeepPropertyVal)
  • "expect([1, 2, 3]).to.include.ordered.members([2, 3]); // This will! Read the docs to know more." (missing "fail" in comment)

@lucasfcosta
Copy link
Member Author

lucasfcosta commented Oct 20, 2016

@meeber done!
Thank you for the review, it's always good to have your eyes on the things I write.
If anyone finds anything else that should be changed, let me know 😄

Also, we could also a link on the README for the ones interested into the migration guide, I think it would be useful, what do you guys think?

@inancgumus
Copy link

inancgumus commented Nov 18, 2016

Hi @lucasfcosta You may want to update for this PR too: #868 and #872

@lucasfcosta
Copy link
Member Author

lucasfcosta commented Nov 18, 2016

Hi @DeeperX, thanks for your input.
This is the migration guide for 4.0.0 only, which has already been released under the canary tag.
I think that changes made after that version should be within release logs, not in this guide. However, if the rest of the team finds it suitable we may add it here.

@lucasfcosta
Copy link
Member Author

Updated today (May, 9th - 2017) with all the changes included in the 4.0.0 release.

We're finally ready 🎉 🎉 🎉 🎉 🎉 🎉 🎉

@keithamus
Copy link
Member

I've moved this guide into https://github.com/chaijs/chai/releases/tag/4.0.0, this means it'll show up the site by the next rebuild I think. Thanks so much @lucasfcosta for your awesome work here!

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

No branches or pull requests

4 participants