-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Components: Refactor AlignmentMatrixControl
tests to use @testing-library/react
#44670
Conversation
…rary/react render
I started refactoring this test last night mostly for fun. Here's my result: describe( 'AlignmentMatrixControl', () => {
it( 'should render', () => {
render( <AlignmentMatrixControl /> );
expect( screen.getByRole( 'grid' ) ).toBeInTheDocument();
} );
it( 'should change value on cell click', async () => {
const onChangeSpy = jest.fn();
render(
<AlignmentMatrixControl
value={ 'center' }
onChange={ onChangeSpy }
/>
);
screen
.getByRole( 'gridcell', {
name: 'center left',
} )
.focus();
expect( onChangeSpy ).toHaveBeenCalledWith( 'center left' );
screen
.getByRole( 'gridcell', {
name: 'bottom center',
} )
.focus();
expect( onChangeSpy ).toHaveBeenCalledWith( 'bottom center' );
} );
} ); Edit: I just realized I could get rid of |
Size Change: -38 B (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
Hey, @Mamaduka thanks a lot! I've borrowed some of your ideas, could you take another look? |
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.
This looks good to me, @tyxla.
Let's remove these lines as well, and I think we're good to go.
const __windowFocus = window.focus; | |
beforeAll( () => { | |
window.focus = jest.fn(); | |
} ); | |
afterAll( () => { | |
window.focus = __windowFocus; | |
} ); | |
Oh, of course! Thanks for the great review 🙌 |
What?
This PR refactors the tests of
AlignmentMatrixControl
to use@testing-library/react
's render instead ofreact-dom
render. This addresses one of the changes needed to upgrade to React 18, as touched in #32765.Why?
This addresses one of the changes needed to upgrade to React 18, as touched in #32765.
How?
We're just using the
@testing-library/react
render method and updating a few utilities, but another special.We're also using the opportunities to refactor all queries to use
screen
rather thanquerySelector
andquerySelectorAll
.Testing Instructions
Verify tests still pass:
npm run test:unit packages/components/src/alignment-matrix-control/test/index.js