Skip to content
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.

Sign up for GitHub

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 more tests to use real timers #47318

Merged
merged 1 commit into from
Jan 31, 2023
Merged

Components: Refactor more tests to use real timers #47318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 43 additions & 44 deletions packages/block-editor/src/components/block-switcher/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { act, render, screen, within } from '@testing-library/react';
import { render, screen, within, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

/**
Expand All @@ -16,8 +16,6 @@ import { copy } from '@wordpress/icons';
*/
import { BlockSwitcher, BlockSwitcherDropdownMenu } from '../';

jest.useFakeTimers();

jest.mock( '@wordpress/data/src/components/use-select', () => jest.fn() );
jest.mock( '../../block-title/use-block-display-title', () =>
jest.fn().mockReturnValue( 'Block Name' )
Expand Down Expand Up @@ -185,9 +183,7 @@ describe( 'BlockSwitcherDropdownMenu', () => {
} );

test( 'should simulate a keydown event, which should open transform toggle.', async () => {
const user = userEvent.setup( {
advanceTimers: jest.advanceTimersByTime,
} );
const user = userEvent.setup();

render(
<BlockSwitcherDropdownMenu blocks={ [ headingBlock1 ] } />
Expand All @@ -212,26 +208,27 @@ describe( 'BlockSwitcherDropdownMenu', () => {
} ),
'[ArrowDown]'
);
await act( () => Promise.resolve() );

expect(
screen.getByRole( 'button', {
name: 'Block Name',
expanded: true,
} )
).toBeVisible();
await waitFor( () =>
expect(
screen.getByRole( 'button', {
name: 'Block Name',
expanded: true,
} )
).toBeVisible()
);

const menu = screen.getByRole( 'menu', {
name: 'Block Name',
} );
expect( menu ).toBeInTheDocument();
expect( menu ).not.toBeVisible();
await waitFor( () =>
expect(
screen.getByRole( 'menu', {
name: 'Block Name',
} )
).toBeVisible()
);
Comment on lines -224 to +227
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as previous comment

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replied there

} );

test( 'should simulate a click event, which should call onToggle.', async () => {
const user = userEvent.setup( {
advanceTimers: jest.advanceTimersByTime,
} );
const user = userEvent.setup();

render(
<BlockSwitcherDropdownMenu blocks={ [ headingBlock1 ] } />
Expand All @@ -255,26 +252,27 @@ describe( 'BlockSwitcherDropdownMenu', () => {
expanded: false,
} )
);
await act( () => Promise.resolve() );

expect(
screen.getByRole( 'button', {
name: 'Block Name',
expanded: true,
} )
).toBeVisible();
await waitFor( () =>
expect(
screen.getByRole( 'button', {
name: 'Block Name',
expanded: true,
} )
).toBeVisible()
);

const menu = screen.getByRole( 'menu', {
name: 'Block Name',
} );
expect( menu ).toBeInTheDocument();
expect( menu ).not.toBeVisible();
await waitFor( () =>
expect(
screen.getByRole( 'menu', {
name: 'Block Name',
} )
).toBeVisible()
);
Comment on lines -267 to +271
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand these changes — previously the test was checking for the menu to be in the document, but not visible, while the new version of the test seems to check that the menu is visible.

Can you explain the rationale here? Thank you!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous test was just wrong. If you manually go through the steps, you'll see that both the button and the menu should be visible. The previous test just wasn't properly waiting for the menu to be revealed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦

} );

test( 'should create the transform items for the chosen block.', async () => {
const user = userEvent.setup( {
advanceTimers: jest.advanceTimersByTime,
} );
const user = userEvent.setup();

render(
<BlockSwitcherDropdownMenu blocks={ [ headingBlock1 ] } />
Expand All @@ -286,15 +284,16 @@ describe( 'BlockSwitcherDropdownMenu', () => {
expanded: false,
} )
);
await act( () => Promise.resolve() );

expect(
within(
screen.getByRole( 'menu', {
name: 'Block Name',
} )
).getByRole( 'menuitem' )
).toBeInTheDocument();
await waitFor( () =>
expect(
within(
screen.getByRole( 'menu', {
name: 'Block Name',
} )
).getByRole( 'menuitem' )
).toBeInTheDocument()
);
} );
} );
} );
48 changes: 24 additions & 24 deletions packages/components/src/higher-order/with-filters/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { act, render } from '@testing-library/react';
import { render, waitFor } from '@testing-library/react';

/**
* WordPress dependencies
Expand All @@ -13,8 +13,6 @@ import { addFilter, removeAllFilters, removeFilter } from '@wordpress/hooks';
*/
import withFilters from '..';

jest.useFakeTimers();

describe( 'withFilters', () => {
const hookName = 'EnhancedComponent';
const MyComponent = () => <div>My component</div>;
Expand Down Expand Up @@ -65,7 +63,7 @@ describe( 'withFilters', () => {
expect( container ).toMatchSnapshot();
} );

it( 'should not re-render component when new filter added before component was mounted', () => {
it( 'should not re-render component when new filter added before component was mounted', async () => {
const SpiedComponent = jest.fn( () => <div>Spied component</div> );
addFilter(
hookName,
Expand All @@ -81,13 +79,13 @@ describe( 'withFilters', () => {

const { container } = render( <EnhancedComponent /> );

act( () => jest.runAllTimers() );

expect( SpiedComponent ).toHaveBeenCalledTimes( 1 );
await waitFor( () =>
expect( SpiedComponent ).toHaveBeenCalledTimes( 1 )
);
expect( container ).toMatchSnapshot();
} );

it( 'should re-render component once when new filter added after component was mounted', () => {
it( 'should re-render component once when new filter added after component was mounted', async () => {
const SpiedComponent = jest.fn( () => <div>Spied component</div> );
const EnhancedComponent = withFilters( hookName )( SpiedComponent );

Expand All @@ -106,13 +104,13 @@ describe( 'withFilters', () => {
)
);

act( () => jest.runAllTimers() );

expect( SpiedComponent ).toHaveBeenCalledTimes( 1 );
await waitFor( () =>
expect( SpiedComponent ).toHaveBeenCalledTimes( 1 )
);
expect( container ).toMatchSnapshot();
} );

it( 'should re-render component once when two filters added in the same animation frame', () => {
it( 'should re-render component once when two filters added in the same animation frame', async () => {
const SpiedComponent = jest.fn( () => <div>Spied component</div> );
const EnhancedComponent = withFilters( hookName )( SpiedComponent );

Expand Down Expand Up @@ -141,13 +139,13 @@ describe( 'withFilters', () => {
)
);

act( () => jest.runAllTimers() );

expect( SpiedComponent ).toHaveBeenCalledTimes( 1 );
await waitFor( () =>
expect( SpiedComponent ).toHaveBeenCalledTimes( 1 )
);
expect( container ).toMatchSnapshot();
} );

it( 'should re-render component twice when new filter added and removed in two different animation frames', () => {
it( 'should re-render component twice when new filter added and removed in two different animation frames', async () => {
const SpiedComponent = jest.fn( () => <div>Spied component</div> );
const EnhancedComponent = withFilters( hookName )( SpiedComponent );
const { container } = render( <EnhancedComponent /> );
Expand All @@ -165,17 +163,19 @@ describe( 'withFilters', () => {
)
);

act( () => jest.runAllTimers() );
await waitFor( () =>
expect( SpiedComponent ).toHaveBeenCalledTimes( 1 )
);

removeFilter( hookName, 'test/enhanced-component-spy' );

act( () => jest.runAllTimers() );

expect( SpiedComponent ).toHaveBeenCalledTimes( 2 );
await waitFor( () =>
expect( SpiedComponent ).toHaveBeenCalledTimes( 2 )
);
expect( container ).toMatchSnapshot();
} );

it( 'should re-render both components once each when one filter added', () => {
it( 'should re-render both components once each when one filter added', async () => {
const SpiedComponent = jest.fn( () => <div>Spied component</div> );
const EnhancedComponent = withFilters( hookName )( SpiedComponent );

Expand All @@ -200,9 +200,9 @@ describe( 'withFilters', () => {
)
);

act( () => jest.runAllTimers() );

expect( SpiedComponent ).toHaveBeenCalledTimes( 2 );
await waitFor( () =>
expect( SpiedComponent ).toHaveBeenCalledTimes( 2 )
);
expect( container ).toMatchSnapshot();
} );
} );
36 changes: 11 additions & 25 deletions packages/components/src/higher-order/with-focus-outside/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { render, screen } from '@testing-library/react';
import { render, screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

/**
Expand All @@ -14,8 +14,6 @@ import { Component } from '@wordpress/element';
*/
import withFocusOutside from '../';

jest.useFakeTimers();

let onFocusOutside;

describe( 'withFocusOutside', () => {
Expand Down Expand Up @@ -57,7 +55,7 @@ describe( 'withFocusOutside', () => {
document.hasFocus = origHasFocus;
} );

it( 'should not call handler if focus shifts to element within component', () => {
it( 'should not call handler if focus shifts to element within component', async () => {
render( <TestComponent onFocusOutside={ onFocusOutside } /> );

const input = screen.getByRole( 'textbox' );
Expand All @@ -67,15 +65,11 @@ describe( 'withFocusOutside', () => {
input.blur();
button.focus();

jest.runAllTimers();

expect( onFocusOutside ).not.toHaveBeenCalled();
await waitFor( () => expect( onFocusOutside ).not.toHaveBeenCalled() );
} );

it( 'should not call handler if focus transitions via click to button', async () => {
const user = userEvent.setup( {
advanceTimers: jest.advanceTimersByTime,
} );
const user = userEvent.setup();
render( <TestComponent onFocusOutside={ onFocusOutside } /> );

const input = screen.getByRole( 'textbox' );
Expand All @@ -84,24 +78,20 @@ describe( 'withFocusOutside', () => {
input.focus();
await user.click( button );

jest.runAllTimers();

expect( onFocusOutside ).not.toHaveBeenCalled();
await waitFor( () => expect( onFocusOutside ).not.toHaveBeenCalled() );
} );

it( 'should call handler if focus doesn’t shift to element within component', () => {
it( 'should call handler if focus doesn’t shift to element within component', async () => {
render( <TestComponent onFocusOutside={ onFocusOutside } /> );

const input = screen.getByRole( 'textbox' );
input.focus();
input.blur();

jest.runAllTimers();

expect( onFocusOutside ).toHaveBeenCalled();
await waitFor( () => expect( onFocusOutside ).toHaveBeenCalled() );
} );

it( 'should not call handler if focus shifts outside the component when the document does not have focus', () => {
it( 'should not call handler if focus shifts outside the component when the document does not have focus', async () => {
render( <TestComponent onFocusOutside={ onFocusOutside } /> );

// Force document.hasFocus() to return false to simulate the window/document losing focus
Expand All @@ -112,12 +102,10 @@ describe( 'withFocusOutside', () => {
input.focus();
input.blur();

jest.runAllTimers();

expect( onFocusOutside ).not.toHaveBeenCalled();
await waitFor( () => expect( onFocusOutside ).not.toHaveBeenCalled() );
} );

it( 'should cancel check when unmounting while queued', () => {
it( 'should cancel check when unmounting while queued', async () => {
const { rerender } = render(
<TestComponent onFocusOutside={ onFocusOutside } />
);
Expand All @@ -128,8 +116,6 @@ describe( 'withFocusOutside', () => {

rerender( <div /> );

jest.runAllTimers();

expect( onFocusOutside ).not.toHaveBeenCalled();
await waitFor( () => expect( onFocusOutside ).not.toHaveBeenCalled() );
} );
} );
Loading