Skip to content

Data: Refactor 'withDispatch' tests to use RTL #44855

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
Mamaduka merged 5 commits into from
Oct 11, 2022
Merged

Data: Refactor 'withDispatch' tests to use RTL #44855

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1b0592a
Data: Refactor 'withDispatch' tests to use RTL
Mamaduka Oct 10, 2022
4bfd555
Add inline comment
Mamaduka Oct 11, 2022
2fb959a
Restore function value reference tests.
Mamaduka Oct 11, 2022
1859ac6
Restore action return values test
Mamaduka Oct 11, 2022
0947484
Wrap component in a spy
Mamaduka Oct 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
289 changes: 114 additions & 175 deletions packages/data/src/components/with-dispatch/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
/**
* External dependencies
*/
import TestRenderer, { act } from 'react-test-renderer';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

/**
* WordPress dependencies
*/
import { memo } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -10,224 +16,157 @@ import withDispatch from '../';
import { createRegistry } from '../../../registry';
import { RegistryProvider } from '../../registry-provider';

describe( 'withDispatch', () => {
let registry;
beforeEach( () => {
registry = createRegistry();
} );
jest.useRealTimers();

it( 'passes the relevant data to the component', () => {
const store = registry.registerStore( 'counter', {
reducer: ( state = 0, action ) => {
if ( action.type === 'increment' ) {
return state + action.count;
}
return state;
},
actions: {
increment: ( count = 1 ) => ( { type: 'increment', count } ),
},
describe( 'withDispatch', () => {
const storeOptions = {
reducer: ( state = 0, action ) => {
if ( action.type === 'inc' ) {
return state + action.count;
}
return state;
},
actions: {
increment: ( count = 1 ) => ( { type: 'inc', count } ),
},
selectors: {
getCount: ( state ) => state,
},
};

it( 'passes the relevant data to the component', async () => {
const user = userEvent.setup();
const buttonSpy = jest.fn();
const registry = createRegistry();
registry.registerStore( 'counter', storeOptions );

const Button = memo( ( { onClick } ) => {
buttonSpy();
return <button onClick={ onClick } />;
} );
Copy link
Member

@jsnajdr jsnajdr Oct 11, 2022

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

@Mamaduka Mamaduka Oct 11, 2022

Choose a reason for hiding this comment

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

Oh, that's a neat one. Thanks, @jsnajdr!


const Component = withDispatch( ( _dispatch, ownProps ) => {
const Component = withDispatch( ( dispatch, ownProps ) => {
const { count } = ownProps;

return {
increment: () => {
const actionReturnedFromDispatch = Promise.resolve(
_dispatch( 'counter' ).increment( count )
dispatch( 'counter' ).increment( count )
);
return expect(
actionReturnedFromDispatch
).resolves.toEqual( {
type: 'increment',
type: 'inc',
count,
} );
Copy link
Member

@jsnajdr jsnajdr Oct 11, 2022

Choose a reason for hiding this comment

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

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( ... );
}

Copy link
Member Author

@Mamaduka Mamaduka Oct 11, 2022

Choose a reason for hiding this comment

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

Restored in 1859ac6.

},
};
} )( ( props ) => <button onClick={ props.increment } /> );

let testRenderer;
act( () => {
testRenderer = TestRenderer.create(
<RegistryProvider value={ registry }>
<Component count={ 0 } />
</RegistryProvider>
);
} );
const testInstance = testRenderer.root;
} )( ( props ) => <Button onClick={ props.increment } /> );

const incrementBeforeSetProps =
testInstance.findByType( 'button' ).props.onClick;
const { rerender } = render(
<RegistryProvider value={ registry }>
<Component count={ 0 } />
</RegistryProvider>
);

// Verify that dispatch respects props at the time of being invoked by
// changing props after the initial mount.
act( () => {
testRenderer.update(
<RegistryProvider value={ registry }>
<Component count={ 2 } />
</RegistryProvider>
);
} );

// Function value reference should not have changed in props update.
expect( testInstance.findByType( 'button' ).props.onClick ).toBe(
incrementBeforeSetProps
Copy link
Member

@jsnajdr jsnajdr Oct 11, 2022

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

@Mamaduka Mamaduka Oct 11, 2022

Choose a reason for hiding this comment

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

Would something like 2fb959a work?

rerender(
<RegistryProvider value={ registry }>
<Component count={ 2 } />
</RegistryProvider>
);

act( () => {
incrementBeforeSetProps();
} );
// Function value reference should not have changed in props update.
// The spy method is only called during inital render.
expect( buttonSpy ).toBeCalledTimes( 1 );

expect( store.getState() ).toBe( 2 );
await user.click( screen.getByRole( 'button' ) );
expect( registry.select( 'counter' ).getCount() ).toBe( 2 );
} );

it( 'calls dispatch on the correct registry if updated', () => {
const reducer = ( state = null ) => state;
const noop = () => ( { type: '__INERT__' } );
const firstRegistryAction = jest.fn().mockImplementation( noop );
const secondRegistryAction = jest.fn().mockImplementation( noop );

const firstRegistry = registry;
firstRegistry.registerStore( 'demo', {
reducer,
actions: {
noop: firstRegistryAction,
},
} );
it( 'calls dispatch on the correct registry if updated', async () => {
const user = userEvent.setup();

const Component = withDispatch( ( _dispatch ) => {
const noopByReference = _dispatch( 'demo' ).noop;
const firstRegistry = createRegistry();
firstRegistry.registerStore( 'demo', storeOptions );

const secondRegistry = createRegistry();
secondRegistry.registerStore( 'demo', storeOptions );

const Component = withDispatch( ( dispatch ) => {
return {
noop() {
_dispatch( 'demo' ).noop();
noopByReference();
increment() {
dispatch( 'demo' ).increment();
},
};
} )( ( props ) => <button onClick={ props.noop } /> );

let testRenderer;
act( () => {
testRenderer = TestRenderer.create(
<RegistryProvider value={ firstRegistry }>
<Component />
</RegistryProvider>
);
} );
const testInstance = testRenderer.root;
} )( ( props ) => <button onClick={ props.increment } /> );

act( () => {
testInstance.findByType( 'button' ).props.onClick();
} );
expect( firstRegistryAction ).toHaveBeenCalledTimes( 2 );
expect( secondRegistryAction ).toHaveBeenCalledTimes( 0 );
const { rerender } = render(
<RegistryProvider value={ firstRegistry }>
<Component />
</RegistryProvider>
);

const secondRegistry = createRegistry();
secondRegistry.registerStore( 'demo', {
reducer,
actions: {
noop: secondRegistryAction,
},
} );
await user.click( screen.getByRole( 'button' ) );
expect( firstRegistry.select( 'demo' ).getCount() ).toBe( 1 );
expect( secondRegistry.select( 'demo' ).getCount() ).toBe( 0 );

act( () => {
testRenderer.update(
<RegistryProvider value={ secondRegistry }>
<Component />
</RegistryProvider>
);
} );
act( () => {
testInstance.findByType( 'button' ).props.onClick();
} );
expect( firstRegistryAction ).toHaveBeenCalledTimes( 2 );
expect( secondRegistryAction ).toHaveBeenCalledTimes( 2 );
rerender(
<RegistryProvider value={ secondRegistry }>
<Component />
</RegistryProvider>
);
await user.click( screen.getByRole( 'button' ) );
expect( firstRegistry.select( 'demo' ).getCount() ).toBe( 1 );
expect( secondRegistry.select( 'demo' ).getCount() ).toBe( 1 );
} );

it(
'always calls select with the latest state in the handler passed to ' +
'the component',
() => {
const store = registry.registerStore( 'counter', {
reducer: ( state = 0, action ) => {
if ( action.type === 'update' ) {
return action.count;
}
return state;
},
actions: {
update: ( count ) => ( { type: 'update', count } ),
},
selectors: {
getCount: ( state ) => state,
it( 'always calls select with the latest state in the handler passed to the component', async () => {
const user = userEvent.setup();
const registry = createRegistry();
registry.registerStore( 'counter', storeOptions );

const Component = withDispatch( ( dispatch, ownProps, { select } ) => {
return {
update: () => {
const currentCount = select( 'counter' ).getCount();
dispatch( 'counter' ).increment( currentCount + 1 );
},
} );

const Component = withDispatch(
( _dispatch, ownProps, { select: _select } ) => {
const outerCount = _select( 'counter' ).getCount();
return {
update: () => {
const innerCount = _select( 'counter' ).getCount();
expect( innerCount ).toBe( outerCount );
const actionReturnedFromDispatch = Promise.resolve(
_dispatch( 'counter' ).update( innerCount + 1 )
);
return expect(
actionReturnedFromDispatch
).resolves.toEqual( {
type: 'update',
count: innerCount + 1,
} );
},
};
}
)( ( props ) => <button onClick={ props.update } /> );

let testRenderer;
act( () => {
testRenderer = TestRenderer.create(
<RegistryProvider value={ registry }>
<Component />
</RegistryProvider>
);
} );

const counterUpdateHandler =
testRenderer.root.findByType( 'button' ).props.onClick;

act( () => {
counterUpdateHandler();
} );
expect( store.getState() ).toBe( 1 );

act( () => {
counterUpdateHandler();
} );
expect( store.getState() ).toBe( 2 );

act( () => {
counterUpdateHandler();
} );
expect( store.getState() ).toBe( 3 );
}
);
};
} )( ( props ) => <button onClick={ props.update } /> );

render(
<RegistryProvider value={ registry }>
<Component />
</RegistryProvider>
);

await user.click( screen.getByRole( 'button' ) );
// expectedValue = 2 * currentValue + 1.
expect( registry.select( 'counter' ).getCount() ).toBe( 1 );

await user.click( screen.getByRole( 'button' ) );
expect( registry.select( 'counter' ).getCount() ).toBe( 3 );

await user.click( screen.getByRole( 'button' ) );
expect( registry.select( 'counter' ).getCount() ).toBe( 7 );
Copy link
Member

@tyxla tyxla Oct 11, 2022

Choose a reason for hiding this comment

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

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.

Copy link
Member

@jsnajdr jsnajdr Oct 11, 2022

Choose a reason for hiding this comment

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

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.

Copy link
Member

@tyxla tyxla Oct 11, 2022

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

@Mamaduka Mamaduka Oct 11, 2022

Choose a reason for hiding this comment

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

I will add the inline comment.

} );

it( 'warns when mapDispatchToProps returns non-function property', () => {
const registry = createRegistry();
const Component = withDispatch( () => {
return {
count: 3,
};
} )( () => null );

act( () => {
TestRenderer.create(
<RegistryProvider value={ registry }>
<Component />
</RegistryProvider>
);
} );
render(
<RegistryProvider value={ registry }>
<Component />
</RegistryProvider>
);

expect( console ).toHaveWarnedWith(
'Property count returned from dispatchMap in useDispatchWithMap must be a function.'
);
Expand Down