-
Notifications
You must be signed in to change notification settings - Fork 2
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
Migrate enzyme tests to testing-library #52
Changes from all commits
03afac5
db3101c
4fc7c40
5ecc0dc
c16bd75
48fe4b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,24 +25,29 @@ export default class AbstractMenu extends Component { | |
return; | ||
} | ||
|
||
switch (e.keyCode) { | ||
case 37: // left arrow | ||
case 27: // escape | ||
switch (e.key) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The user-event library doesn't use See https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key/Key_Values for the values for this property. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Works for me. I'm not really worried about IE, but good to include them too :) |
||
case 'ArrowLeft': // left arrow | ||
case 'Left': // IE specific value | ||
case 'Escape': // escape | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess some of the comments are now redundant, but I don't mind keeping them. |
||
case 'Esc': // IE specific value | ||
e.preventDefault(); | ||
this.hideMenu(e); | ||
break; | ||
case 38: // up arrow | ||
case 'ArrowUp': // up arrow | ||
case 'Up': // IE specific value | ||
e.preventDefault(); | ||
this.selectChildren(true); | ||
break; | ||
case 40: // down arrow | ||
case 'ArrowDown': // down arrow | ||
case 'Down': // IE specific value | ||
e.preventDefault(); | ||
this.selectChildren(false); | ||
break; | ||
case 39: // right arrow | ||
case 'ArrowRight': // right arrow | ||
case 'Right': // IE specific value | ||
this.tryToOpenSubMenu(e); | ||
break; | ||
case 13: // enter | ||
case 'Enter': // enter | ||
e.preventDefault(); | ||
this.tryToOpenSubMenu(e); | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,17 @@ | ||
--- | ||
env: | ||
jest: true | ||
rules: | ||
prefer-arrow-callback: 0 | ||
no-mixed-requires: 0 | ||
plugins: | ||
- jest | ||
- jest-dom | ||
- testing-library | ||
extends: | ||
- plugin:jest/recommended | ||
- plugin:jest-dom/recommended | ||
- plugin:testing-library/react | ||
env: | ||
jest: true | ||
rules: | ||
prefer-arrow-callback: 0 | ||
no-mixed-requires: 0 | ||
# This disallows using direct Node properties (eg: firstChild), but we have | ||
# legitimate uses: | ||
testing-library/no-node-access: 0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,8 @@ | ||
const jsdom = require('jsdom'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This commit has the bulk of the changes. The test files aren't perfect but good enough to move us forward. In the future we can try to improve the coverage. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I feel like these tests are really not enough. But this is something for the future. |
||
// Importing this here makes it work everywhere. | ||
import '@testing-library/jest-dom'; | ||
|
||
const documentHTML = '<!doctype html><html><body><div id="root"></div></body></html>'; | ||
const dom = new jsdom.JSDOM(documentHTML); | ||
global.document = dom.window.document; | ||
global.window = dom.window; | ||
global.window.resizeTo = (width, height) => { | ||
global.window.innerWidth = width || global.window.innerWidth; | ||
global.window.innerHeight = width || global.window.innerHeight; | ||
global.window.dispatchEvent(new Event('resize')); | ||
window.resizeTo = (width, height) => { | ||
window.innerWidth = width || window.innerWidth; | ||
window.innerHeight = width || window.innerHeight; | ||
window.dispatchEvent(new Event('resize')); | ||
}; | ||
global.window.requestAnimationFrame = jest.fn(); | ||
|
||
const Enzyme = require('enzyme'); | ||
const Adapter = require('@wojtekmaj/enzyme-adapter-react-17'); | ||
|
||
Enzyme.configure({ adapter: new Adapter() }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curiously the test files weren't checked by eslint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, interesting. Thanks for adding them!