Skip to content

test: remove await from userEvent usage #182

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

Merged
merged 3 commits into from
Nov 28, 2020
Merged

test: remove await from userEvent usage #182

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a92fa74
test: remove await from userEvent usage
nickserv Nov 23, 2020
e93d6ed
test: fix test failure with waitFor
nickserv Nov 28, 2020
9c565ef
Merge branch 'master' into nickmccurdy-patch-1
nickserv Nov 28, 2020
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions src/__tests__/user-event.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import '@testing-library/jest-dom'
import {render} from '@testing-library/vue'
import {render, waitFor} from '@testing-library/vue'
import userEvent from '@testing-library/user-event'
import Form from './components/Form'
import Select from './components/Select'
Expand All @@ -24,49 +24,49 @@ test('User events in a form', async () => {
expect(submitButton).toBeDisabled()

const titleInput = getByLabelText(/title of the movie/i)
await userEvent.type(titleInput, fakeReview.title)
userEvent.type(titleInput, fakeReview.title)
expect(titleInput.value).toEqual(fakeReview.title)

const textArea = getByLabelText(/Your review/i)
await userEvent.type(textArea, 'The t-rex went insane!')
userEvent.type(textArea, 'The t-rex went insane!')
expect(textArea.value).toEqual('The t-rex went insane!')

await userEvent.clear(textArea)
userEvent.clear(textArea)
expect(textArea.value).toEqual('')
await userEvent.type(textArea, fakeReview.review)
userEvent.type(textArea, fakeReview.review)
expect(textArea.value).toEqual(fakeReview.review)

const initialSelectedRating = getByLabelText(/Awful/i)
const wonderfulRadioInput = getByLabelText(/Wonderful/i)
expect(initialSelectedRating).toBeChecked()
expect(wonderfulRadioInput).not.toBeChecked()

await userEvent.click(wonderfulRadioInput)
userEvent.click(wonderfulRadioInput)
expect(wonderfulRadioInput).toBeChecked()
expect(initialSelectedRating).not.toBeChecked()
await waitFor(() => expect(initialSelectedRating).not.toBeChecked())
Copy link
Member

@afontcu afontcu Nov 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Not 100% sure it's better for users to learn when/why they should use waitFor instead of just awaiting userEvent? (I honestly don't know 😂)
Copy link
Member Author

@nickserv nickserv Nov 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I assume you mean userEvent, not userSelect. I don't think we should encourage users to await click because it's not async, and awaiting it implies that the test code will wait for Vue to render (not necessarily true as far as I understand, because Vue renders asyncronously). Awaiting anything before this line (even await null) will pass the test. So while await userEvent.click(...) is simpler, I don't think it's correct, and await waitFor(...) better expresses that the assertion will pass some time after clicking. Alternatively we could use one of the other functions I suggested or make user event properly asyncronous.
Copy link
Member

Choose a reason for hiding this comment

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

👍
Copy link
Member

Choose a reason for hiding this comment

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

deal 😉
Copy link
Member Author

Choose a reason for hiding this comment

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

Want to merge this as is, or do you prefer any of the alternatives?
Copy link
Member

Choose a reason for hiding this comment

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

I guess it's fine as is 😃 I wonder if ppl will understand/know the difference between "test is failing because Vue hasn't updated the DOM yet" and "test is failing because my component is actually wrong", but that's something beyond our control (as long as Vue updates async)
Copy link
Member Author

Choose a reason for hiding this comment

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

If we make userEvent async for Vue users, this can be reverted, and "test is failing because Vue hasn't updated the DOM yet" should be more obvious when an await is missing (some linters can even warn you about not awaiting an async function).

const recommendInput = getByLabelText(/Would you recommend this movie?/i)
await userEvent.click(recommendInput)
userEvent.click(recommendInput)
expect(recommendInput).toBeChecked()

userEvent.tab()
expect(submitButton).toHaveFocus()
expect(submitButton).toBeEnabled()
await userEvent.type(submitButton, '{enter}')
userEvent.type(submitButton, '{enter}')
expect(emitted().submit[0][0]).toMatchObject(fakeReview)

expect(console.warn).not.toHaveBeenCalled()
})

test('selecting option with user events', async () => {
test('selecting option with user events', () => {
const {getByDisplayValue} = render(Select)
const select = getByDisplayValue('Tyrannosaurus')
expect(select.value).toBe('dino1')

await userEvent.selectOptions(select, 'dino2')
userEvent.selectOptions(select, 'dino2')
expect(select.value).toBe('dino2')

await userEvent.selectOptions(select, 'dino3')
userEvent.selectOptions(select, 'dino3')
expect(select.value).not.toBe('dino2')
expect(select.value).toBe('dino3')
})