Skip to content
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.

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

Improve documentation on unit testing redux components #1462

Closed
c089 opened this issue Feb 29, 2016 · 29 comments
Closed

Improve documentation on unit testing redux components #1462

c089 opened this issue Feb 29, 2016 · 29 comments
Labels

Comments

@c089
Copy link

c089 commented Feb 29, 2016

I've noticed that the Writing Tests page explains how to unit test react components, but doesn't really go into details / show examples for containers wrapped by redux. I believe that unit testing vanilla JS components should rather be documented in the react documentation instead and redux docs should show users how to test components that are connected to redux.

My personal approach to this is to use shallow rendering and assert on the props of the wrapped component, so that I test the logic in mapStateToProps in one unit test and have independent unit tests for the wrapped components (if at all, as they often end up being static markup with no logic).

I'm happy to work on a PR for this but want to get some opinions before starting that.

@mxstbr
Copy link
Contributor

mxstbr commented Feb 29, 2016

👍 I've run into this and have to yet to find an elegant solution on how to test mapStateToProps and mapDispatchToProps. I'd love if there was an official guide for that, though I think we can let the explanation on how to test non-connected components in there too.

@gaearon gaearon added the docs label Feb 29, 2016
@gaearon
Copy link
Contributor

gaearon commented Feb 29, 2016

I don’t have definitive opinions on this. I think mapStateToProps is best defined outside components (like “selectors” in reducers folder of shopping-cart example). Then you can test that the selector returns expected state as you apply reducer after several actions without actually relying on the state shape itself.

However I’m open to other suggestions from people with experience of testing container components themselves.

@c089
Copy link
Author

c089 commented Feb 29, 2016

👍 I've thought about testing the selectors independent of their containers before as that massively simplifies the tests (plain JS in, plain JS out \o/) for those but was unsure because most documentation seems to keep them inside the containers and not assign them their own name (I like "selectors". fluxible uses "views" in their reducer store reference but that word is horribly overused).

@gaearon
Copy link
Contributor

gaearon commented Feb 29, 2016

Documentation doesn’t use them because selectors were not as popular at the time of writing the documentation.

@c089
Copy link
Author

c089 commented Feb 29, 2016

So should we rather update documentation to include that concept instead of documenting testing them integrated with react? I'd like that because redux is not really tied to react anyway...

@c089
Copy link
Author

c089 commented Feb 29, 2016

This tip from @gaearon hints in the same direction: move selectors out of the container and keep them close to the reducer.

Also for the record I was wrong about fluxible using "views" for that before, they use "getters" (but still follow the same pattern of keeping them together with the reducers, see docs).

@gaearon
Copy link
Contributor

gaearon commented Feb 29, 2016

Redux is not tied to React but practically speaking most Redux users are React users, React is used in the docs anyway, and testing isn’t that much different for different view libraries.

We should amend the docs to include different techniques but we shouldn’t remove useful parts just because they are React-specific. People are going to come to issues and ask anyway. Rather, we should improve those parts as well with more specific instructions, but at this point I can’t give recommendations—if you have experience, you tell me how to best test container components 😄

@fdecampredon
Copy link

One of the thing that I end doing a lot is exporting the different part of the component as named export, and the final component as default, this way I can easily test the different part separately :

export const maspStateToProps = (....) => {
 ...
} 

export const mapDispatchToProps = (...) => {
  ...
}

export class MyComponenent extends React.Component {
  ...
}

export default connect(mapStateToProps, mapDispatchToProps)(MyComponent);

I'm not really happy with this since I think that exporting private parts of a module just for the sake of testing is a bad habit.
But It often becomes too much a pain to test the whole logic just from the the final component.

@fshowalter
Copy link
Contributor

I came to the same solution as @fdecampredon.

It's pragmatic. You test mapStateToProps to verify your shaped state. If you're reaching into another bit of shared state and that node's name changes, your test will break and tell you. You test mapActionCreatorsToProps to verify you don't have any typos in your imports. 😄

You trust connect to work as advertised.

The extra exports don't bother me, as no one familiar with react-redux will mistake them for an exposed component API.

@c089
Copy link
Author

c089 commented Feb 29, 2016

I'm not really happy with this since I think that exporting private parts of a module just for the sake of testing is a bad habit.

👍

You trust connect to work as advertised.

👍

I think if we have selectors tested separately in the reducers, we can combine both of those good patterns: connect will receieve a minimal mapDispatchToProps that only calls the selectors that are already tested.

@dmitry-zaets
Copy link
Contributor

Currently Our team also ended up with exporting mapStateToProps and mapDispatchToProps only for test purposes. This looks not really handy and we've been thinking to separate them into new file which placed near smart component, something like:

  • MyComponent folder
    • index.js (contains simply export default connect ...)
    • map.js or mapping.js (maspStateToProps and mapDispatchToProps)
    • MyComponent.js (React component)

@dmitry-zaets
Copy link
Contributor

Also we've faced issue with testing async action creators, would be good to cover more complex cases in documentation as well.

@Phoenixmatrix
Copy link

I'm leaning torward one of two ways.

A) A 'perfect' world would be: given a mock store, and stubbing out the connected component, run assertions on the props of the stub component to match what I'd expect. That way mapStateToProp/mapDispatchToProp could stay private and it doesn't matter. They obviously also could be public if it makes sense and you could test them separately or stub them.

B) externalize/export mapStateToProp/mapDispatchToProp/selectors and test them separately. Assume that the call to connect is correct and don't test it.

B is drastically easier, but there's room for mistakes, bugs and untested glue. Plus you may be making public stuff you normally wouldn't purely for testing purpose. A could be made easier with some utilities (which isn't a bad thing. redux-mock-store is a utility that makes testing async creators much easier, and I don't see that as wrong).

I need to try out more real world scenarios to make up my mind, personally.

@weicheng113
Copy link

Hi Guys, how about the approach here #1458, which covers some area. This is a known practice used in OOP world.

Cheers, Cheng

@randycoulman
Copy link

I've written a blog post about how I test Redux applications at http://randycoulman.com/blog/2016/03/15/testing-redux-applications/. If there's anything in there that you feel would be useful in the Redux documentation, let me know and I'll be happy to pull it into a PR.

@onedesert
Copy link

onedesert commented Apr 8, 2016

Regarding the action testing we have faced this problem during our product development and finally we end up with a small package to easy test redux actions. It’s build on top of redux-mock-store extending your assertion library to make the process more fluent. Here and example using mjackson/expect assertion library:

it('should dispatch actionCreator1 actions', (done) => {
  expect(updateActionCreator()).withState(initialStore).toDispatchActions([
    actionCreator1(randomArg),
    { type: 'EXAMPLE', data: [] }
  ], done);
});

You could check it out in redux-actions-assertions.

Currently we have support for the following assertion libraries:

@gertsonderby
Copy link

gertsonderby commented Jul 27, 2016

I use Mocha, with the unexpected testing framework and unexpected-react.

Here's a simple container component:

import { connect } from 'react-redux';
import TabPage from '../presentation/TabPage';

function mapStateToProps(state) {
    return { name: state.pages.currentPage };
}

const ShownTab = connect(mapStateToProps)(TabPage);

export default ShownTab;

Here's the test for it:

describe('ShownTab', () => {
    let renderer, state, store, props;
    beforeEach(() => {
        renderer = TestUtils.createRenderer();
        state = {
            pages: {
                currentPage: 'test1',
                pages: ['test1', 'test2']
            }
        };
        store = {
            subscribe: sinon.spy().named('subscribe'),
            dispatch: sinon.spy().named('dispatch'),
            getState: sinon.spy(() => state).named('getState')
        };
        props = {
            test: 'value',
            pageDefs: {} // required for TabPage
        };
    });

    it('renders a TabPage with props and page name to show', () => {
        renderer.render(<ShownTab store={store} {...props}/>);
        return Promise.all([
            expect(renderer, 'to have rendered', <TabPage {...props} name='test1'/>),
            expect(store.dispatch, 'was not called'),
            expect(store.subscribe, 'was not called'),
            expect(store.getState, 'was called')
        ]);
    });
});

The store stub I used is probably a bit ungainly, but the overall strategy seems sound - test the actual output rather than any internals.

You could probably set up some custom assertions and other utility stuff to generalize this some, but since I just wrote this today (and am fairly new at React and Redux both), I haven't identified where best to do that.

@lytc
Copy link

lytc commented Nov 14, 2016

How about this approach?

// /store/store.js

import configureStore from './configureStore';
export default configureStore();
// /store/dispatcher.js

import store from './store';
import * as actions from './actions';

const dispatcher = {};
Object.keys(actions).forEach((actionName) => {
  dispatcher[actionName] = (...args) => store.dispatch(actions[actionName](...args));
});
export default dispatcher;
// /components/FileItem.js

import React, { Component } from 'react';
import dispatcher from '../store/dispatcher';

export default class FileItem extends Component {
  handleClick = () => {
    dispatcher.fileItemClick(this.props.id);
  }

  render() {
    return <div onClick={this.handleClick}></div>;
  }
}
// /components/FileItem.spec.js

import { shallow } from 'enzyme';
import td from 'testdouble';

import FileItem from './FileItem';
import dispatcher from '../store/dispatcher';

it('should dispatch fileItemClick on click', () => {
  const fileItemClick = td.replace(dispatcher, 'fileItemClick');
  const wrapper = shallow(<FileItem id={1} />);

  wrapper.simulate('click');
  td.verify(fileItemClick(1));
});

@tugorez
Copy link

tugorez commented Feb 14, 2017

Well, the first thing to note here is the philosophy of the <Containers />:

  1. Communicate the state from the store to a <Component />
  2. Modify the state through dispatching actions based on the <Component /> events.

If I'm ok then you only have to test 2 things:

  1. is your component receiveng the correct props generated from state (could be via selectors) ?
  2. is your component dispatching the correct actions ?

So this is my approach

import React from 'react';
import { shallow } from 'enzyme';
import { fromJS } from 'immutable';
import { createStore } from 'redux';
// this is the <Map /> container
import Map from '../index';
// this is an action handled by a reducer
import { mSetRegion } from '../reducer';
// this is an action handled by a saga
import { sNewChan } from '../sagas';
// this is the component wrapped by the <Map /> container
import MapComponent from '../../../components/Map';

const region = {
  latitude: 20.1449858,
  longitude: -16.8884463,
  latitudeDelta: 0.0222,
  longitudeDelta: 0.0321,
};
const otherRegion = {
  latitude: 21.1449858,
  longitude: -12.8884463,
  latitudeDelta: 1.0222,
  longitudeDelta: 2.0321,
};
const coordinate = {
  latitude: 31.788,
  longitude: -102.43,
};
const marker1 = {
  coordinate,
};
const markers = [marker1];
const mapState = fromJS({
  region,
  markers,
});
const initialState = {
  map: mapState,
};
// we are not testing the reducer, so it's ok to have a do-nothing reducer
const reducer = state => state;
// just a mock
const dispatch = jest.fn();
// this is how i mock the dispatch method
const store = {
  ...createStore(reducer, initialState),
  dispatch,
};
const wrapper = shallow(
  <Map store={store} />,
);
const component = wrapper.find(MapComponent);

describe('<Map />', () => {
  it('should render', () => {
    expect(wrapper).toBeTruthy();
  });

  it('should pass the region as prop', () => {
    expect(component.props().region).toEqual(region);
  });

  it('should pass the markers as prop', () => {
    expect(component.props().markers).toEqual(markers);
  });
  // a signal is an action wich will be handled by a saga
  it('should dispatch an newChan signal', () => {
    component.simulate('longPress', { nativeEvent: { coordinate } });
    expect(dispatch.mock.calls.length).toBe(1);
    expect(dispatch.mock.calls[0][0]).toEqual(sNewChan(coordinate));
  });
  // a message is an action wich will be handled by a reducer
  it('should dispatch a setRegion message', () => {
    component.simulate('regionChange', otherRegion);
    expect(dispatch.mock.calls.length).toBe(2);
    expect(dispatch.mock.calls[1][0]).toEqual(mSetRegion(otherRegion));
  });
});

@mealeyst
Copy link

mealeyst commented Apr 6, 2017

So I didn't write this, but I found this example by @paulsturgess https://gist.github.com/paulsturgess/e0f06a12821343d7854718c34394b60e#file-test-redux-container-component-js. I have been trying to replicate what he has done here by first testing our action creators using enzyme and sinon, mocha, and chai. I was able to see that dispatch was fired but when I tried to check the arguments that were passed, I was getting a type mismatch where my spied dispatch was being fired with undefined as it's argument.

@mealeyst
Copy link

mealeyst commented Apr 7, 2017

This might be an over simplification, but since we test our action creators in our redux modules and the rest of the container's job is to mapStateToProps, would it be sufficient to simply check that the keys of the objects that are being passed to our child component are correct?

  it('should render the StyleFileView with', () => {
    expect(instance.find(StyleFileView)).to.exist
  })
  it('should have action creators passed as props', () => {
    let actions = ['setKidSelected', 'updateKidProfile']
    expect(instance.find(StyleFileView).props()).to.contain.all.keys(actions)
  })

  it('should map state data to props', () => {
    let stateData = [
      'currentUser',
      'kidSelectedProfile',
      'kidProfiles',
      'kidProfileUpdating'
    ]
    expect(instance.find(StyleFileView).props()).to.contain.all.keys(stateData)
  })

@stevemao
Copy link
Contributor

stevemao commented May 5, 2017

Isn't this related to reduxjs/react-redux#325 (comment)?

@anikolaev
Copy link

I know there are many forms of connect() and even connectAdvanced() but usually you have typical code in container:

const AppContainer = connect(
  mapStateToProps,
  mapDispatchToProps
)(App)

So why not to just write connect() stub which would get mapStateToProps, mapDispatchToProps and App (component) allowing us to test each entity separately. First two are simple functions. If mapStateToProps uses selectors we just check that mapping is correct. For mapDispatchToProps we should check that component callbacks are correctly mapped to action creators through dispatch() calls. Component should be tested as any other React component.

@neekey
Copy link

neekey commented May 18, 2018

From all the points above, it seems to me that testing mapStateToProps and mapDispatchToProps is an easier way.

testing mapStateToProps is easy, the only preparation needs to be done is the pure state, but mapDispatchToProps is much more complex from my perspective:

  • actions usually involve data request to the server, which means we need to mock that
  • consider actions using the callback form: (dispatch, getState) => ..., yeah that's another case need to mock
  • I also sometimes nested dispatch in my callbacks, for example:
function mapDispatchToProps(dispatch, props) {
  const workspaceId = props.workspaceId;
  return {
    onRemoveMember: (userId, isCurrentUser) => {
      dispatch(removeWorkspaceMember(workspaceId, userId)).then(() => {
        if (isCurrentUser) {
          dispatch(goToWorkspaceList());
        }
      });
    },
  };
}

So it seems quite a lot work need to put in to test mapDispatchToProps. Anyone have any easier ways to do that?

@c089
Copy link
Author

c089 commented May 18, 2018

@neekey i haven’t worked with redux in a while, but I feel adding a side effect library may help. They usually work by dispatching some kind of plain object instead of a thunk function, which are much easier to assert on. Back then I wasn’t happy with the way redux-saga coupled tests to execution order in the code among others, but it was a step in the right direction for me.

@markerikson
Copy link
Contributor

@neekey : my personal advice is to not write an actual mapDispatch function, but rather to write separate action creators (thunks and otherwise), and use the "object shorthand" form of passing them to connect as an object. Then you can test those by themselves.

@malcolm-kee
Copy link

Adding my two cents based on recent experience:

  1. Don't treat each .js file as an unit, instead we treat combination of action creators, reducers, selectors, and thunk actions as an unit (as they are usually tightly coupled together). Therefore, usually we test those together in a single file called <feature>-redux.spec.js.

Example:

import { wait } from 'react-testing-library';
import configureStore from '../configureStore';

describe('todoStore', () => {
  test('it is initialized with default state', () => {
    const store = configureStore();
    selectTodo(store.getState()).toEqual([]);
  });

  test('add todo', () => {
  const store = configureStore();
   store.dispatch(addTodo('write test'));
    selectTodo(store.getState()).toEqual(['write test']);
  });

  test('add todo with server', async () => {
  // do some mocking, e.g. for axios, will mock at axios level
  const store = configureStore();
   store.dispatch(addTodoThunk('write test'));
    await wait(); // so async call will resolve
    selectTodo(store.getState()).toEqual(['write test']);
  }); 
});
  1. If you write selectors and thunk separately, then mapStates and mapDispatch are just few lines of straightforward code, which is usually fine to leave it not tested. But if you really want to test it, then render the <Provider> wrapping the component, do the similar mocking as the test case above, and test the whole connected component as a whole. (Note that this approach is heavily influenced by react-testing-library).

Example:

import { render, fireEvent } from 'react-testing-library';

describe('<ConnectedComponent />', () => {

  test('add todo', async () => {
  // do some mocking, e.g. for axios, will mock at axios level
  const store = configureStore();
  const { getByText, getByLabel } = render(<Provider store={store}><ConnectedComponent /></Provider>);
  fireEvent.change(getByLabel('New Todo'), { target: {value: 'add test'}}); // here we type in the new todo name
    fireEvent.click(getByText('Add Todo')); // here we clicking the button
    await wait(); // so async call will resolve
    expect(getByText('add test')).not.toBeNull();
  }); 
});

@hbarcelos
Copy link

I wrote this a few days ago: A better approach for testing your Redux code. Maybe there's something there that could be added to the docs.

TBH, I have just "discovered" this approach and I am using it in some real apps I have been working on, but I am not sure of the long-term impacts. At least in theory it seems to have a good maintainability.

Redux has great docs overall, I'd be glad to help rewriting this section to make it as awesome as the rest.

@joshribakoff
Copy link

Is this issue considered "done" now @markerikson now that #3708 merged? If not, what should be added to the docs to complete this task?

@timdorr timdorr closed this as completed Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests