feat: replace use of enzyme with react-testing-library #1144

ryan-m-walker · 2019-12-21T03:11:19Z

added @testing-library/react to graphiql package
added @testing-libary/jest-dom to graphiql package
removed usage of ENZYME env variable in test script in graphiql packages
removed enzyme adapter setup from enzyme.config.js file and renamed it
to test.config.js
removed use of enzyme and replaced with react-testing-library in all
graphiql package test files and updated tests to use
react-testing-library patterns
moved mockStorage to own file so it can be reused in other test files
and added other methods to it
added a CodeMirror mock to be used in tests
mocked out codemirror addon modules

completes: https://github.com/graphql/graphiql/projects/4#card-30693199

This PR gets rid off all uses of Enzyme in the GraphiQL package and replaces them with @testing-libary/react.

codecov · 2019-12-21T03:19:17Z

Codecov Report

Merging #1144 into master will increase coverage by 2.46%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1144      +/-   ##
==========================================
+ Coverage   46.22%   48.68%   +2.46%     
==========================================
  Files          64       64              
  Lines        3003     3003              
  Branches      650      650              
==========================================
+ Hits         1388     1462      +74     
+ Misses       1363     1306      -57     
+ Partials      252      235      -17

Impacted Files	Coverage Δ
packages/graphiql/src/utility/QueryStore.js	`54.54% <0%> (-3.04%)`	⬇️
packages/graphiql/src/components/ResultViewer.js	`71.42% <0%> (+2.85%)`	⬆️
packages/graphiql/src/components/QueryEditor.js	`64.28% <0%> (+5.95%)`	⬆️
packages/graphiql/src/components/VariableEditor.js	`65.07% <0%> (+6.34%)`	⬆️
packages/graphiql/src/components/GraphiQL.js	`50.39% <0%> (+11.4%)`	⬆️
...es/graphiql/src/components/DocExplorer/FieldDoc.js	`100% <0%> (+14.28%)`	⬆️
packages/graphiql/src/utility/getQueryFacts.js	`95.23% <0%> (+28.57%)`	⬆️
packages/graphiql/src/utility/elementPosition.js	`33.33% <0%> (+33.33%)`	⬆️
...s/graphiql/src/utility/getSelectedOperationName.js	`81.81% <0%> (+63.63%)`	⬆️
packages/graphiql/src/utility/debounce.js	`100% <0%> (+80%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5765394...817d304. Read the comment docs.

acao · 2019-12-21T20:53:51Z

@ryan-m-walker ok so i merged the react 16 upgrade PR, so you'll just need to rebase it looks like?

ryan-m-walker · 2019-12-22T16:21:42Z

@acao ok, I rebased

acao · 2019-12-22T16:22:18Z

@ryan-m-walker nice that was quick, thank you!

acao · 2019-12-23T13:36:21Z

oh drat, i just merged the storybooks PR and so another conflict here

ryan-m-walker · 2019-12-23T14:40:30Z

@acao no worries, i'll try to fix later tonight

ryan-m-walker · 2019-12-23T15:03:22Z

@acao there's a minor change I want to make anyways

ryan-m-walker · 2019-12-23T23:36:05Z

@acao should be good now

- added @testing-library/react to graphiql package - added @testing-libary/jest-dom to graphiql package - removed usage of ENZYM env variable in test script in graphiql packaget - removed enzyme adapter setup from enzyme.config.js file and renamed it to test.config.js - removed use of enzyme and replaced with react-testing-library in all graphiql package test files and updated tests to use react-testing-library patterns - moved mockStorage to own file so it can be reused in other test files and added other methods to it - added a CodeMirror mock to be used in tests - mocked out codemirror addon modules - moved QueryHistory tests from own file into GraphiQL.spec.js file

acao · 2019-12-26T07:58:12Z

packages/graphiql/src/components/__tests__/helpers/codeMirror.js

@@ -0,0 +1,81 @@
+export function MockCodeMirror(node, { value, ...options }) {
+  let _eventListeners = {};
+  const mockTextArea = document.createElement('textarea');


I don't think it matters now but it may eventually, but our instantiation pattern for GraphiQL query editor is a bit different from the generic Codemirror example that tends to use<textarea />.

We actually end up using a <section />:

Codemirror Invocation

this_node attached via old ref pattern from <section /> element

we target a <section /> to instantiate CodeMirror but code mirror renders some more elements including a <textarea /> inside that root node:

they handle the typing and presentation in a pretty complex way it seems, where they use the type area as a temporary moving cursor and then render out typed text in some presentational elements. i figured for our testing purposes it'd just be easier to use a base text area and the abstracted mocked internals of CodeMirror for our testing purposes and keep the API our code interacts with the same. If you have a better idea let me know though.

indeed yes! just pointing out there will be a subtle difference in the DOM hierarchy, but I think the only delta would be caused by container targets for full-height container

either way this is really great, and we can use this mock for codemirror-graphql I hope, which we still need to convert from mocha to jest

gotcha, yeah this might need some changes as we go. I already added a few more elements to the mock CodeMirror DOM in another PR that uses this branch as its base because it need to listen for click drag events on the gutter bar.

packages/graphiql/package.json

acao · 2019-12-26T17:05:19Z

packages/graphiql/src/components/DocExplorer/__tests__/FieldDoc.spec.js

-    expect(W.find('MarkdownContent').text()).toEqual('No Description\n');
-    expect(W.find('TypeLink').text()).toEqual('String');
-    expect(W.find('Argument').length).toEqual(0);
+    expect(container.querySelector('.doc-type-description')).toHaveTextContent(


I'm worried about how stable it will be using classes instead of component names as before, especially since we are about to change how layout/theming works and use theme-ui/emotion/etc

hmm yeah, I wasn't to keen on using querySelector but I was trying to avoid making any changes to non-test code. Do you think it's worth adding either labels or testids to the components? We mostly just use a lot of classes in these components and react-testing-library doesn't let you target react implementation code

packages/graphiql/src/components/__tests__/helpers/codeMirror.js

- removed `cross-env` from test script - moved CodeMirror mock to __mock__ file

ryan-m-walker marked this pull request as ready for review December 21, 2019 03:11

ryan-m-walker force-pushed the feat/react-testing-libary branch 2 times, most recently from 40aa663 to e842329 Compare December 22, 2019 16:20

ryan-m-walker force-pushed the feat/react-testing-libary branch from e842329 to 785ac99 Compare December 22, 2019 17:54

ryan-m-walker force-pushed the feat/react-testing-libary branch from 785ac99 to 2a6e4d4 Compare December 23, 2019 23:34

ryan-m-walker force-pushed the feat/react-testing-libary branch 2 times, most recently from 96434cf to 1c95b6d Compare December 24, 2019 22:32

ryan-m-walker force-pushed the feat/react-testing-libary branch from 1c95b6d to 3c4ee46 Compare December 25, 2019 00:17

acao reviewed Dec 26, 2019

View reviewed changes

packages/graphiql/package.json Outdated Show resolved Hide resolved

acao reviewed Dec 26, 2019

View reviewed changes

packages/graphiql/src/components/__tests__/helpers/codeMirror.js Outdated Show resolved Hide resolved

ryan-m-walker and others added 3 commits December 26, 2019 13:47

feat: replace use of enzyme with react-testing-library

e3ffa8b

- removed `cross-env` from test script - moved CodeMirror mock to __mock__ file

Merge branch 'master' into feat/react-testing-libary

017695a

Merge branch 'master' into feat/react-testing-libary

817d304

acao merged commit de73d6c into graphql:master Dec 27, 2019

acao added this to the GraphiQL-1.0.0-beta milestone Mar 14, 2020

feat: replace use of enzyme with react-testing-library #1144

feat: replace use of enzyme with react-testing-library #1144

Uh oh!

Conversation

ryan-m-walker commented Dec 21, 2019 • edited by acao Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Dec 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

acao commented Dec 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryan-m-walker commented Dec 22, 2019

Uh oh!

acao commented Dec 22, 2019

Uh oh!

acao commented Dec 23, 2019

Uh oh!

ryan-m-walker commented Dec 23, 2019

Uh oh!

ryan-m-walker commented Dec 23, 2019

Uh oh!

ryan-m-walker commented Dec 23, 2019

Uh oh!

acao Dec 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryan-m-walker Dec 26, 2019

Choose a reason for hiding this comment

Uh oh!

acao Dec 26, 2019

Choose a reason for hiding this comment

Uh oh!

acao Dec 26, 2019

Choose a reason for hiding this comment

Uh oh!

ryan-m-walker Dec 26, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

acao Dec 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryan-m-walker Dec 26, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ryan-m-walker commented Dec 21, 2019 •

edited by acao

Loading

codecov bot commented Dec 21, 2019 •

edited

Loading

acao commented Dec 21, 2019 •

edited

Loading

acao Dec 26, 2019 •

edited

Loading

acao Dec 26, 2019 •

edited

Loading