test: remove await from userEvent usage #182
Conversation
|
The checkbox group test fails if called synchronously :( |
|
What should we be using, |
To keep it relevant to Testing Library users… I'd say |
| expect(wonderfulRadioInput).toBeChecked() | ||
| expect(initialSelectedRating).not.toBeChecked() | ||
| await waitFor(() => expect(initialSelectedRating).not.toBeChecked()) |
There was a problem hiding this comment.
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 😂)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Want to merge this as is, or do you prefer any of the alternatives?
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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).
Codecov Report
@@ Coverage Diff @@
## master #182 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 100 100
Branches 35 35
=========================================
Hits 100 100 Continue to review full report at Codecov.
|
|
🎉 This PR is included in version 5.6.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 6.6.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
userEventis entirely synchronous (except fortype'sdelayoption which isn't used), so we should use it withoutawaitand encourage users to do the same.