Data: Refactor 'withDispatch' tests to use RTL #44855
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Mamaduka
requested review from
jsnajdr and
tyxla
and removed request for
nerrad
Mamaduka
added
[Type] Enhancement
30 tasks
|
Size Change: +71 B (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
jsnajdr
approved these changes
Oct 11, 2022
| ).resolves.toEqual( { | ||
| type: 'increment', | ||
| count, | ||
| } ); |
Member
There was a problem hiding this comment.
This is not a bad thing to check: that the increment function prop injected to Component has the right return value. In case of a plain action, it's a promise of the action object. Would be nice to keep the check.
I would put it to the user-land code, to the button onClick handler:
async function onClick() {
const result = await props.increment();
expect( result ).toEqual( ... );
}|
|
||
| // Function value reference should not have changed in props update. | ||
| expect( testInstance.findByType( 'button' ).props.onClick ).toBe( | ||
| incrementBeforeSetProps |
Member
There was a problem hiding this comment.
This is also a useful property worth testing: that the increment prop is always an identical function, even though after a rerender with different props it behaves differently. One way to test this is wrapping the button in a helper component with React.memo and then checking there are no extra rerenders.
tyxla
reviewed
Oct 11, 2022
| expect( registry.select( 'counter' ).getCount() ).toBe( 3 ); | ||
|
|
||
| await user.click( screen.getByRole( 'button' ) ); | ||
| expect( registry.select( 'counter' ).getCount() ).toBe( 7 ); |
Member
There was a problem hiding this comment.
I find this odd. How come from 1 after the first click it becomes 7 after the third click? I'd expect this to stay 1, 2 and 3, respectively.
Member
There was a problem hiding this comment.
The test is testing whether the dispatch function can correctly read current state with registry.select. And the increment action increments by current_value + 1, equivalent to new_value = 2 * current_value + 1.
Member
There was a problem hiding this comment.
Sorry for being unclear - I understand how it works, but I think it might be easy to miss why it's working that way. Might be a good idea to elaborate, either in a comment or in the name of the test.
Member
Author
There was a problem hiding this comment.
I will add the inline comment.
jsnajdr
reviewed
Oct 11, 2022
| const Button = memo( ( { onClick } ) => { | ||
| buttonSpy(); | ||
| return <button onClick={ onClick } />; | ||
| } ); |
Member
There was a problem hiding this comment.
It's also possible, and elegant, to wrap the component function itself in a spy:
const ButtonSpy = jest.fn( ( { onClick } ) => <button onClick={ onClick } /> );
const Button = memo( ButtonSpy );The useSelect tests use this pattern to count renders.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
PR of #44780.
PR refactors
withDispatchHOC tests to use@testing-library/reactinstead ofreact-test-renderer.Why?
It is a part of recent efforts to use
@testing-library/reactas the project's primary testing library.How?
Updates the
rendermethod to use RTL, improves assertions and usesuserEventfor user actions.Testing Instructions