Incremental UI Tests #3 #82
Conversation
aradmargalit
force-pushed
the
inc-ui-test-3
branch
from
037ad53 to
ddf4ab3
Compare
| expect(screen.getByText(/Delete this Draft/)).toBeDefined(); | ||
| }); | ||
|
|
||
| it('deletes a draft from Redux when the delete button is clicked', async () => { |
There was a problem hiding this comment.
this is where userEvent as a library is really nice. I think this test is really easy to understand, and mirrors how a user would interact with the table. What do you think?
There was a problem hiding this comment.
This is really nice, great demonstration of the value of that library! I imagine writing this test otherwise would be so complicated that I would have probably suggested we forget automating it (and just test manually)
| userEvent.click(screen.getByText(/Delete this Draft/)); | ||
|
|
||
| // Wait for antd to render the confirm modal, then click Yes | ||
| await waitFor(() => screen.getByText('Yes')); |
There was a problem hiding this comment.
In theory, you could combine lines 69 and 70, but according to Kent himself, it's an antipattern:
waitFor is intended for things that have a non-deterministic amount of time between the action you performed and the assertion passing. Because of this, the callback can be called (or checked for errors) a non-deterministic number of times and frequency (it's called both on an interval as well as when there are DOM mutations). So this means that your side-effect could run multiple times!
| visible={props.visible} | ||
| onCancel={props.onClose} | ||
| footer={props.footer} | ||
| destroyOnClose |
There was a problem hiding this comment.
this is the only line that changed. Keeping the modal in the DOM is wasteful and confuses the test runner
| expect(screen.getByText(/Delete this Draft/)).toBeDefined(); | ||
| }); | ||
|
|
||
| it('deletes a draft from Redux when the delete button is clicked', async () => { |
There was a problem hiding this comment.
This is really nice, great demonstration of the value of that library! I imagine writing this test otherwise would be so complicated that I would have probably suggested we forget automating it (and just test manually)
| visible={props.visible} | ||
| onCancel={props.onClose} | ||
| footer={props.footer} | ||
| destroyOnClose |
* App tests * Add scaffolding for useIsMounted * Ignore .js, .jsx, and other CRA files * Action creator tests * renderCommaSepList tests * removeMiddleAuthorsTest * shortenAuthors test * shortenTableString * getTagColor test * isDOI test * markdown test * shhhh is okay * self review feedback * DraftsRedux test * Woohoo! A Redux test * finish drafts tests Co-authored-by: Eshed Margalit <eshedmargalit@users.noreply.github.com>
Review #81 first! Once that's done, change this base branch to
react-ts(or whatever the mainline is at that point).🚨 Important! This fixes a legitimate bug with
goBackvs.back. Apparently there's an incompatibility betweenreact-router-domandhistory@5, so I downgraded to 4. Here's the literature on thatThis also fixes a silly bug where the
reduxRenderwouldn't properly nest Routes in a<Switch />