Upgrade some dependencies: React 16, Enzyme 3 and Jest#2813
Upgrade some dependencies: React 16, Enzyme 3 and Jest#2813youknowriad wants to merge 3 commits intomasterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2813 +/- ##
==========================================
- Coverage 33.81% 33.73% -0.08%
==========================================
Files 190 190
Lines 5678 5676 -2
Branches 992 991 -1
==========================================
- Hits 1920 1915 -5
- Misses 3181 3183 +2
- Partials 577 578 +1
Continue to review full report at Codecov.
|
|
@gziolo If you have some ideas on how to fix these skipped tests, that would be awesome. |
|
|
||
| it( 'should render into provider context', () => { | ||
| // skipped because it's not working with React/Enzyme upgrade | ||
| it.skip( 'should render into provider context', () => { |
There was a problem hiding this comment.
I think we need to wait until Support Portals properly is resolved :)
There was a problem hiding this comment.
Do you think this should block the PR? thoughts @aduth
There was a problem hiding this comment.
I don't consider it as a blocker. I should phrase it explicitly. Just saying we would have to fix Enzyme in the first place or keep the shim that was used before.
| }; | ||
|
|
||
| describe( 'InserterMenu', () => { | ||
| describe.skip( 'InserterMenu', () => { |
There was a problem hiding this comment.
I did some heave debugging here and it looks like we are having the same kind of issues that are reported in Enzyme repository here and here.
In our case it might be a mix of those two. I tried to add recommended wrapper.update(); call after the click event is simulated, but it didn't help. It's quite interesting scenario because the part of the component that depends on the component's state (
gutenberg/editor/inserter/menu.js
Line 384 in ad316ab
gutenberg/editor/inserter/menu.js
Line 366 in ad316ab
wrapper.debug() call. When I debugged it turned out that React component does everything properly behind the scenes. It looks like Enzyme gets out of sync.
|
I would wait a week or two with this PR. Mostly because there is one known issue which doesn't work anymore with Enzyme 3 reported in 2 places enzymejs/enzyme#1153 and enzymejs/enzyme#1163 and 2 React 16 features are not implemented yet: enzymejs/enzyme#1149 (Array-rendering components) and enzymejs/enzyme#1150 (Portals - which we use). |
|
They published enzyme 3.1.0 and enzyme-adapter-react-16 1.0.1, but it makes even more tests to fail. I spent maybe 15 minutes trying to fix them, but I didn't have enough courage to get into debug mode :) |
|
@gziolo Should we close this for now. We can revisit in some weeks |
|
Yes, it's a very good idea :) |
Checklist:
The trickiest part here is the Enzyme and React upgrade in Unit tests. I think we should run the same version we use on Prod on our unit tests. Also, Enzyme 3 comes with some annoying breaking changes.
This change has the side effect of showing some warnings on
npm installbecause some React libraries do not support React 16. I haven't checked yet if there are newer versions for these dependencies (will do). I personally the warning is worth seeing to remind us what we need to update.There are still two unit tests I'm not able to fix with Enzyme3 (the one marked as skipped)