From ca67d3af04fec133880408b4994474670b6f4eb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petter=20Walb=C3=B8=20Johnsg=C3=A5rd?= Date: Tue, 9 Aug 2022 00:21:18 +0200 Subject: [PATCH 1/5] Components: Refactor Placeholder tests to @testing-library/react --- .../components/src/placeholder/test/index.js | 147 +++++++++--------- 1 file changed, 71 insertions(+), 76 deletions(-) diff --git a/packages/components/src/placeholder/test/index.js b/packages/components/src/placeholder/test/index.js index e2a6090764cb2..b852f36fd8248 100644 --- a/packages/components/src/placeholder/test/index.js +++ b/packages/components/src/placeholder/test/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { shallow } from 'enzyme'; +import { render, screen, within } from '@testing-library/react'; /** * WordPress dependencies @@ -12,7 +12,7 @@ import { useResizeObserver } from '@wordpress/compose'; /** * Internal dependencies */ -import Placeholder from '../'; +import BasePlaceholder from '../'; jest.mock( '@wordpress/compose', () => { return { @@ -21,6 +21,17 @@ jest.mock( '@wordpress/compose', () => { }; } ); +const Placeholder = ( props ) => ( + +); + +const getPlaceholder = () => screen.getByTestId( 'placeholder' ); + +const getLabel = () => { + const placeholder = getPlaceholder(); + return placeholder.querySelector( '.components-placeholder__label' ); +}; + describe( 'Placeholder', () => { beforeEach( () => { useResizeObserver.mockReturnValue( [ @@ -31,105 +42,87 @@ describe( 'Placeholder', () => { describe( 'basic rendering', () => { it( 'should by default render label section and fieldset.', () => { - const placeholder = shallow( ); - const placeholderLabel = placeholder.find( - '.components-placeholder__label' - ); - const placeholderInstructions = placeholder.find( - '.components-placeholder__instructions' - ); - const placeholderFieldset = placeholder.find( - '.components-placeholder__fieldset' - ); + render( ); + const placeholder = getPlaceholder(); + + expect( placeholder ).toHaveClass( 'components-placeholder' ); - expect( placeholder.hasClass( 'components-placeholder' ) ).toBe( - true - ); // Test for empty label. - expect( placeholderLabel.exists() ).toBe( true ); - expect( placeholderLabel.find( 'Dashicon' ).exists() ).toBe( - false + const label = getLabel(); + expect( label ).toBeInTheDocument(); + expect( label ).toBeEmptyDOMElement(); + + // Test for non existent instructions. + const placeholderInstructions = placeholder.querySelector( + '.components-placeholder__instructions' ); - // Test for non existant instructions. - expect( placeholderInstructions.exists() ).toBe( false ); + expect( placeholderInstructions ).not.toBeInTheDocument(); + // Test for empty fieldset. - expect( placeholderFieldset.exists() ).toBe( true ); + const placeholderFieldset = + within( placeholder ).getByRole( 'group' ); + expect( placeholderFieldset ).toBeInTheDocument(); + expect( placeholderFieldset ).toBeEmptyDOMElement(); } ); it( 'should render an Icon in the label section', () => { - const placeholder = shallow( ); - const placeholderLabel = placeholder.find( - '.components-placeholder__label' - ); + render( ); + const icon = getLabel()?.querySelector( 'svg' ); - expect( placeholderLabel.exists() ).toBe( true ); - expect( placeholderLabel.find( 'Icon' ).exists() ).toBe( true ); + expect( icon ).toBeInTheDocument(); } ); it( 'should render a label section', () => { const label = 'WordPress'; - const placeholder = shallow( ); - const placeholderLabel = placeholder.find( - '.components-placeholder__label' - ); - const child = placeholderLabel.childAt( 1 ); + render( ); + const placeholderLabel = getLabel(); - expect( child.text() ).toBe( label ); + expect( placeholderLabel ).toHaveTextContent( label ); } ); it( 'should display an instructions element', () => { - const element =
Instructions
; - const placeholder = shallow( - - ); - const placeholderInstructions = placeholder.find( - '.components-placeholder__instructions' - ); - const child = placeholderInstructions.childAt( 0 ); + const element =
Instructions
; + render( ); + const placeholderInstructions = + screen.getByTestId( 'instructions' ); - expect( placeholderInstructions.exists() ).toBe( true ); - expect( child.matchesElement( element ) ).toBe( true ); + expect( placeholderInstructions ).toBeInTheDocument(); } ); it( 'should display a fieldset from the children property', () => { - const element =
Fieldset
; - const placeholder = shallow( ); - const placeholderFieldset = placeholder.find( - 'fieldset.components-placeholder__fieldset' - ); - const child = placeholderFieldset.childAt( 0 ); + const content = 'Fieldset'; + render( { content } ); + const placeholderFieldset = screen.getByRole( 'group' ); - expect( placeholderFieldset.exists() ).toBe( true ); - expect( child.matchesElement( element ) ).toBe( true ); + expect( placeholderFieldset ).toBeInTheDocument(); + expect( placeholderFieldset ).toHaveTextContent( content ); } ); it( 'should display a legend if instructions are passed', () => { - const element =
Fieldset
; const instructions = 'Choose an option.'; - const placeholder = shallow( - - ); - const placeholderLegend = placeholder.find( - 'legend.components-placeholder__instructions' + render( + +
Fieldset
+ ); + const placeholderLegend = screen.getByText( instructions ); - expect( placeholderLegend.exists() ).toBe( true ); - expect( placeholderLegend.text() ).toEqual( instructions ); + expect( placeholderLegend ).toBeInTheDocument(); + expect( placeholderLegend.tagName ).toBe( 'LEGEND' ); } ); it( 'should add an additional className to the top container', () => { - const placeholder = shallow( - - ); - expect( placeholder.hasClass( 'wp-placeholder' ) ).toBe( true ); + render( ); + const placeholder = getPlaceholder(); + + expect( placeholder ).toHaveClass( 'wp-placeholder' ); } ); it( 'should add additional props to the top level container', () => { - const placeholder = shallow( ); - expect( placeholder.prop( 'test' ) ).toBe( 'test' ); + render( ); + const placeholder = getPlaceholder(); + + expect( placeholder ).toHaveAttribute( 'test', 'test' ); } ); } ); @@ -140,11 +133,12 @@ describe( 'Placeholder', () => { { width: 480 }, ] ); - const placeholder = shallow( ); + render( ); + const placeholder = getPlaceholder(); - expect( placeholder.hasClass( 'is-large' ) ).toBe( true ); - expect( placeholder.hasClass( 'is-medium' ) ).toBe( false ); - expect( placeholder.hasClass( 'is-small' ) ).toBe( false ); + expect( placeholder ).toHaveClass( 'is-large' ); + expect( placeholder ).not.toHaveClass( 'is-medium' ); + expect( placeholder ).not.toHaveClass( 'is-small' ); } ); it( 'should assign modifier class', () => { @@ -153,11 +147,12 @@ describe( 'Placeholder', () => { { width: null }, ] ); - const placeholder = shallow( ); + render( ); + const placeholder = getPlaceholder(); - expect( placeholder.hasClass( 'is-large' ) ).toBe( false ); - expect( placeholder.hasClass( 'is-medium' ) ).toBe( false ); - expect( placeholder.hasClass( 'is-small' ) ).toBe( false ); + expect( placeholder ).not.toHaveClass( 'is-large' ); + expect( placeholder ).not.toHaveClass( 'is-medium' ); + expect( placeholder ).not.toHaveClass( 'is-small' ); } ); } ); } ); From 1b47272d3742cf99518994f4827d097ba2ec5e54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petter=20Walb=C3=B8=20Johnsg=C3=A5rd?= Date: Tue, 9 Aug 2022 19:36:55 +0200 Subject: [PATCH 2/5] Convert to TypeScript --- .../placeholder/test/{index.js => index.tsx} | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) rename packages/components/src/placeholder/test/{index.js => index.tsx} (88%) diff --git a/packages/components/src/placeholder/test/index.js b/packages/components/src/placeholder/test/index.tsx similarity index 88% rename from packages/components/src/placeholder/test/index.js rename to packages/components/src/placeholder/test/index.tsx index b852f36fd8248..a8f42d3b36091 100644 --- a/packages/components/src/placeholder/test/index.js +++ b/packages/components/src/placeholder/test/index.tsx @@ -13,6 +13,8 @@ import { useResizeObserver } from '@wordpress/compose'; * Internal dependencies */ import BasePlaceholder from '../'; +import type { WordPressComponentProps } from '../../ui/context'; +import type { PlaceholderProps } from '../types'; jest.mock( '@wordpress/compose', () => { return { @@ -21,9 +23,12 @@ jest.mock( '@wordpress/compose', () => { }; } ); -const Placeholder = ( props ) => ( - -); +const Placeholder = ( + props: Omit< + WordPressComponentProps< PlaceholderProps< unknown >, 'div', false >, + 'ref' + > +) => ; const getPlaceholder = () => screen.getByTestId( 'placeholder' ); @@ -34,6 +39,7 @@ const getLabel = () => { describe( 'Placeholder', () => { beforeEach( () => { + // @ts-ignore useResizeObserver.mockReturnValue( [
, { width: 320 }, @@ -80,15 +86,6 @@ describe( 'Placeholder', () => { expect( placeholderLabel ).toHaveTextContent( label ); } ); - it( 'should display an instructions element', () => { - const element =
Instructions
; - render( ); - const placeholderInstructions = - screen.getByTestId( 'instructions' ); - - expect( placeholderInstructions ).toBeInTheDocument(); - } ); - it( 'should display a fieldset from the children property', () => { const content = 'Fieldset'; render( { content } ); @@ -119,15 +116,16 @@ describe( 'Placeholder', () => { } ); it( 'should add additional props to the top level container', () => { - render( ); + render( ); const placeholder = getPlaceholder(); - expect( placeholder ).toHaveAttribute( 'test', 'test' ); + expect( placeholder ).toHaveAttribute( 'data-test', 'test' ); } ); } ); describe( 'resize aware', () => { it( 'should not assign modifier class in first-pass `null` width from `useResizeObserver`', () => { + // @ts-ignore useResizeObserver.mockReturnValue( [
, { width: 480 }, @@ -142,6 +140,7 @@ describe( 'Placeholder', () => { } ); it( 'should assign modifier class', () => { + // @ts-ignore useResizeObserver.mockReturnValue( [
, { width: null }, From 3e8a69864cb1b4d1128a0e5263e935c9b6b462da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petter=20Walb=C3=B8=20Johnsg=C3=A5rd?= Date: Wed, 10 Aug 2022 16:05:58 +0200 Subject: [PATCH 3/5] Update packages/components/src/placeholder/test/index.tsx Co-authored-by: Lena Morita --- packages/components/src/placeholder/test/index.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/components/src/placeholder/test/index.tsx b/packages/components/src/placeholder/test/index.tsx index a8f42d3b36091..0107435624df6 100644 --- a/packages/components/src/placeholder/test/index.tsx +++ b/packages/components/src/placeholder/test/index.tsx @@ -102,10 +102,11 @@ describe( 'Placeholder', () => {
Fieldset
); - const placeholderLegend = screen.getByText( instructions ); + const captionedFieldset = screen.getByRole( 'group', { + name: instructions, + } ); - expect( placeholderLegend ).toBeInTheDocument(); - expect( placeholderLegend.tagName ).toBe( 'LEGEND' ); + expect( captionedFieldset ).toBeInTheDocument(); } ); it( 'should add an additional className to the top container', () => { From a9233203a873b70f444a6291fa0d68a1389eac67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petter=20Walb=C3=B8=20Johnsg=C3=A5rd?= Date: Thu, 11 Aug 2022 19:18:29 +0200 Subject: [PATCH 4/5] Update README.md --- packages/components/src/placeholder/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/placeholder/README.md b/packages/components/src/placeholder/README.md index bc59b593b463a..f260ab15378f0 100644 --- a/packages/components/src/placeholder/README.md +++ b/packages/components/src/placeholder/README.md @@ -39,7 +39,7 @@ Changes placeholder children layout from flex-row to flex-column. Title of the placeholder. -- Required: Yes +- Required: No ### `notices`: `ReactNode` From bfaec6f18c4e60bb52ffe062e6ec47852a3c8a60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petter=20Walb=C3=B8=20Johnsg=C3=A5rd?= Date: Thu, 11 Aug 2022 19:48:05 +0200 Subject: [PATCH 5/5] Optimize test to not use querySelector and add comments where needed --- .../components/src/placeholder/test/index.tsx | 42 +++++++++++++------ 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/packages/components/src/placeholder/test/index.tsx b/packages/components/src/placeholder/test/index.tsx index 0107435624df6..9e47d4d016a7f 100644 --- a/packages/components/src/placeholder/test/index.tsx +++ b/packages/components/src/placeholder/test/index.tsx @@ -6,8 +6,8 @@ import { render, screen, within } from '@testing-library/react'; /** * WordPress dependencies */ -import { more } from '@wordpress/icons'; import { useResizeObserver } from '@wordpress/compose'; +import { SVG, Path } from '@wordpress/primitives'; /** * Internal dependencies @@ -23,6 +23,15 @@ jest.mock( '@wordpress/compose', () => { }; } ); +/** + * Test icon that can be queried by `getByTestId` + */ +const testIcon = ( + + + +); + const Placeholder = ( props: Omit< WordPressComponentProps< PlaceholderProps< unknown >, 'div', false >, @@ -32,11 +41,6 @@ const Placeholder = ( const getPlaceholder = () => screen.getByTestId( 'placeholder' ); -const getLabel = () => { - const placeholder = getPlaceholder(); - return placeholder.querySelector( '.components-placeholder__label' ); -}; - describe( 'Placeholder', () => { beforeEach( () => { // @ts-ignore @@ -53,12 +57,16 @@ describe( 'Placeholder', () => { expect( placeholder ).toHaveClass( 'components-placeholder' ); - // Test for empty label. - const label = getLabel(); + // Test for empty label. When the label is empty, the only way to + // query the div is with `querySelector`. + const label = placeholder.querySelector( + '.components-placeholder__label' + ); expect( label ).toBeInTheDocument(); expect( label ).toBeEmptyDOMElement(); - // Test for non existent instructions. + // Test for non existent instructions. When the instructions is + // empty, the only way to query the div is with `querySelector`. const placeholderInstructions = placeholder.querySelector( '.components-placeholder__instructions' ); @@ -72,18 +80,25 @@ describe( 'Placeholder', () => { } ); it( 'should render an Icon in the label section', () => { - render( ); - const icon = getLabel()?.querySelector( 'svg' ); + render( ); + const placeholder = getPlaceholder(); + const icon = within( placeholder ).getByTestId( 'icon' ); + expect( icon.parentNode ).toHaveClass( + 'components-placeholder__label' + ); expect( icon ).toBeInTheDocument(); } ); it( 'should render a label section', () => { const label = 'WordPress'; render( ); - const placeholderLabel = getLabel(); + const placeholderLabel = screen.getByText( label ); - expect( placeholderLabel ).toHaveTextContent( label ); + expect( placeholderLabel ).toHaveClass( + 'components-placeholder__label' + ); + expect( placeholderLabel ).toBeInTheDocument(); } ); it( 'should display a fieldset from the children property', () => { @@ -113,6 +128,7 @@ describe( 'Placeholder', () => { render( ); const placeholder = getPlaceholder(); + expect( placeholder ).toHaveClass( 'components-placeholder' ); expect( placeholder ).toHaveClass( 'wp-placeholder' ); } );