From dfdc258a3b24de87624cd70262aa3a7aaa8f7562 Mon Sep 17 00:00:00 2001 From: Jack Stevens Date: Fri, 3 May 2024 20:50:43 +0100 Subject: [PATCH 1/5] chore: migrate focal-point-picker media tests to typescript --- .../test/{media.js => media.tsx} | 40 +++++++++++++++---- 1 file changed, 33 insertions(+), 7 deletions(-) rename packages/components/src/focal-point-picker/test/{media.js => media.tsx} (62%) diff --git a/packages/components/src/focal-point-picker/test/media.js b/packages/components/src/focal-point-picker/test/media.tsx similarity index 62% rename from packages/components/src/focal-point-picker/test/media.js rename to packages/components/src/focal-point-picker/test/media.tsx index 25296424cf65e5..cfe1c593308e1d 100644 --- a/packages/components/src/focal-point-picker/test/media.js +++ b/packages/components/src/focal-point-picker/test/media.tsx @@ -7,11 +7,17 @@ import { render, screen } from '@testing-library/react'; * Internal dependencies */ import Media from '../media'; +import type { FocalPointPickerMediaProps } from '../types'; + +const props: FocalPointPickerMediaProps = { + alt: '', + src: '', +}; describe( 'FocalPointPicker/Media', () => { describe( 'Basic rendering', () => { it( 'should render', () => { - render( ); + render( ); expect( screen.getByTestId( 'media' ) ).toBeVisible(); } ); @@ -19,7 +25,7 @@ describe( 'FocalPointPicker/Media', () => { describe( 'Media types', () => { it( 'should render a placeholder by default', () => { - render( ); + render( ); expect( screen.getByTestId( 'media' ) ).toHaveClass( 'components-focal-point-picker__media--placeholder' @@ -28,19 +34,34 @@ describe( 'FocalPointPicker/Media', () => { it( 'should render an video if src is a video file', () => { const { rerender } = render( - + ); expect( screen.getByTestId( 'media' ).tagName ).toBe( 'VIDEO' ); rerender( - + ); expect( screen.getByTestId( 'media' ).tagName ).toBe( 'VIDEO' ); rerender( - + ); expect( screen.getByTestId( 'media' ).tagName ).toBe( 'VIDEO' ); @@ -48,13 +69,18 @@ describe( 'FocalPointPicker/Media', () => { it( 'should render an image file, if not video', () => { const { rerender } = render( - + ); expect( screen.getByTestId( 'media' ).tagName ).toBe( 'IMG' ); rerender( - + ); expect( screen.getByTestId( 'media' ).tagName ).toBe( 'IMG' ); From 42ad69f7e88ece3f0c373e919ba83cecf6a3579f Mon Sep 17 00:00:00 2001 From: Jack Stevens Date: Fri, 3 May 2024 20:51:03 +0100 Subject: [PATCH 2/5] chore: migrate focal-point-picker index tests to typescript --- .../test/{index.js => index.tsx} | 64 ++++++++++++++----- 1 file changed, 47 insertions(+), 17 deletions(-) rename packages/components/src/focal-point-picker/test/{index.js => index.tsx} (75%) diff --git a/packages/components/src/focal-point-picker/test/index.js b/packages/components/src/focal-point-picker/test/index.tsx similarity index 75% rename from packages/components/src/focal-point-picker/test/index.js rename to packages/components/src/focal-point-picker/test/index.tsx index d5c7946cffd860..9883bf660fd237 100644 --- a/packages/components/src/focal-point-picker/test/index.js +++ b/packages/components/src/focal-point-picker/test/index.tsx @@ -3,12 +3,28 @@ */ import { fireEvent, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +/** + * Internal dependencies + */ +import type { FocalPointPickerProps } from '../types'; /** * Internal dependencies */ import Picker from '..'; +type Log = { name: string; args: any[] }; +type EventLogger = ( name: string, args: any[] ) => void; + +const props: FocalPointPickerProps = { + onChange: jest.fn(), + url: 'test-url', + value: { + x: 0, + y: 0, + }, +}; + describe( 'FocalPointPicker', () => { describe( 'focus and blur', () => { it( 'clicking the draggable area should focus it', async () => { @@ -16,7 +32,7 @@ describe( 'FocalPointPicker', () => { const mockOnChange = jest.fn(); - render( ); + render( ); const draggableArea = screen.getByRole( 'button' ); @@ -32,6 +48,7 @@ describe( 'FocalPointPicker', () => { render( { describe( 'drag gestures', () => { it( 'should call onDragStart, onDrag, onDragEnd and onChange in that order', () => { - const logs = []; - const eventLogger = ( name, args ) => logs.push( { name, args } ); + const logs: Log[] = []; + const eventLogger: EventLogger = ( name, args ) => + logs.push( { name, args } ); const events = [ 'onDragStart', 'onDrag', 'onDragEnd', 'onChange' ]; - const handlers = {}; + const handlers: { [ key: string ]: EventLogger } = {}; events.forEach( ( name ) => { handlers[ name ] = ( ...all ) => eventLogger( name, all ); } ); - render( ); + render( ); const dragArea = screen.getByRole( 'button' ); @@ -92,6 +110,7 @@ describe( 'FocalPointPicker', () => { render( { @@ -118,12 +137,12 @@ describe( 'FocalPointPicker', () => { describe( 'controllability', () => { it( 'should update value from props', () => { const { rerender } = render( - + ); const xInput = screen.getByRole( 'spinbutton', { name: 'Focal point left position', - } ); - rerender( ); + } ) as HTMLButtonElement; + rerender( ); expect( xInput.value ).toBe( '93' ); } ); it( 'call onChange with the expected values', async () => { @@ -131,7 +150,11 @@ describe( 'FocalPointPicker', () => { const spyChange = jest.fn(); render( - + ); // Focus and press arrow up const dragArea = screen.getByRole( 'button' ); @@ -151,20 +174,27 @@ describe( 'FocalPointPicker', () => { const onChangeSpy = jest.fn(); render( ); - expect( - screen.getByRole( 'spinbutton', { - name: 'Focal point left position', - } ).value + ( + screen.getByRole( 'spinbutton', { + name: 'Focal point left position', + } ) as HTMLButtonElement + ).value ).toBe( '10' ); expect( - screen.getByRole( 'spinbutton', { - name: 'Focal point top position', - } ).value + ( + screen.getByRole( 'spinbutton', { + name: 'Focal point top position', + } ) as HTMLButtonElement + ).value ).toBe( '20' ); expect( onChangeSpy ).not.toHaveBeenCalled(); } ); From 5eb0ba898d5454d09b58c7cb2f3111e53e7bf16a Mon Sep 17 00:00:00 2001 From: Jack Stevens Date: Fri, 3 May 2024 20:56:19 +0100 Subject: [PATCH 3/5] fix: remove duplicate internal dependencies comment --- packages/components/src/focal-point-picker/test/index.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/components/src/focal-point-picker/test/index.tsx b/packages/components/src/focal-point-picker/test/index.tsx index 9883bf660fd237..dde9ec8cb67e24 100644 --- a/packages/components/src/focal-point-picker/test/index.tsx +++ b/packages/components/src/focal-point-picker/test/index.tsx @@ -3,15 +3,12 @@ */ import { fireEvent, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -/** - * Internal dependencies - */ -import type { FocalPointPickerProps } from '../types'; /** * Internal dependencies */ import Picker from '..'; +import type { FocalPointPickerProps } from '../types'; type Log = { name: string; args: any[] }; type EventLogger = ( name: string, args: any[] ) => void; From bd57b124051f5838c501b4957a023531a07587e0 Mon Sep 17 00:00:00 2001 From: Jack Stevens Date: Thu, 9 May 2024 14:03:36 +0100 Subject: [PATCH 4/5] chore: use `unknown[]` instead of `any[]` for test types Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com> --- packages/components/src/focal-point-picker/test/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/components/src/focal-point-picker/test/index.tsx b/packages/components/src/focal-point-picker/test/index.tsx index dde9ec8cb67e24..1eccced32c70af 100644 --- a/packages/components/src/focal-point-picker/test/index.tsx +++ b/packages/components/src/focal-point-picker/test/index.tsx @@ -10,8 +10,8 @@ import userEvent from '@testing-library/user-event'; import Picker from '..'; import type { FocalPointPickerProps } from '../types'; -type Log = { name: string; args: any[] }; -type EventLogger = ( name: string, args: any[] ) => void; +type Log = { name: string; args: unknown[] }; +type EventLogger = ( name: string, args: unknown[] ) => void; const props: FocalPointPickerProps = { onChange: jest.fn(), From c2315a81e787ba1df0c58bcb87e17be39accf48d Mon Sep 17 00:00:00 2001 From: Jack Stevens Date: Thu, 9 May 2024 18:03:58 +0100 Subject: [PATCH 5/5] chore: extract data-testid to common prop object --- .../src/focal-point-picker/test/media.tsx | 47 +++++-------------- 1 file changed, 13 insertions(+), 34 deletions(-) diff --git a/packages/components/src/focal-point-picker/test/media.tsx b/packages/components/src/focal-point-picker/test/media.tsx index cfe1c593308e1d..e2c03968f795d9 100644 --- a/packages/components/src/focal-point-picker/test/media.tsx +++ b/packages/components/src/focal-point-picker/test/media.tsx @@ -9,15 +9,20 @@ import { render, screen } from '@testing-library/react'; import Media from '../media'; import type { FocalPointPickerMediaProps } from '../types'; -const props: FocalPointPickerMediaProps = { +type FocalPointPickerMediaTestProps = FocalPointPickerMediaProps & { + 'data-testid': string; +}; + +const props: FocalPointPickerMediaTestProps = { alt: '', src: '', + 'data-testid': 'media', }; describe( 'FocalPointPicker/Media', () => { describe( 'Basic rendering', () => { it( 'should render', () => { - render( ); + render( ); expect( screen.getByTestId( 'media' ) ).toBeVisible(); } ); @@ -25,7 +30,7 @@ describe( 'FocalPointPicker/Media', () => { describe( 'Media types', () => { it( 'should render a placeholder by default', () => { - render( ); + render( ); expect( screen.getByTestId( 'media' ) ).toHaveClass( 'components-focal-point-picker__media--placeholder' @@ -34,54 +39,28 @@ describe( 'FocalPointPicker/Media', () => { it( 'should render an video if src is a video file', () => { const { rerender } = render( - + ); expect( screen.getByTestId( 'media' ).tagName ).toBe( 'VIDEO' ); - rerender( - - ); + rerender( ); expect( screen.getByTestId( 'media' ).tagName ).toBe( 'VIDEO' ); - rerender( - - ); + rerender( ); expect( screen.getByTestId( 'media' ).tagName ).toBe( 'VIDEO' ); } ); it( 'should render an image file, if not video', () => { const { rerender } = render( - + ); expect( screen.getByTestId( 'media' ).tagName ).toBe( 'IMG' ); - rerender( - - ); + rerender( ); expect( screen.getByTestId( 'media' ).tagName ).toBe( 'IMG' ); } );