From 98bfb2ba42916a6dc921ef7beadacfef04c4eb23 Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Tue, 22 Nov 2022 13:28:44 +0200 Subject: [PATCH 01/12] Fix no-node-access violation --- packages/components/src/box-control/test/index.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/components/src/box-control/test/index.js b/packages/components/src/box-control/test/index.js index d52b50924f46b..60c8b79cc8c52 100644 --- a/packages/components/src/box-control/test/index.js +++ b/packages/components/src/box-control/test/index.js @@ -27,10 +27,11 @@ const getReset = () => screen.getByText( /Reset/ ); describe( 'BoxControl', () => { describe( 'Basic rendering', () => { it( 'should render', () => { - const { container } = render( ); - const input = container.querySelector( 'input' ); + render( ); - expect( input ).toBeTruthy(); + expect( + screen.getByRole( 'textbox', { name: 'Box Control' } ) + ).toBeVisible(); } ); it( 'should update values when interacting with input', async () => { From 26658f21bf0bf3c39fae3c44521f4a2a1aaff8ab Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Tue, 22 Nov 2022 13:29:38 +0200 Subject: [PATCH 02/12] Get rid of getInput() and improve input query --- .../components/src/box-control/test/index.js | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/packages/components/src/box-control/test/index.js b/packages/components/src/box-control/test/index.js index 60c8b79cc8c52..3943e8b0344dd 100644 --- a/packages/components/src/box-control/test/index.js +++ b/packages/components/src/box-control/test/index.js @@ -19,8 +19,6 @@ const setupUser = () => advanceTimers: jest.advanceTimersByTime, } ); -const getInput = () => - screen.getByLabelText( 'Box Control', { selector: 'input' } ); const getSelect = () => screen.getByLabelText( 'Select unit' ); const getReset = () => screen.getByText( /Reset/ ); @@ -37,7 +35,9 @@ describe( 'BoxControl', () => { it( 'should update values when interacting with input', async () => { const user = setupUser(); render( ); - const input = getInput(); + const input = screen.getByRole( 'textbox', { + name: 'Box Control', + } ); const select = getSelect(); await user.type( input, '100%' ); @@ -52,7 +52,9 @@ describe( 'BoxControl', () => { it( 'should reset values when clicking Reset', async () => { const user = setupUser(); render( ); - const input = getInput(); + const input = screen.getByRole( 'textbox', { + name: 'Box Control', + } ); const select = getSelect(); const reset = getReset(); @@ -81,7 +83,9 @@ describe( 'BoxControl', () => { }; const user = setupUser(); render( ); - const input = getInput(); + const input = screen.getByRole( 'textbox', { + name: 'Box Control', + } ); const select = getSelect(); const reset = getReset(); @@ -117,7 +121,9 @@ describe( 'BoxControl', () => { }; const user = setupUser(); render( ); - const input = getInput(); + const input = screen.getByRole( 'textbox', { + name: 'Box Control', + } ); const select = getSelect(); const reset = getReset(); @@ -281,7 +287,9 @@ describe( 'BoxControl', () => { render( ); const user = setupUser(); - const input = getInput(); + const input = screen.getByRole( 'textbox', { + name: 'Box Control', + } ); await user.type( input, '7.5rem' ); await user.keyboard( '{Enter}' ); From 29c56f0fea80a46f3b752ac1a5acefd892ad1f05 Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Tue, 22 Nov 2022 13:31:24 +0200 Subject: [PATCH 03/12] Get rid of getSelect() and improve combobox query --- .../components/src/box-control/test/index.js | 29 ++++++++++++++----- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/packages/components/src/box-control/test/index.js b/packages/components/src/box-control/test/index.js index 3943e8b0344dd..e5fd17cdafc78 100644 --- a/packages/components/src/box-control/test/index.js +++ b/packages/components/src/box-control/test/index.js @@ -19,7 +19,6 @@ const setupUser = () => advanceTimers: jest.advanceTimersByTime, } ); -const getSelect = () => screen.getByLabelText( 'Select unit' ); const getReset = () => screen.getByText( /Reset/ ); describe( 'BoxControl', () => { @@ -38,7 +37,9 @@ describe( 'BoxControl', () => { const input = screen.getByRole( 'textbox', { name: 'Box Control', } ); - const select = getSelect(); + const select = screen.getByRole( 'combobox', { + name: 'Select unit', + } ); await user.type( input, '100%' ); await user.keyboard( '{Enter}' ); @@ -55,7 +56,9 @@ describe( 'BoxControl', () => { const input = screen.getByRole( 'textbox', { name: 'Box Control', } ); - const select = getSelect(); + const select = screen.getByRole( 'combobox', { + name: 'Select unit', + } ); const reset = getReset(); await user.type( input, '100px' ); @@ -86,7 +89,9 @@ describe( 'BoxControl', () => { const input = screen.getByRole( 'textbox', { name: 'Box Control', } ); - const select = getSelect(); + const select = screen.getByRole( 'combobox', { + name: 'Select unit', + } ); const reset = getReset(); await user.type( input, '100px' ); @@ -124,7 +129,9 @@ describe( 'BoxControl', () => { const input = screen.getByRole( 'textbox', { name: 'Box Control', } ); - const select = getSelect(); + const select = screen.getByRole( 'combobox', { + name: 'Select unit', + } ); const reset = getReset(); await user.type( input, '100px' ); @@ -243,7 +250,9 @@ describe( 'BoxControl', () => { const user = setupUser(); // Make unit selection on all input control. - const allUnitSelect = getSelect(); + const allUnitSelect = screen.getByRole( 'combobox', { + name: 'Select unit', + } ); await user.selectOptions( allUnitSelect, [ 'em' ] ); // Unlink the controls. @@ -260,7 +269,9 @@ describe( 'BoxControl', () => { const user = setupUser(); // Make unit selection on all input control. - const allUnitSelect = getSelect(); + const allUnitSelect = screen.getByRole( 'combobox', { + name: 'Select unit', + } ); await user.selectOptions( allUnitSelect, [ 'vw' ] ); // Unlink the controls. @@ -308,7 +319,9 @@ describe( 'BoxControl', () => { render( ); const user = setupUser(); - const allUnitSelect = getSelect(); + const allUnitSelect = screen.getByRole( 'combobox', { + name: 'Select unit', + } ); await user.selectOptions( allUnitSelect, 'rem' ); expect( setState ).toHaveBeenCalledWith( { From df5753f8a3c73954ce5be2b69cb59c801e667bc9 Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Tue, 22 Nov 2022 13:33:34 +0200 Subject: [PATCH 04/12] Get rid of getReset() and improve reset button query --- packages/components/src/box-control/test/index.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/components/src/box-control/test/index.js b/packages/components/src/box-control/test/index.js index e5fd17cdafc78..f308209129a7b 100644 --- a/packages/components/src/box-control/test/index.js +++ b/packages/components/src/box-control/test/index.js @@ -19,8 +19,6 @@ const setupUser = () => advanceTimers: jest.advanceTimersByTime, } ); -const getReset = () => screen.getByText( /Reset/ ); - describe( 'BoxControl', () => { describe( 'Basic rendering', () => { it( 'should render', () => { @@ -59,7 +57,7 @@ describe( 'BoxControl', () => { const select = screen.getByRole( 'combobox', { name: 'Select unit', } ); - const reset = getReset(); + const reset = screen.getByRole( 'button', { name: 'Reset' } ); await user.type( input, '100px' ); await user.keyboard( '{Enter}' ); @@ -92,7 +90,7 @@ describe( 'BoxControl', () => { const select = screen.getByRole( 'combobox', { name: 'Select unit', } ); - const reset = getReset(); + const reset = screen.getByRole( 'button', { name: 'Reset' } ); await user.type( input, '100px' ); await user.keyboard( '{Enter}' ); @@ -132,7 +130,7 @@ describe( 'BoxControl', () => { const select = screen.getByRole( 'combobox', { name: 'Select unit', } ); - const reset = getReset(); + const reset = screen.getByRole( 'button', { name: 'Reset' } ); await user.type( input, '100px' ); await user.keyboard( '{Enter}' ); From 54f80360612a6a01050c5f5f1da9a4a5c2e2321a Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Tue, 22 Nov 2022 13:36:15 +0200 Subject: [PATCH 05/12] Get rid of setupUser() and inline it --- .../components/src/box-control/test/index.js | 52 +++++++++++++------ 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/packages/components/src/box-control/test/index.js b/packages/components/src/box-control/test/index.js index f308209129a7b..627bf99a731e7 100644 --- a/packages/components/src/box-control/test/index.js +++ b/packages/components/src/box-control/test/index.js @@ -14,11 +14,6 @@ import { useState } from '@wordpress/element'; */ import BoxControl from '../'; -const setupUser = () => - userEvent.setup( { - advanceTimers: jest.advanceTimersByTime, - } ); - describe( 'BoxControl', () => { describe( 'Basic rendering', () => { it( 'should render', () => { @@ -30,7 +25,9 @@ describe( 'BoxControl', () => { } ); it( 'should update values when interacting with input', async () => { - const user = setupUser(); + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); render( ); const input = screen.getByRole( 'textbox', { name: 'Box Control', @@ -49,7 +46,9 @@ describe( 'BoxControl', () => { describe( 'Reset', () => { it( 'should reset values when clicking Reset', async () => { - const user = setupUser(); + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); render( ); const input = screen.getByRole( 'textbox', { name: 'Box Control', @@ -72,6 +71,9 @@ describe( 'BoxControl', () => { } ); it( 'should reset values when clicking Reset, if controlled', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); const Example = () => { const [ state, setState ] = useState(); @@ -82,7 +84,6 @@ describe( 'BoxControl', () => { /> ); }; - const user = setupUser(); render( ); const input = screen.getByRole( 'textbox', { name: 'Box Control', @@ -105,6 +106,9 @@ describe( 'BoxControl', () => { } ); it( 'should reset values when clicking Reset, if controlled <-> uncontrolled state changes', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); const Example = () => { const [ state, setState ] = useState(); @@ -122,7 +126,6 @@ describe( 'BoxControl', () => { /> ); }; - const user = setupUser(); render( ); const input = screen.getByRole( 'textbox', { name: 'Box Control', @@ -145,7 +148,9 @@ describe( 'BoxControl', () => { } ); it( 'should persist cleared value when focus changes', async () => { - const user = setupUser(); + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); const spyChange = jest.fn(); render( spyChange( v ) } /> ); const input = screen.getByLabelText( 'Box Control', { @@ -176,6 +181,9 @@ describe( 'BoxControl', () => { describe( 'Unlinked sides', () => { it( 'should update a single side value when unlinked', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); let state = {}; const setState = ( newState ) => ( state = newState ); @@ -185,7 +193,6 @@ describe( 'BoxControl', () => { onChange={ ( next ) => setState( next ) } /> ); - const user = setupUser(); const unlink = screen.getByLabelText( /Unlink sides/ ); await user.click( unlink ); @@ -208,6 +215,9 @@ describe( 'BoxControl', () => { } ); it( 'should update a whole axis when value is changed when unlinked', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); let state = {}; const setState = ( newState ) => ( state = newState ); @@ -218,7 +228,6 @@ describe( 'BoxControl', () => { splitOnAxis={ true } /> ); - const user = setupUser(); const unlink = screen.getByLabelText( /Unlink sides/ ); await user.click( unlink ); @@ -243,9 +252,12 @@ describe( 'BoxControl', () => { describe( 'Unit selections', () => { it( 'should update unlinked controls unit selection based on all input control', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + // Render control. render( ); - const user = setupUser(); // Make unit selection on all input control. const allUnitSelect = screen.getByRole( 'combobox', { @@ -262,9 +274,12 @@ describe( 'BoxControl', () => { } ); it( 'should use individual side attribute unit when available', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + // Render control. const { rerender } = render( ); - const user = setupUser(); // Make unit selection on all input control. const allUnitSelect = screen.getByRole( 'combobox', { @@ -292,10 +307,13 @@ describe( 'BoxControl', () => { describe( 'onChange updates', () => { it( 'should call onChange when values contain more than just CSS units', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + const setState = jest.fn(); render( ); - const user = setupUser(); const input = screen.getByRole( 'textbox', { name: 'Box Control', } ); @@ -312,10 +330,12 @@ describe( 'BoxControl', () => { } ); it( 'should not pass invalid CSS unit only values to onChange', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); const setState = jest.fn(); render( ); - const user = setupUser(); const allUnitSelect = screen.getByRole( 'combobox', { name: 'Select unit', From 95c25d378f4c3f86a408863249947187af586782 Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Tue, 22 Nov 2022 13:37:59 +0200 Subject: [PATCH 06/12] Add spaces to improve readability --- packages/components/src/box-control/test/index.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/components/src/box-control/test/index.js b/packages/components/src/box-control/test/index.js index 627bf99a731e7..94a34b0910745 100644 --- a/packages/components/src/box-control/test/index.js +++ b/packages/components/src/box-control/test/index.js @@ -28,7 +28,9 @@ describe( 'BoxControl', () => { const user = userEvent.setup( { advanceTimers: jest.advanceTimersByTime, } ); + render( ); + const input = screen.getByRole( 'textbox', { name: 'Box Control', } ); @@ -49,7 +51,9 @@ describe( 'BoxControl', () => { const user = userEvent.setup( { advanceTimers: jest.advanceTimersByTime, } ); + render( ); + const input = screen.getByRole( 'textbox', { name: 'Box Control', } ); @@ -84,7 +88,9 @@ describe( 'BoxControl', () => { /> ); }; + render( ); + const input = screen.getByRole( 'textbox', { name: 'Box Control', } ); @@ -126,7 +132,9 @@ describe( 'BoxControl', () => { /> ); }; + render( ); + const input = screen.getByRole( 'textbox', { name: 'Box Control', } ); @@ -152,7 +160,9 @@ describe( 'BoxControl', () => { advanceTimers: jest.advanceTimersByTime, } ); const spyChange = jest.fn(); + render( spyChange( v ) } /> ); + const input = screen.getByLabelText( 'Box Control', { selector: 'input', } ); @@ -193,6 +203,7 @@ describe( 'BoxControl', () => { onChange={ ( next ) => setState( next ) } /> ); + const unlink = screen.getByLabelText( /Unlink sides/ ); await user.click( unlink ); @@ -228,6 +239,7 @@ describe( 'BoxControl', () => { splitOnAxis={ true } /> ); + const unlink = screen.getByLabelText( /Unlink sides/ ); await user.click( unlink ); @@ -310,10 +322,10 @@ describe( 'BoxControl', () => { const user = userEvent.setup( { advanceTimers: jest.advanceTimersByTime, } ); - const setState = jest.fn(); render( ); + const input = screen.getByRole( 'textbox', { name: 'Box Control', } ); From b44098ccd119732038e12c19f25ffbe877cbdf8e Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Tue, 22 Nov 2022 13:38:42 +0200 Subject: [PATCH 07/12] Improve test name --- .eslintrc.js | 2 +- packages/components/src/box-control/test/index.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 22cb9209b2e9e..6c8976cfb0bd0 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -348,7 +348,7 @@ module.exports = { ], rules: { 'testing-library/no-container': 'off', - 'testing-library/no-node-access': 'off', + //'testing-library/no-node-access': 'off', }, }, { diff --git a/packages/components/src/box-control/test/index.js b/packages/components/src/box-control/test/index.js index 94a34b0910745..53466308bb5ca 100644 --- a/packages/components/src/box-control/test/index.js +++ b/packages/components/src/box-control/test/index.js @@ -16,7 +16,7 @@ import BoxControl from '../'; describe( 'BoxControl', () => { describe( 'Basic rendering', () => { - it( 'should render', () => { + it( 'should render a box control input', () => { render( ); expect( From 5ee14ccfb27b5c3adbf2fdedeff65eb90b08376a Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Tue, 22 Nov 2022 13:44:25 +0200 Subject: [PATCH 08/12] Inline all constants that are used just once --- .../components/src/box-control/test/index.js | 93 +++++++++---------- 1 file changed, 45 insertions(+), 48 deletions(-) diff --git a/packages/components/src/box-control/test/index.js b/packages/components/src/box-control/test/index.js index 53466308bb5ca..69640c8e4bc79 100644 --- a/packages/components/src/box-control/test/index.js +++ b/packages/components/src/box-control/test/index.js @@ -34,15 +34,16 @@ describe( 'BoxControl', () => { const input = screen.getByRole( 'textbox', { name: 'Box Control', } ); - const select = screen.getByRole( 'combobox', { - name: 'Select unit', - } ); await user.type( input, '100%' ); await user.keyboard( '{Enter}' ); expect( input ).toHaveValue( '100' ); - expect( select ).toHaveValue( '%' ); + expect( + screen.getByRole( 'combobox', { + name: 'Select unit', + } ) + ).toHaveValue( '%' ); } ); } ); @@ -60,7 +61,6 @@ describe( 'BoxControl', () => { const select = screen.getByRole( 'combobox', { name: 'Select unit', } ); - const reset = screen.getByRole( 'button', { name: 'Reset' } ); await user.type( input, '100px' ); await user.keyboard( '{Enter}' ); @@ -68,7 +68,7 @@ describe( 'BoxControl', () => { expect( input ).toHaveValue( '100' ); expect( select ).toHaveValue( 'px' ); - await user.click( reset ); + await user.click( screen.getByRole( 'button', { name: 'Reset' } ) ); expect( input ).toHaveValue( '' ); expect( select ).toHaveValue( 'px' ); @@ -97,7 +97,6 @@ describe( 'BoxControl', () => { const select = screen.getByRole( 'combobox', { name: 'Select unit', } ); - const reset = screen.getByRole( 'button', { name: 'Reset' } ); await user.type( input, '100px' ); await user.keyboard( '{Enter}' ); @@ -105,7 +104,7 @@ describe( 'BoxControl', () => { expect( input ).toHaveValue( '100' ); expect( select ).toHaveValue( 'px' ); - await user.click( reset ); + await user.click( screen.getByRole( 'button', { name: 'Reset' } ) ); expect( input ).toHaveValue( '' ); expect( select ).toHaveValue( 'px' ); @@ -141,7 +140,6 @@ describe( 'BoxControl', () => { const select = screen.getByRole( 'combobox', { name: 'Select unit', } ); - const reset = screen.getByRole( 'button', { name: 'Reset' } ); await user.type( input, '100px' ); await user.keyboard( '{Enter}' ); @@ -149,7 +147,7 @@ describe( 'BoxControl', () => { expect( input ).toHaveValue( '100' ); expect( select ).toHaveValue( 'px' ); - await user.click( reset ); + await user.click( screen.getByRole( 'button', { name: 'Reset' } ) ); expect( input ).toHaveValue( '' ); expect( select ).toHaveValue( 'px' ); @@ -166,13 +164,12 @@ describe( 'BoxControl', () => { const input = screen.getByLabelText( 'Box Control', { selector: 'input', } ); - const unitSelect = screen.getByLabelText( 'Select unit' ); await user.type( input, '100%' ); await user.keyboard( '{Enter}' ); expect( input ).toHaveValue( '100' ); - expect( unitSelect ).toHaveValue( '%' ); + expect( screen.getByLabelText( 'Select unit' ) ).toHaveValue( '%' ); await user.clear( input ); expect( input ).toHaveValue( '' ); @@ -204,18 +201,17 @@ describe( 'BoxControl', () => { /> ); - const unlink = screen.getByLabelText( /Unlink sides/ ); - - await user.click( unlink ); + await user.click( screen.getByLabelText( /Unlink sides/ ) ); const input = screen.getByLabelText( /Top/ ); - const select = screen.getAllByLabelText( /Select unit/ )[ 0 ]; await user.type( input, '100px' ); await user.keyboard( '{Enter}' ); expect( input ).toHaveValue( '100' ); - expect( select ).toHaveValue( 'px' ); + expect( + screen.getAllByLabelText( /Select unit/ )[ 0 ] + ).toHaveValue( 'px' ); expect( state ).toEqual( { top: '100px', @@ -240,18 +236,17 @@ describe( 'BoxControl', () => { /> ); - const unlink = screen.getByLabelText( /Unlink sides/ ); - - await user.click( unlink ); + await user.click( screen.getByLabelText( /Unlink sides/ ) ); const input = screen.getByLabelText( /Vertical/ ); - const select = screen.getAllByLabelText( /Select unit/ )[ 0 ]; await user.type( input, '100px' ); await user.keyboard( '{Enter}' ); expect( input ).toHaveValue( '100' ); - expect( select ).toHaveValue( 'px' ); + expect( + screen.getAllByLabelText( /Select unit/ )[ 0 ] + ).toHaveValue( 'px' ); expect( state ).toEqual( { top: '100px', @@ -272,17 +267,18 @@ describe( 'BoxControl', () => { render( ); // Make unit selection on all input control. - const allUnitSelect = screen.getByRole( 'combobox', { - name: 'Select unit', - } ); - await user.selectOptions( allUnitSelect, [ 'em' ] ); + await user.selectOptions( + screen.getByRole( 'combobox', { + name: 'Select unit', + } ), + [ 'em' ] + ); // Unlink the controls. await user.click( screen.getByLabelText( /Unlink sides/ ) ); // Confirm that each individual control has the selected unit - const unlinkedSelects = screen.getAllByDisplayValue( 'em' ); - expect( unlinkedSelects.length ).toEqual( 4 ); + expect( screen.getAllByDisplayValue( 'em' ).length ).toEqual( 4 ); } ); it( 'should use individual side attribute unit when available', async () => { @@ -294,26 +290,24 @@ describe( 'BoxControl', () => { const { rerender } = render( ); // Make unit selection on all input control. - const allUnitSelect = screen.getByRole( 'combobox', { - name: 'Select unit', - } ); - await user.selectOptions( allUnitSelect, [ 'vw' ] ); + await user.selectOptions( + screen.getByRole( 'combobox', { + name: 'Select unit', + } ), + [ 'vw' ] + ); // Unlink the controls. await user.click( screen.getByLabelText( /Unlink sides/ ) ); // Confirm that each individual control has the selected unit - const unlinkedSelects = screen.getAllByDisplayValue( 'vw' ); - expect( unlinkedSelects.length ).toEqual( 4 ); + expect( screen.getAllByDisplayValue( 'vw' ).length ).toEqual( 4 ); // Rerender with individual side value & confirm unit is selected. rerender( ); - const topSelect = screen.getByDisplayValue( 'em' ); - const otherSelects = screen.getAllByDisplayValue( 'vw' ); - - expect( topSelect ).toBeInTheDocument(); - expect( otherSelects.length ).toEqual( 3 ); + expect( screen.getByDisplayValue( 'em' ) ).toBeInTheDocument(); + expect( screen.getAllByDisplayValue( 'vw' ).length ).toEqual( 3 ); } ); } ); @@ -326,11 +320,12 @@ describe( 'BoxControl', () => { render( ); - const input = screen.getByRole( 'textbox', { - name: 'Box Control', - } ); - - await user.type( input, '7.5rem' ); + await user.type( + screen.getByRole( 'textbox', { + name: 'Box Control', + } ), + '7.5rem' + ); await user.keyboard( '{Enter}' ); expect( setState ).toHaveBeenCalledWith( { @@ -349,10 +344,12 @@ describe( 'BoxControl', () => { render( ); - const allUnitSelect = screen.getByRole( 'combobox', { - name: 'Select unit', - } ); - await user.selectOptions( allUnitSelect, 'rem' ); + await user.selectOptions( + screen.getByRole( 'combobox', { + name: 'Select unit', + } ), + 'rem' + ); expect( setState ).toHaveBeenCalledWith( { top: undefined, From fe590215b1248dcb0cabfe5755ca6814dd7af1f7 Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Tue, 22 Nov 2022 13:45:30 +0200 Subject: [PATCH 09/12] Use toHaveLength to improve assertion --- packages/components/src/box-control/test/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/components/src/box-control/test/index.js b/packages/components/src/box-control/test/index.js index 69640c8e4bc79..ff4b59d256d1e 100644 --- a/packages/components/src/box-control/test/index.js +++ b/packages/components/src/box-control/test/index.js @@ -278,7 +278,7 @@ describe( 'BoxControl', () => { await user.click( screen.getByLabelText( /Unlink sides/ ) ); // Confirm that each individual control has the selected unit - expect( screen.getAllByDisplayValue( 'em' ).length ).toEqual( 4 ); + expect( screen.getAllByDisplayValue( 'em' ) ).toHaveLength( 4 ); } ); it( 'should use individual side attribute unit when available', async () => { @@ -301,13 +301,13 @@ describe( 'BoxControl', () => { await user.click( screen.getByLabelText( /Unlink sides/ ) ); // Confirm that each individual control has the selected unit - expect( screen.getAllByDisplayValue( 'vw' ).length ).toEqual( 4 ); + expect( screen.getAllByDisplayValue( 'vw' ) ).toHaveLength( 4 ); // Rerender with individual side value & confirm unit is selected. rerender( ); expect( screen.getByDisplayValue( 'em' ) ).toBeInTheDocument(); - expect( screen.getAllByDisplayValue( 'vw' ).length ).toEqual( 3 ); + expect( screen.getAllByDisplayValue( 'vw' ) ).toHaveLength( 3 ); } ); } ); From 27ed32c818f3530c4196a17a1e80d4792a6daed7 Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Tue, 22 Nov 2022 13:45:49 +0200 Subject: [PATCH 10/12] Revert unwanted change --- .eslintrc.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.eslintrc.js b/.eslintrc.js index 6c8976cfb0bd0..22cb9209b2e9e 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -348,7 +348,7 @@ module.exports = { ], rules: { 'testing-library/no-container': 'off', - //'testing-library/no-node-access': 'off', + 'testing-library/no-node-access': 'off', }, }, { From 7f379b702368535abd4b230735567697e283f7a2 Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Tue, 22 Nov 2022 14:01:16 +0200 Subject: [PATCH 11/12] Improve precision of existing queries --- .../components/src/box-control/test/index.js | 76 +++++++++++++++---- 1 file changed, 61 insertions(+), 15 deletions(-) diff --git a/packages/components/src/box-control/test/index.js b/packages/components/src/box-control/test/index.js index ff4b59d256d1e..0100a1d62b877 100644 --- a/packages/components/src/box-control/test/index.js +++ b/packages/components/src/box-control/test/index.js @@ -161,15 +161,19 @@ describe( 'BoxControl', () => { render( spyChange( v ) } /> ); - const input = screen.getByLabelText( 'Box Control', { - selector: 'input', + const input = screen.getByRole( 'textbox', { + name: 'Box Control', } ); await user.type( input, '100%' ); await user.keyboard( '{Enter}' ); expect( input ).toHaveValue( '100' ); - expect( screen.getByLabelText( 'Select unit' ) ).toHaveValue( '%' ); + expect( + screen.getByRole( 'combobox', { + name: 'Select unit', + } ) + ).toHaveValue( '%' ); await user.clear( input ); expect( input ).toHaveValue( '' ); @@ -201,16 +205,20 @@ describe( 'BoxControl', () => { /> ); - await user.click( screen.getByLabelText( /Unlink sides/ ) ); + await user.click( + screen.getByRole( 'button', { name: 'Unlink sides' } ) + ); - const input = screen.getByLabelText( /Top/ ); + const input = screen.getByRole( 'textbox', { name: 'Top' } ); await user.type( input, '100px' ); await user.keyboard( '{Enter}' ); expect( input ).toHaveValue( '100' ); expect( - screen.getAllByLabelText( /Select unit/ )[ 0 ] + screen.getAllByRole( 'combobox', { + name: 'Select unit', + } )[ 0 ] ).toHaveValue( 'px' ); expect( state ).toEqual( { @@ -236,16 +244,22 @@ describe( 'BoxControl', () => { /> ); - await user.click( screen.getByLabelText( /Unlink sides/ ) ); + await user.click( + screen.getByRole( 'button', { name: 'Unlink sides' } ) + ); - const input = screen.getByLabelText( /Vertical/ ); + const input = screen.getByRole( 'textbox', { + name: 'Vertical', + } ); await user.type( input, '100px' ); await user.keyboard( '{Enter}' ); expect( input ).toHaveValue( '100' ); expect( - screen.getAllByLabelText( /Select unit/ )[ 0 ] + screen.getAllByRole( 'combobox', { + name: 'Select unit', + } )[ 0 ] ).toHaveValue( 'px' ); expect( state ).toEqual( { @@ -275,10 +289,21 @@ describe( 'BoxControl', () => { ); // Unlink the controls. - await user.click( screen.getByLabelText( /Unlink sides/ ) ); + await user.click( + screen.getByRole( 'button', { name: 'Unlink sides' } ) + ); + + const controls = screen.getAllByRole( 'combobox', { + name: 'Select unit', + } ); + + // Confirm we have exactly 4 controls. + expect( controls ).toHaveLength( 4 ); // Confirm that each individual control has the selected unit - expect( screen.getAllByDisplayValue( 'em' ) ).toHaveLength( 4 ); + controls.forEach( ( control ) => { + expect( control ).toHaveValue( 'em' ); + } ); } ); it( 'should use individual side attribute unit when available', async () => { @@ -298,16 +323,37 @@ describe( 'BoxControl', () => { ); // Unlink the controls. - await user.click( screen.getByLabelText( /Unlink sides/ ) ); + await user.click( + screen.getByRole( 'button', { name: 'Unlink sides' } ) + ); + + const controls = screen.getAllByRole( 'combobox', { + name: 'Select unit', + } ); + + // Confirm we have exactly 4 controls. + expect( controls ).toHaveLength( 4 ); // Confirm that each individual control has the selected unit - expect( screen.getAllByDisplayValue( 'vw' ) ).toHaveLength( 4 ); + controls.forEach( ( control ) => { + expect( control ).toHaveValue( 'vw' ); + } ); // Rerender with individual side value & confirm unit is selected. rerender( ); - expect( screen.getByDisplayValue( 'em' ) ).toBeInTheDocument(); - expect( screen.getAllByDisplayValue( 'vw' ) ).toHaveLength( 3 ); + const rerenderedControls = screen.getAllByRole( 'combobox', { + name: 'Select unit', + } ); + + // Confirm we have exactly 4 controls. + expect( rerenderedControls ).toHaveLength( 4 ); + + // Confirm that each individual control has the right selected unit + rerenderedControls.forEach( ( control, index ) => { + const expected = index === 0 ? 'em' : 'vw'; + expect( control ).toHaveValue( expected ); + } ); } ); } ); From df405ba19b4b761c301e94932e0ef38fd0aff1c1 Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Wed, 23 Nov 2022 14:33:55 +0200 Subject: [PATCH 12/12] Use the same mock for setState(), improve assertions --- .../components/src/box-control/test/index.js | 132 +++++++----------- 1 file changed, 54 insertions(+), 78 deletions(-) diff --git a/packages/components/src/box-control/test/index.js b/packages/components/src/box-control/test/index.js index 0100a1d62b877..5da4994cf4612 100644 --- a/packages/components/src/box-control/test/index.js +++ b/packages/components/src/box-control/test/index.js @@ -14,6 +14,18 @@ import { useState } from '@wordpress/element'; */ import BoxControl from '../'; +const Example = ( extraProps ) => { + const [ state, setState ] = useState(); + + return ( + setState( next ) } + { ...extraProps } + /> + ); +}; + describe( 'BoxControl', () => { describe( 'Basic rendering', () => { it( 'should render a box control input', () => { @@ -78,16 +90,6 @@ describe( 'BoxControl', () => { const user = userEvent.setup( { advanceTimers: jest.advanceTimersByTime, } ); - const Example = () => { - const [ state, setState ] = useState(); - - return ( - setState( next ) } - /> - ); - }; render( ); @@ -114,23 +116,6 @@ describe( 'BoxControl', () => { const user = userEvent.setup( { advanceTimers: jest.advanceTimersByTime, } ); - const Example = () => { - const [ state, setState ] = useState(); - - return ( - { - if ( next.top ) { - setState( next ); - } else { - // This reverts it to being uncontrolled. - setState( undefined ); - } - } } - /> - ); - }; render( ); @@ -195,79 +180,70 @@ describe( 'BoxControl', () => { const user = userEvent.setup( { advanceTimers: jest.advanceTimersByTime, } ); - let state = {}; - const setState = ( newState ) => ( state = newState ); - - render( - setState( next ) } - /> - ); + + render( ); await user.click( screen.getByRole( 'button', { name: 'Unlink sides' } ) ); - const input = screen.getByRole( 'textbox', { name: 'Top' } ); - - await user.type( input, '100px' ); + await user.type( + screen.getByRole( 'textbox', { name: 'Top' } ), + '100px' + ); await user.keyboard( '{Enter}' ); - expect( input ).toHaveValue( '100' ); expect( - screen.getAllByRole( 'combobox', { - name: 'Select unit', - } )[ 0 ] - ).toHaveValue( 'px' ); - - expect( state ).toEqual( { - top: '100px', - right: undefined, - bottom: undefined, - left: undefined, - } ); + screen.getByRole( 'textbox', { name: 'Top' } ) + ).toHaveValue( '100' ); + expect( + screen.getByRole( 'textbox', { name: 'Right' } ) + ).not.toHaveValue(); + expect( + screen.getByRole( 'textbox', { name: 'Bottom' } ) + ).not.toHaveValue(); + expect( + screen.getByRole( 'textbox', { name: 'Left' } ) + ).not.toHaveValue(); + + screen + .getAllByRole( 'combobox', { name: 'Select unit' } ) + .forEach( ( combobox ) => { + expect( combobox ).toHaveValue( 'px' ); + } ); } ); it( 'should update a whole axis when value is changed when unlinked', async () => { const user = userEvent.setup( { advanceTimers: jest.advanceTimersByTime, } ); - let state = {}; - const setState = ( newState ) => ( state = newState ); - - render( - setState( next ) } - splitOnAxis={ true } - /> - ); + + render( ); await user.click( screen.getByRole( 'button', { name: 'Unlink sides' } ) ); - const input = screen.getByRole( 'textbox', { - name: 'Vertical', - } ); - - await user.type( input, '100px' ); + await user.type( + screen.getByRole( 'textbox', { + name: 'Vertical', + } ), + '100px' + ); await user.keyboard( '{Enter}' ); - expect( input ).toHaveValue( '100' ); expect( - screen.getAllByRole( 'combobox', { - name: 'Select unit', - } )[ 0 ] - ).toHaveValue( 'px' ); - - expect( state ).toEqual( { - top: '100px', - right: undefined, - bottom: '100px', - left: undefined, - } ); + screen.getByRole( 'textbox', { name: 'Vertical' } ) + ).toHaveValue( '100' ); + expect( + screen.getByRole( 'textbox', { name: 'Horizontal' } ) + ).not.toHaveValue(); + + screen + .getAllByRole( 'combobox', { name: 'Select unit' } ) + .forEach( ( combobox ) => { + expect( combobox ).toHaveValue( 'px' ); + } ); } ); } );