From 2ad0daaed57d0846e68a8a3837a9af32e2a8b530 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 31 Dec 2023 19:59:56 +0100 Subject: [PATCH 1/2] Fix warning in AppUpdateBanner --- src/common/AppUpdateBanner.tsx | 13 +++---- test/app/App.test.tsx | 20 ++++++----- test/common/AppUpdateBanner.test.tsx | 19 ++++++---- test/servers/DeleteServerModal.test.tsx | 47 ++++++++++++------------- 4 files changed, 54 insertions(+), 45 deletions(-) diff --git a/src/common/AppUpdateBanner.tsx b/src/common/AppUpdateBanner.tsx index 06470b7b..9e58e575 100644 --- a/src/common/AppUpdateBanner.tsx +++ b/src/common/AppUpdateBanner.tsx @@ -1,7 +1,8 @@ import { faSyncAlt as reloadIcon } from '@fortawesome/free-solid-svg-icons'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { SimpleCard, useToggle } from '@shlinkio/shlink-frontend-kit'; -import type { FC, MouseEventHandler } from 'react'; +import type { MouseEventHandler } from 'react'; +import { forwardRef, useCallback } from 'react'; import { Alert, Button } from 'reactstrap'; import './AppUpdateBanner.scss'; @@ -11,15 +12,15 @@ interface AppUpdateBannerProps { forceUpdate: Function; } -export const AppUpdateBanner: FC = ({ isOpen, toggle, forceUpdate }) => { +export const AppUpdateBanner = forwardRef(({ isOpen, toggle, forceUpdate }, ref) => { const [isUpdating,, setUpdating] = useToggle(); - const update = () => { + const update = useCallback(() => { setUpdating(); forceUpdate(); - }; + }, [forceUpdate, setUpdating]); return ( - +

This app has just been updated!

Restart it to enjoy the new features. @@ -30,4 +31,4 @@ export const AppUpdateBanner: FC = ({ isOpen, toggle, forc

); -}; +}); diff --git a/test/app/App.test.tsx b/test/app/App.test.tsx index 6a2b043a..5616b3f8 100644 --- a/test/app/App.test.tsx +++ b/test/app/App.test.tsx @@ -1,4 +1,4 @@ -import { render, screen } from '@testing-library/react'; +import { act, render, screen, waitFor } from '@testing-library/react'; import { fromPartial } from '@total-typescript/shoehorn'; import { MemoryRouter } from 'react-router-dom'; import { AppFactory } from '../../src/app/App'; @@ -17,7 +17,7 @@ describe('', () => { ShlinkVersionsContainer: () => <>ShlinkVersions, }), ); - const setUp = (activeRoute = '/') => render( + const setUp = async (activeRoute = '/') => act(() => render( {}} @@ -27,15 +27,17 @@ describe('', () => { resetAppUpdate={() => {}} /> , - ); + )); it('passes a11y checks', () => checkAccessibility(setUp())); - it('renders children components', () => { - setUp(); + it('renders children components', async () => { + await setUp(); expect(screen.getByText('MainHeader')).toBeInTheDocument(); expect(screen.getByText('ShlinkVersions')).toBeInTheDocument(); + + await waitFor(() => screen.getByRole('alert')); expect(screen.getByText('This app has just been updated!')).toBeInTheDocument(); }); @@ -50,16 +52,16 @@ describe('', () => { ['/server/def456/bar', 'ShlinkWebComponentContainer'], ['/other', 'Oops! We could not find requested route.'], ])('renders expected route', async (activeRoute, expectedComponent) => { - setUp(activeRoute); - expect(await screen.findByText(expectedComponent)).toBeInTheDocument(); + await setUp(activeRoute); + expect(screen.getByText(expectedComponent)).toBeInTheDocument(); }); it.each([ ['/foo', 'shlink-wrapper'], ['/bar', 'shlink-wrapper'], ['/', 'shlink-wrapper d-flex d-md-block align-items-center'], - ])('renders expected classes on shlink-wrapper based on current pathname', (pathname, expectedClasses) => { - const { container } = setUp(pathname); + ])('renders expected classes on shlink-wrapper based on current pathname', async (pathname, expectedClasses) => { + const { container } = await setUp(pathname); const shlinkWrapper = container.querySelector('.shlink-wrapper'); expect(shlinkWrapper).toHaveAttribute('class', expectedClasses); diff --git a/test/common/AppUpdateBanner.test.tsx b/test/common/AppUpdateBanner.test.tsx index bdffe572..e98ad2cb 100644 --- a/test/common/AppUpdateBanner.test.tsx +++ b/test/common/AppUpdateBanner.test.tsx @@ -1,4 +1,4 @@ -import { screen } from '@testing-library/react'; +import { act, screen, waitFor } from '@testing-library/react'; import { AppUpdateBanner } from '../../src/common/AppUpdateBanner'; import { checkAccessibility } from '../__helpers__/accessibility'; import { renderWithEvents } from '../__helpers__/setUpTest'; @@ -6,12 +6,19 @@ import { renderWithEvents } from '../__helpers__/setUpTest'; describe('', () => { const toggle = vi.fn(); const forceUpdate = vi.fn(); - const setUp = () => renderWithEvents(); + const setUp = async () => { + const result = await act( + () => renderWithEvents(), + ); + await waitFor(() => screen.getByRole('alert')); + + return result; + }; it('passes a11y checks', () => checkAccessibility(setUp())); - it('renders initial state', () => { - setUp(); + it('renders initial state', async () => { + await setUp(); expect(screen.getByRole('heading')).toHaveTextContent('This app has just been updated!'); expect(screen.queryByText('Restarting...')).not.toBeInTheDocument(); @@ -19,7 +26,7 @@ describe('', () => { }); it('invokes toggle when alert is closed', async () => { - const { user } = setUp(); + const { user } = await setUp(); expect(toggle).not.toHaveBeenCalled(); await user.click(screen.getByLabelText('Close')); @@ -27,7 +34,7 @@ describe('', () => { }); it('triggers the update when clicking the button', async () => { - const { user } = setUp(); + const { user } = await setUp(); expect(forceUpdate).not.toHaveBeenCalled(); await user.click(screen.getByText(/^Restart now/)); diff --git a/test/servers/DeleteServerModal.test.tsx b/test/servers/DeleteServerModal.test.tsx index af228aad..e8d7d9e5 100644 --- a/test/servers/DeleteServerModal.test.tsx +++ b/test/servers/DeleteServerModal.test.tsx @@ -1,4 +1,4 @@ -import { screen, waitFor } from '@testing-library/react'; +import { act, screen, waitFor } from '@testing-library/react'; import { fromPartial } from '@total-typescript/shoehorn'; import { createMemoryHistory } from 'history'; import { Router } from 'react-router-dom'; @@ -10,37 +10,36 @@ import { TestModalWrapper } from '../__helpers__/TestModalWrapper'; describe('', () => { const deleteServerMock = vi.fn(); const serverName = 'the_server_name'; - const setUp = () => { + const setUp = async () => { const history = createMemoryHistory({ initialEntries: ['/foo'] }); - return { - history, - ...renderWithEvents( - - ( - - )} - /> - , - ), - }; + const result = await act(() => renderWithEvents( + + ( + + )} + /> + , + )); + + return { history, ...result }; }; it('passes a11y checks', () => checkAccessibility(setUp())); - it('renders a modal window', () => { - setUp(); + it('renders a modal window', async () => { + await setUp(); expect(screen.getByRole('dialog')).toBeInTheDocument(); expect(screen.getByRole('heading')).toHaveTextContent('Remove server'); }); - it('displays the name of the server as part of the content', () => { - setUp(); + it('displays the name of the server as part of the content', async () => { + await setUp(); expect(screen.getByText(/^Are you sure you want to remove/)).toBeInTheDocument(); expect(screen.getByText(serverName)).toBeInTheDocument(); @@ -50,7 +49,7 @@ describe('', () => { [() => screen.getByRole('button', { name: 'Cancel' })], [() => screen.getByLabelText('Close')], ])('toggles when clicking cancel button', async (getButton) => { - const { user, history } = setUp(); + const { user, history } = await setUp(); expect(history.location.pathname).toEqual('/foo'); await user.click(getButton()); @@ -60,7 +59,7 @@ describe('', () => { }); it('deletes server when clicking accept button', async () => { - const { user, history } = setUp(); + const { user, history } = await setUp(); expect(deleteServerMock).not.toHaveBeenCalled(); expect(history.location.pathname).toEqual('/foo'); From e7fab8c2d6e55b58339a0226079e0cb16ad7b1cf Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 31 Dec 2023 20:16:06 +0100 Subject: [PATCH 2/2] Fix remaining act-related warningsin tests --- test/app/App.test.tsx | 7 ++---- .../helpers/DuplicatedServersModal.test.tsx | 22 +++++++++---------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/test/app/App.test.tsx b/test/app/App.test.tsx index 5616b3f8..2be2b3c4 100644 --- a/test/app/App.test.tsx +++ b/test/app/App.test.tsx @@ -1,4 +1,4 @@ -import { act, render, screen, waitFor } from '@testing-library/react'; +import { act, render, screen } from '@testing-library/react'; import { fromPartial } from '@total-typescript/shoehorn'; import { MemoryRouter } from 'react-router-dom'; import { AppFactory } from '../../src/app/App'; @@ -23,7 +23,7 @@ describe('', () => { fetchServers={() => {}} servers={{}} settings={fromPartial({})} - appUpdated + appUpdated={false} resetAppUpdate={() => {}} /> , @@ -36,9 +36,6 @@ describe('', () => { expect(screen.getByText('MainHeader')).toBeInTheDocument(); expect(screen.getByText('ShlinkVersions')).toBeInTheDocument(); - - await waitFor(() => screen.getByRole('alert')); - expect(screen.getByText('This app has just been updated!')).toBeInTheDocument(); }); it.each([ diff --git a/test/servers/helpers/DuplicatedServersModal.test.tsx b/test/servers/helpers/DuplicatedServersModal.test.tsx index 7c838701..99a99846 100644 --- a/test/servers/helpers/DuplicatedServersModal.test.tsx +++ b/test/servers/helpers/DuplicatedServersModal.test.tsx @@ -1,4 +1,4 @@ -import { screen } from '@testing-library/react'; +import { act, screen } from '@testing-library/react'; import { fromPartial } from '@total-typescript/shoehorn'; import type { ServerData } from '../../../src/servers/data'; import { DuplicatedServersModal } from '../../../src/servers/helpers/DuplicatedServersModal'; @@ -8,9 +8,9 @@ import { renderWithEvents } from '../../__helpers__/setUpTest'; describe('', () => { const onDiscard = vi.fn(); const onSave = vi.fn(); - const setUp = (duplicatedServers: ServerData[] = []) => renderWithEvents( + const setUp = (duplicatedServers: ServerData[] = []) => act(() => renderWithEvents( , - ); + )); const mockServer = (data: Partial = {}) => fromPartial(data); it('passes a11y checks', () => checkAccessibility(setUp())); @@ -21,8 +21,8 @@ describe('', () => { [[mockServer(), mockServer()], 2], [[mockServer(), mockServer(), mockServer()], 3], [[mockServer(), mockServer(), mockServer(), mockServer()], 4], - ])('renders expected amount of items', (duplicatedServers, expectedItems) => { - setUp(duplicatedServers); + ])('renders expected amount of items', async (duplicatedServers, expectedItems) => { + await setUp(duplicatedServers); expect(screen.queryAllByRole('listitem')).toHaveLength(expectedItems); }); @@ -45,8 +45,8 @@ describe('', () => { discardBtn: 'Ignore duplicates', }, ], - ])('renders expected texts based on amount of servers', (duplicatedServers, assertions) => { - setUp(duplicatedServers); + ])('renders expected texts based on amount of servers', async (duplicatedServers, assertions) => { + await setUp(duplicatedServers); expect(screen.getByRole('heading')).toHaveTextContent(assertions.header); expect(screen.getByText(assertions.firstParagraph)).toBeInTheDocument(); @@ -61,8 +61,8 @@ describe('', () => { mockServer({ url: 'url_1', apiKey: 'apiKey_1' }), mockServer({ url: 'url_2', apiKey: 'apiKey_2' }), ]], - ])('displays provided server data', (duplicatedServers) => { - setUp(duplicatedServers); + ])('displays provided server data', async (duplicatedServers) => { + await setUp(duplicatedServers); if (duplicatedServers.length === 0) { expect(screen.queryByRole('listitem')).not.toBeInTheDocument(); @@ -81,7 +81,7 @@ describe('', () => { }); it('invokes onDiscard when appropriate button is clicked', async () => { - const { user } = setUp(); + const { user } = await setUp(); expect(onDiscard).not.toHaveBeenCalled(); await user.click(screen.getByRole('button', { name: 'Discard' })); @@ -89,7 +89,7 @@ describe('', () => { }); it('invokes onSave when appropriate button is clicked', async () => { - const { user } = setUp(); + const { user } = await setUp(); expect(onSave).not.toHaveBeenCalled(); await user.click(screen.getByRole('button', { name: 'Save anyway' }));