-
-
Notifications
You must be signed in to change notification settings - Fork 697
test: migrate to Vitest fake timers, remove @testing-library/user-event, and resolve act warnings #3147
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.
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
base: main
Are you sure you want to change the base?
test: migrate to Vitest fake timers, remove @testing-library/user-event, and resolve act warnings #3147
Changes from 48 commits
93723d9
9c44611
c93671a
c871edf
1c953e2
a06801a
f6140d4
1e1686e
5a0345a
f9fad21
f94ed77
56c178a
70ee812
d046470
f5c7ea2
ecabe83
aeee4dd
00dd86f
d6fb6f0
54a94c4
6dad0b2
61be143
54e3df2
5a3a628
1c76bce
801f462
a84617a
501c99c
69f6dbd
118a594
0a7f38d
899d135
96bfcc6
117443d
44e2422
a3fed86
d693163
521c20f
a537d89
a9a4abf
105be28
db93174
66b310c
90b902e
01ac25b
528d028
1696533
03add44
ed61b2f
16e6546
9c85c41
d697f23
a3bf362
99c839e
54d053f
0124c86
cae2ff2
7647031
eb4b6b4
278abe2
aaef034
b8c465c
db138c3
f8cf85e
f49290e
3f49240
ffda141
1442388
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,22 +1,24 @@ | ||
| import { StrictMode, Suspense, useState } from 'react' | ||
| import { act, render, screen, waitFor } from '@testing-library/react' | ||
| import userEventOrig from '@testing-library/user-event' | ||
| import { describe, expect, it } from 'vitest' | ||
| import { act, fireEvent, render, screen } from '@testing-library/react' | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' | ||
| import { useAtomValue, useSetAtom } from 'jotai/react' | ||
| import { atom } from 'jotai/vanilla' | ||
|
|
||
| const userEvent = { | ||
| click: (element: Element) => act(() => userEventOrig.click(element)), | ||
| } | ||
| beforeEach(() => { | ||
| vi.useFakeTimers() | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| vi.useRealTimers() | ||
| }) | ||
|
|
||
| describe('abortable atom test', () => { | ||
| it('can abort with signal.aborted', async () => { | ||
| const countAtom = atom(0) | ||
| let abortedCount = 0 | ||
| const resolve: (() => void)[] = [] | ||
| const derivedAtom = atom(async (get, { signal }) => { | ||
| const count = get(countAtom) | ||
| await new Promise<void>((r) => resolve.push(r)) | ||
| await new Promise((resolve) => setTimeout(resolve, 100)) | ||
| if (signal.aborted) { | ||
| ++abortedCount | ||
| } | ||
|
|
@@ -37,47 +39,51 @@ describe('abortable atom test', () => { | |
| ) | ||
| } | ||
|
|
||
| await act(async () => { | ||
| await act(() => | ||
| render( | ||
| <StrictMode> | ||
| <Suspense fallback="loading"> | ||
| <Suspense fallback={<div>loading</div>}> | ||
| <Component /> | ||
| <Controls /> | ||
| </Suspense> | ||
| </StrictMode>, | ||
| ) | ||
| }) | ||
| ), | ||
| ) | ||
|
|
||
| expect(await screen.findByText('loading')).toBeInTheDocument() | ||
| expect(screen.getByText('loading')).toBeInTheDocument() | ||
|
|
||
| await act(() => vi.advanceTimersByTimeAsync(100)) | ||
| expect(screen.getByText('count: 0')).toBeInTheDocument() | ||
|
|
||
| resolve.splice(0).forEach((fn) => fn()) | ||
| expect(await screen.findByText('count: 0')).toBeInTheDocument() | ||
| expect(abortedCount).toBe(0) | ||
|
|
||
| await userEvent.click(screen.getByText('button')) | ||
| await userEvent.click(screen.getByText('button')) | ||
| resolve.splice(0).forEach((fn) => fn()) | ||
| expect(await screen.findByText('count: 2')).toBeInTheDocument() | ||
| await act(() => fireEvent.click(screen.getByText('button'))) | ||
| expect(screen.getByText('loading')).toBeInTheDocument() | ||
| await act(() => fireEvent.click(screen.getByText('button'))) | ||
| expect(screen.getByText('loading')).toBeInTheDocument() | ||
| await act(() => vi.advanceTimersByTimeAsync(100)) | ||
| expect(screen.getByText('count: 2')).toBeInTheDocument() | ||
|
|
||
| expect(abortedCount).toBe(1) | ||
|
|
||
| await userEvent.click(screen.getByText('button')) | ||
| resolve.splice(0).forEach((fn) => fn()) | ||
| expect(await screen.findByText('count: 3')).toBeInTheDocument() | ||
| await act(() => fireEvent.click(screen.getByText('button'))) | ||
| expect(screen.getByText('loading')).toBeInTheDocument() | ||
| await act(() => vi.advanceTimersByTimeAsync(100)) | ||
| expect(screen.getByText('count: 3')).toBeInTheDocument() | ||
|
|
||
| expect(abortedCount).toBe(1) | ||
| }) | ||
|
|
||
| it('can abort with event listener', async () => { | ||
| const countAtom = atom(0) | ||
| let abortedCount = 0 | ||
| const resolve: (() => void)[] = [] | ||
| const derivedAtom = atom(async (get, { signal }) => { | ||
| const count = get(countAtom) | ||
| const callback = () => { | ||
| ++abortedCount | ||
| } | ||
| signal.addEventListener('abort', callback) | ||
| await new Promise<void>((r) => resolve.push(r)) | ||
| await new Promise((resolve) => setTimeout(resolve, 100)) | ||
| signal.removeEventListener('abort', callback) | ||
| return count | ||
| }) | ||
|
|
@@ -96,44 +102,47 @@ describe('abortable atom test', () => { | |
| ) | ||
| } | ||
|
|
||
| await act(async () => { | ||
| await act(() => | ||
| render( | ||
| <StrictMode> | ||
| <Suspense fallback="loading"> | ||
| <Suspense fallback={<div>loading</div>}> | ||
| <Component /> | ||
| <Controls /> | ||
| </Suspense> | ||
| </StrictMode>, | ||
| ) | ||
| }) | ||
| ), | ||
| ) | ||
|
|
||
| expect(await screen.findByText('loading')).toBeInTheDocument() | ||
| resolve.splice(0).forEach((fn) => fn()) | ||
| expect(await screen.findByText('count: 0')).toBeInTheDocument() | ||
| expect(screen.getByText('loading')).toBeInTheDocument() | ||
|
|
||
| await act(() => vi.advanceTimersByTimeAsync(100)) | ||
| expect(screen.getByText('count: 0')).toBeInTheDocument() | ||
|
|
||
| expect(abortedCount).toBe(0) | ||
|
|
||
| await userEvent.click(screen.getByText('button')) | ||
| await userEvent.click(screen.getByText('button')) | ||
| resolve.splice(0).forEach((fn) => fn()) | ||
| expect(await screen.findByText('count: 2')).toBeInTheDocument() | ||
| await act(() => fireEvent.click(screen.getByText('button'))) | ||
| expect(screen.getByText('loading')).toBeInTheDocument() | ||
| await act(() => fireEvent.click(screen.getByText('button'))) | ||
| expect(screen.getByText('loading')).toBeInTheDocument() | ||
| await act(() => vi.advanceTimersByTimeAsync(100)) | ||
| expect(screen.getByText('count: 2')).toBeInTheDocument() | ||
|
|
||
| expect(abortedCount).toBe(1) | ||
|
|
||
| await userEvent.click(screen.getByText('button')) | ||
| resolve.splice(0).forEach((fn) => fn()) | ||
| expect(await screen.findByText('count: 3')).toBeInTheDocument() | ||
| await act(() => fireEvent.click(screen.getByText('button'))) | ||
| expect(screen.getByText('loading')).toBeInTheDocument() | ||
| await act(() => vi.advanceTimersByTimeAsync(100)) | ||
| expect(screen.getByText('count: 3')).toBeInTheDocument() | ||
|
|
||
| expect(abortedCount).toBe(1) | ||
| }) | ||
|
|
||
| it('does not abort on unmount', async () => { | ||
| const countAtom = atom(0) | ||
| let abortedCount = 0 | ||
| const resolve: (() => void)[] = [] | ||
| const derivedAtom = atom(async (get, { signal }) => { | ||
| const count = get(countAtom) | ||
| await new Promise<void>((r) => resolve.push(r)) | ||
| await new Promise((resolve) => setTimeout(resolve, 100)) | ||
| if (signal.aborted) { | ||
| ++abortedCount | ||
| } | ||
|
|
@@ -157,37 +166,36 @@ describe('abortable atom test', () => { | |
| ) | ||
| } | ||
|
|
||
| await act(async () => { | ||
| await act(() => | ||
| render( | ||
| <StrictMode> | ||
| <Suspense fallback="loading"> | ||
| <Suspense fallback={<div>loading</div>}> | ||
| <Parent /> | ||
| </Suspense> | ||
| </StrictMode>, | ||
| ) | ||
| }) | ||
| ), | ||
| ) | ||
|
|
||
| expect(await screen.findByText('loading')).toBeInTheDocument() | ||
| expect(screen.getByText('loading')).toBeInTheDocument() | ||
|
|
||
| resolve.splice(0).forEach((fn) => fn()) | ||
| expect(await screen.findByText('count: 0')).toBeInTheDocument() | ||
| expect(abortedCount).toBe(0) | ||
| await act(() => vi.advanceTimersByTimeAsync(100)) | ||
| expect(screen.getByText('count: 0')).toBeInTheDocument() | ||
|
|
||
| await userEvent.click(screen.getByText('button')) | ||
| await userEvent.click(screen.getByText('toggle')) | ||
| expect(abortedCount).toBe(0) | ||
|
||
|
|
||
| expect(await screen.findByText('hidden')).toBeInTheDocument() | ||
| await act(() => fireEvent.click(screen.getByText('button'))) | ||
| expect(screen.getByText('loading')).toBeInTheDocument() | ||
| await act(() => fireEvent.click(screen.getByText('toggle'))) | ||
| expect(screen.getByText('hidden')).toBeInTheDocument() | ||
|
|
||
| resolve.splice(0).forEach((fn) => fn()) | ||
| await waitFor(() => expect(abortedCount).toBe(0)) | ||
| expect(abortedCount).toBe(0) | ||
| }) | ||
|
|
||
| it('throws aborted error (like fetch)', async () => { | ||
| const countAtom = atom(0) | ||
| const resolve: (() => void)[] = [] | ||
| const derivedAtom = atom(async (get, { signal }) => { | ||
| const count = get(countAtom) | ||
| await new Promise<void>((r) => resolve.push(r)) | ||
| await new Promise((resolve) => setTimeout(resolve, 100)) | ||
| if (signal.aborted) { | ||
| throw new Error('aborted') | ||
| } | ||
|
|
@@ -208,29 +216,32 @@ describe('abortable atom test', () => { | |
| ) | ||
| } | ||
|
|
||
| await act(async () => { | ||
| await act(() => | ||
| render( | ||
| <StrictMode> | ||
| <Suspense fallback="loading"> | ||
| <Suspense fallback={<div>loading</div>}> | ||
| <Component /> | ||
| <Controls /> | ||
| </Suspense> | ||
| </StrictMode>, | ||
| ) | ||
| }) | ||
| ), | ||
| ) | ||
|
|
||
| expect(await screen.findByText('loading')).toBeInTheDocument() | ||
| expect(screen.getByText('loading')).toBeInTheDocument() | ||
|
|
||
| resolve.splice(0).forEach((fn) => fn()) | ||
| expect(await screen.findByText('count: 0')).toBeInTheDocument() | ||
| await act(() => vi.advanceTimersByTimeAsync(100)) | ||
| expect(screen.getByText('count: 0')).toBeInTheDocument() | ||
|
|
||
| await userEvent.click(screen.getByText('button')) | ||
| await userEvent.click(screen.getByText('button')) | ||
| resolve.splice(0).forEach((fn) => fn()) | ||
| expect(await screen.findByText('count: 2')).toBeInTheDocument() | ||
| await act(() => fireEvent.click(screen.getByText('button'))) | ||
| expect(screen.getByText('loading')).toBeInTheDocument() | ||
| await act(() => fireEvent.click(screen.getByText('button'))) | ||
| expect(screen.getByText('loading')).toBeInTheDocument() | ||
| await act(() => vi.advanceTimersByTimeAsync(100)) | ||
| expect(screen.getByText('count: 2')).toBeInTheDocument() | ||
|
|
||
| await userEvent.click(screen.getByText('button')) | ||
| resolve.splice(0).forEach((fn) => fn()) | ||
| expect(await screen.findByText('count: 3')).toBeInTheDocument() | ||
| await act(() => fireEvent.click(screen.getByText('button'))) | ||
| expect(screen.getByText('loading')).toBeInTheDocument() | ||
| await act(() => vi.advanceTimersByTimeAsync(100)) | ||
| expect(screen.getByText('count: 3')).toBeInTheDocument() | ||
| }) | ||
| }) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change important? If so, can you create another PR? We would like to reduce diffs so that it reduces the human brain power for the review.
If you use AI coding assistant, asking them to reduce
git diff -woutput might be helpful.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dai-shi Changed all
Suspensefallback from<div>loading</div>to"loading"for simplicity. Also changederror.test.tsxfromfallback={null}tofallback="loading"since null fallback doesn't provide meaningful loading state in 3f49240.