From edf66c25a22981175b847b36ddac41f5c7d17852 Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Thu, 2 Feb 2023 15:36:30 +0200 Subject: [PATCH 1/2] Components: Remove unnecessary act() from Popover tests --- .../components/src/popover/test/index.tsx | 49 +++++++++++-------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/packages/components/src/popover/test/index.tsx b/packages/components/src/popover/test/index.tsx index 5dfadd35cc0a16..aac44bb170926b 100644 --- a/packages/components/src/popover/test/index.tsx +++ b/packages/components/src/popover/test/index.tsx @@ -1,8 +1,8 @@ /** * External dependencies */ -import { act, render, screen } from '@testing-library/react'; -import type { CSSProperties, ReactElement } from 'react'; +import { render, screen, waitFor } from '@testing-library/react'; +import type { CSSProperties } from 'react'; /** * WordPress dependencies @@ -91,25 +91,21 @@ const ALL_POSITIONS_TO_EXPECTED_PLACEMENTS: PositionToPlacementTuple[] = [ describe( 'Popover', () => { describe( 'Component', () => { - // Render UI and then wait for the `floating-ui` effects inside `Popover` to finish running - // See also: https://floating-ui.com/docs/react-dom#testing - async function renderAsync( ui: ReactElement ) { - const view = render( ui ); - await act( () => Promise.resolve() ); - return view; - } - describe( 'basic behavior', () => { it( 'should render content', async () => { - await renderAsync( Hello ); + render( Hello ); - expect( screen.getByText( 'Hello' ) ).toBeInTheDocument(); + await waitFor( () => + expect( screen.getByText( 'Hello' ) ).toBeVisible() + ); } ); it( 'should forward additional props to portaled element', async () => { - await renderAsync( Hello ); + render( Hello ); - expect( screen.getByRole( 'tooltip' ) ).toBeInTheDocument(); + await waitFor( () => + expect( screen.getByRole( 'tooltip' ) ).toBeVisible() + ); } ); } ); @@ -129,32 +125,43 @@ describe( 'Popover', () => { ); }; - await renderAsync( + render( Popover content ); - expect( - screen.getByText( 'Popover content' ) - ).toBeInTheDocument(); + await waitFor( () => + expect( + screen.getByText( 'Popover content' ) + ).toBeVisible() + ); } ); } ); describe( 'focus behavior', () => { it( 'should focus the popover by default when opened', async () => { - await renderAsync( + render( Popover content ); - expect( screen.getByTestId( 'popover-element' ) ).toHaveFocus(); + const popover = screen.getByTestId( 'popover-element' ); + + await waitFor( () => expect( popover ).toBeVisible() ); + await waitFor( () => expect( popover ).toHaveFocus() ); } ); it( 'should allow focus-on-open behavior to be disabled', async () => { - await renderAsync( + render( Popover content ); + await waitFor( () => + expect( + screen.getByText( 'Popover content' ) + ).toBeVisible() + ); + expect( document.body ).toHaveFocus(); } ); } ); From a9bd72bc5711c012acaed538f9f5b98f89c22b9d Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Fri, 3 Feb 2023 11:38:14 +0200 Subject: [PATCH 2/2] No need to await for the focusing --- packages/components/src/popover/test/index.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/components/src/popover/test/index.tsx b/packages/components/src/popover/test/index.tsx index aac44bb170926b..7497a705112ace 100644 --- a/packages/components/src/popover/test/index.tsx +++ b/packages/components/src/popover/test/index.tsx @@ -148,7 +148,8 @@ describe( 'Popover', () => { const popover = screen.getByTestId( 'popover-element' ); await waitFor( () => expect( popover ).toBeVisible() ); - await waitFor( () => expect( popover ).toHaveFocus() ); + + expect( popover ).toHaveFocus(); } ); it( 'should allow focus-on-open behavior to be disabled', async () => {