Migrate enzyme to testing library #586
Conversation
* Migrate paginator * Add comment * Remove unused import * Migrate tab-bar
* Initial commit * Clean up logs * Add tests more default close interactions
* Migrate loading container * Migrate flash message * Migrate flash message container * Upgdate assertion
| <button>Hi</button> | ||
| </ButtonArea> | ||
| ) | ||
| expect(screen.getByRole('button').closest('div')).toHaveClass('extra classes') |
test/forms/buttons/button.test.js
Outdated
| const wrapper = shallow(<Button submitting={true}> Hi</Button>) | ||
| expect(wrapper.hasClass('in-progress')).toBe(true) | ||
| render(<Button submitting={true}> Hi</Button>) | ||
| expect(screen.getByRole('button').getAttribute("class")).toContain("in-progress") |
| @@ -1,7 +1,7 @@ | |||
| import { castArray, has } from '../../utils' | |||
|
|
|||
| // Get column info from children via props | |||
| function getColumnData(children = [], doDisable) { | |||
| function getColumnData(children, doDisable) { | |||
There was a problem hiding this comment.
This was only called in sortable-table, whose prop types require children. We were getting a flag for missing a branch here, but in reality this isn't a valid use of this util.
|
@chawes13 I just reviewed your latest changes, looking good. Mocking |
| @@ -81,6 +88,7 @@ function getAdjacentControl(control, { previous = false } = {}) { | |||
|
|
|||
| // Recursively searches for the closest parent tab list | |||
| function getClosestTabList(el) { | |||
| /* istanbul ignore next */ | |||
There was a problem hiding this comment.
This isn't possible to test without changing the markup of the component. If this situation does occur, we'll get an error on undefined.querySelectorAll('[role="tab"]')).
However, it still feels good to me to have this guard in a recursive function? Let me know if you think we should remove it outright
|
@josiasds Ready for ya! We got to 100% coverage 😎 |
| render(<WrappedDropdownCheckboxGroup value={['1']} />) | ||
|
|
||
| const select = screen.getByRole('button') | ||
| await act(() => user.click(select)) |
There was a problem hiding this comment.
Why is act necessary here?
There was a problem hiding this comment.
My understanding is that any code that relies on asserting on state changes within a component should be wrapped in act. The button's nextSibling will only have the is-active class applied if the internal expanded state changes
There was a problem hiding this comment.
Hmm, it looks like all events within React's callstack should be covered, so maybe I don't need it here!
https://kentcdodds.com/blog/fix-the-not-wrapped-in-act-warning
Additionally, it's possible that instead of calling act directly, I should be using waitFor (https://testing-library.com/docs/react-testing-library/cheatsheet/#async). What do you think?
| @@ -1,5 +1,5 @@ | |||
| import React from 'react' | |||
| import { render, screen } from '@testing-library/react' | |||
| import { render, screen, fireEvent } from '@testing-library/react' | |||
There was a problem hiding this comment.
Any reason to prefer fireEvent over the userEvent lib here?
There was a problem hiding this comment.
Yes, from what I could tell the keyboard command only works on focusable elements and there are none other than the tabs themselves. Most of the tests that use fireEvent needed an event to be targeted at an element that was not a tab (e.g., pressing directional key when not focused on a tab does not change focus). Lastly, one of the tests required key to be Unidentified, which I wasn't able to replicate with any known keyboard commands (so I used { code: 'Home' } with fireEvent instead).
It's a little bit of an escape hatch – I used keyboard when possible.
This PR contains the migration from
enzymetoreact-testing-library, as requested in #567.From the docs:
Release Notes
Author Checklist
Enzyme(and adapter) libs and setupComponents to migrate:
/controls(RTL migration: controls #602)paginatorcolor-picker(depends on Routine maintenance 2023-08 #596)tab-bar/forms/buttons(in review in this PR Migrate enzyme to testing library #586)button-areabuttonsubmit-button/labels(RTL migration: labels #607)error-labelinput-errorinput-labellabeled-field/inputscloudinary-uploadercheckbox-groupcheckboxcloudinary-file-input(RTL migration: file inputs #590)color-inputdate-inputdropdown-checkbox-groupfile-input(RTL migration: file inputs #590)hidden-inputicon-inputinputmasked-inputradio-grouprange-inputselectswitchtextarea/indicators(RTL migration: indicators #600)flash-message-containerflash-messageloading-containerspinner(in review in this PR)/tables(RTL migration: tables #603)sortable-tabletable-columntablemodal(RTL migration: modal #601)