Can't reference containers wrapped in a Provider or by connect with Enzyme

[mordrax]

I can't seem to reference anything wrapped in a <Provider> and a connect

// test
let component = shallow(<Provider store={store}><ContainerComponent /></Provider>);
component.find('#abc'); // returns null

let component = shallow(<Provider store={store}><div id="abc"></div></Provider>);
component.find('#abc'); // returns the div node

// ContainerComponent
const Component = ({...}) => (<div id="abc"></div>);
export default connect(..., ...)(Component);

I followed: #1481 to the examples where the tests were written with enzyme, however, the containers are never tested in those. So I don't know whether I should/can test smart containers?

@fshowalter Any ideas?

[gaearon]

Neither connect() nor Provider are part of this library. It is easier to discuss and track such issues when they are filed in the appropriate repository instead: https://github.com/reactjs/react-redux.

[markerikson]

I think that actually makes sense. Since you're shallow-rendering, only the Provider component will get rendered - your ContainerComponent will just be left in its object output form or whatever it is shallow rendering produces.

Two thoughts here:

First, note that the wrapper component generated by connect() actually does look for props.store before it looks for context.store, so if you do actually feel like you need to test a connected component, you should be able to render <ConnectedComponent store={myTestStore} /> without needing to worry about Provider or context or anything.

The second question is whether you really need to be worrying about actually testing the fully connected component. The argument I've seen made is that if you can test your "plain" component with specific props, and you can test your mapStateToProps implementation, you can safely assume that react-redux will put them together correctly, and don't need to actually test the connected version itself.

[mordrax]

@gaearon you're right, sorry. Didn't know whether to raise this in react or enzyme repo.

@markerikson Reason for testing smart component is for mapToDispatchProps where I wanted to make sure the right dispatcher was called by the wrapped component. Just passing the store into the ConnectedComponent means I won't be testing the state mapping or dispatch callback functions.

I will think about this more and raise an issue in the relevant repo if I need it. Thanks for the help.

[markerikson]

@Mordra : when you say "right dispatcher was called", do you actually mean "right action creator"?

You could do something like this (syntax is probably off, but you should get the idea):

let actionSpy = sinon.spy();

let wrapper = shallow(<PlainComponent someActionProp={actionSpy} />);
wrapper.find(".triggerActionButton").simulate("click");
expect(actionSpy.calledOnce).to.be.true;
expect(actionSpy.calledWith({type : ACTION_I_WAS_EXPECTING)).to.be.true;

In other words, render your plain component, pass in spies for the props that would have been actions returned from mapDispatch, and trigger whatever behavior the component needs to make it call those actions.

Also, per my earlier comment: you should be able to test your mapStateToProps and mapDispatchToProps separately, and feel safe that React-Redux will call them appropriately, without having to try to test the connected version itself to verify that's the case.

[mordrax]

@markerikson I can't do what you're suggesting because my PlainComponent does not know about actions or action creators. I don't know if this is the right way to do it, but if you look at:
https://github.com/mordra/cotwmtor/blob/master/client/charCreation/charCreation.jsx
you will see that my mapDispatchToProps contains all the logic:

const mapDispatchToProps = (dispatch) => {
  return {
    onCompleted      : (player) => {
      Meteor.call('newGame', player, function (data) {
        console.log('new game return: ' + data);
      });
      dispatch(routeActions.push('/game'));
      dispatch({type: "INIT_GAME", map: generateAreas(), buildings: generateBuildings(dispatch)});
    },
    onCancelled      : () => dispatch(routeActions.push('/')),
    onChangeName     : input => dispatch(actions.changeName(input)),
    onChangeGender   : gender => dispatch(actions.setGender(gender)),
    onSetDifficulty  : lvl => dispatch(actions.setDifficulty(lvl)),
    onChangeAttribute: (attr, val) => dispatch(actions.setAttribute(attr, val))
  }
};

so if I spied on the props passed into the PlainComponent, I wouldn't be able to test whether the right actions are called, but rather, just the parameters passed to the action creators are correct.
I'd then need to test the connect component separately.

[markerikson]

Ah. That helps me understand more.

So with the caveat that I actually have very little practical experience writing tests, a couple thoughts:

• What you actually have there really are action creators, they're just not defined separately because you're wanting access to dispatch. That's not very easily testable in and of itself. You would probably want to be able to test their behavior as well, and to do that you'd want to define them as their own functions outside of mapDispatch.
• Those all look like very good candidates for use with redux-thunk, which would certainly make it easier to define these functions on their own and allow them to access dispatch.
• Your PlainComponent does know about "actions", or at least callbacks in this case, in that you are passing those in, and somewhere in your component you're running this.props.onSetDifficulty("HARD") or something like that. When I suggested passing in spies for actions, this is the kind of thing I was suggesting replacing. So, if you passed in a spy for onSetDifficulty, you could verify that your component called it, and passed in an acceptable value for the difficulty level.

Ultimately, I think you should be able to make things a lot more testable by taking those functions defined in mapDispatch, and define them separately. Then you won't have to worry about having to have mapDispatch hooked up to test your component properly, and can focus on whether or not the component just called a given prop callback with the right arguments or something.

[markerikson]

Also, from the component's point of view: ultimately it's not actually worried about whether {action: CHANGE_NAME, name : "Fred"} got dispatched. All it knows is that it called this.props.onChangeName("Fred"), and that is what you should be trying to test - that it called a prop callback the right way.

[fshowalter]

@Mordra in your case, I would export mapDispatchToProps and test it in isolation. You can verify the property names are all correct and pass a spy for dispatch to test your action creators.

That said, I tend to eschew mapDispatchToProps in favor of mapActionCreatorsToProps which maps already-formed action creators that I can easily test in isolation. In that case I just test that all my props are defined to guard against import typos.

[gaearon]

Finally note that you can use normal (not shallow) rendering. You would need either jsdom or a real browser for that, but then you can render any level deep.

Didn't know whether to raise this in react or enzyme repo

Sorry, I did not mean React or Enzyme repos. I meant React Redux which is the library you are using.

[mordrax]

Thanks for the help guys, that's alot of info for me to go and digest. @markerikson @fshowalter I did some more looking around and will take your suggestion of exporting the mapState/Dispatch and testing them separately, it makes sense to test the callbacks that evoke the expected actions.

@gaearon Stateless component doesn't render without shallow and just skimming the issues, there seems to problems rendering stateful components that has nested stateless components so I've made a mental note to avoid that path for now.

[gaearon]

I'm not sure which issues you are referring to. You can use shallow rendering with functional components just fine. Shallow rendering works only one level deep though (no matter how component is defined). This is its main feature—it doesn't recurse so component tests stay independent. If you have specific issues with shallow rendering that you can reproduce please file bugs against the React repo. Thanks!

[markerikson]

@gaearon : so to clarify, if I do this:

let wrapper = shallow(<A><B /></A>);

will B get rendered, or not? Because I think that's what the original question was - trying to render a <Provider> around a connected component in order to test the connection.

[guatedude2]

Even though it's not directly related to Redux. I solved this by calling shallow() on the container again. It renders the inner component with the store state passed into it.

Example:

import React from 'react';
import {expect} from 'chai';
import {shallow} from 'enzyme';
import configureMockStore from 'redux-mock-store';
import thunk from 'redux-thunk';
import events from 'events';

const mockStore = configureMockStore([ thunk ]);
const storeStateMock = {
  myReducer:{
    someState: 'ABC'
  }
};

let store;
let component;
describe('tests for MyContainerComponent', () => {
  beforeEach(() => {
    store = mockStore(storeStateMock);
    component = shallow(<MyContainerComponent store={store} />).shallow();
  });

  it('renders container', () => {
    expect(component.find('div.somediv')).to.have.length.of(1);
  });
  ...
});

Hope this helps

[evenstensberg]

Should we make a more approachable way to do this through a custom module perhaps?

Edit: So we can declare connect as one method for Enzyme, such as component = shallowWithConnect()

[ConAntonakos]

@ev1stensberg This is a great idea. Have we decided whether this is the approach and/or has work started on this? If not, I'd love to contribute.

[granmoe]

For anyone who stumbles upon this issue in the future, here's the approach that works for me: just test the plain component by itself. Export both the component definition as a named export and the connected component (for use in your app) as the default export. Going back to the code from the original question:

// test
let component = shallow(<Provider store={store}><ContainerComponent /></Provider>);
component.find('#abc'); // returns null

let component = shallow(<Provider store={store}><div id="abc"></div></Provider>);
component.find('#abc'); // returns the div node

// ContainerComponent
export const Component = ({...}) => (<div id="abc"></div>); // I EXPORT THIS, TOO, JUST FOR TESTING
export default connect(..., ...)(Component);

Then just do

const component = shallow(<Component {...props} />)
// test component

And as others have pointed out above, if you also have coverage for the functions you pass into connect, you should have full coverage for your component as a whole.

[tugorez]

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

[dschinkel]

@markerikson what did you mean by:

you can test your mapStateToProps implementation

lets say you're exporting the connected container via an ES6 export. You wouldn't have access to the private mapStateToProps. Were you talking about some other way or can you be specific?

[dschinkel]

@mordrax

You can verify the property names are all correct

So if you expose your mapStateToProps and make it public, then say you render your ContainerComponent from your test including sending in say a fake store and sending in a state prop, then your mapStateToProps receives that state, maps it to a prop. But then how can you test it from that point? Connect will call mapStateToProps thus merging the props but where is the seam and which point is the seam in code during that process/flow where you can intercept the props being set on the presentational component? I guess double shallowing like @guatedude2 might be one way to check out the mapped props without any seams...

also on your shallow.shallow. So the connect() component passes what back, a wrapper right? And that wrapper what wraps the presentational component?

[dschinkel]

@granmoe can you explain this in more detail as to what you mean exactly:

if you also have coverage for the functions you pass into connect, you should have full coverage for your component as a whole.

[dschinkel]

I am also with @tugorez in terms of what I want to test and why.

@markerikson so nice example with the spy. It's one thing you can test, but you'd need more assers there to test the "unit of behavior". Just testing that a spy action was called from mapDispatchToProps doesn't really tell us the whole story about the container component. It doesn't assert on the expected outcome based on your container's handler logic.

It's not enough just to test that you've passed props or state, you want to test the behavior of the container component. That means testing two things:

1. did the container component wire up the props to the presentational component and if it did, is there a certain state I expect the presentational component to have once it's rendered based on that specific state passed by props to the presentational component. Who knows, maybe some dumb developer comes along and adds more code to mapStateToProps, you can't always trust that it'll map things properly, that's the point of testing container logic. While there isn't really any logic in mapStateToProps, who knows again some developer comes along and ads an if statement in there...well that's behavior which could mess things up.

2. does the dispatch handler logic (behavior) work in the container.

[markerikson]

@dschinkel :

The typical practice for defining Redux-connected components is to export default connect()(MyComponent), and also export MyComponent as a named export. Similarly, because mapState is just a function, you can also export const mapState = () => {}, and then import and test it separately.

As for testing the "container component" vs the "presentational component": the general thought here is that you probably don't need to be concerned about testing the container for the most part, where "container" === "the output of connect". React-Redux already has a full test suite for how that should behave, and there's no reason for you to be duplicating that effort. We know that it correctly handles subscribing to the store, calling mapState and mapDispatch, and passing props to the wrapped component.

What you as a consumer of the library are going to be interested in is how your component behaves. It shouldn't actually matter whether your own component is getting props from a connect wrapper, a test, or something else - it's just how that component behaves given a certain set of props.

(Also, FWIW, I realize you are absolutely the expert on testing and I am not, but it does seem like you're getting a wee bit paranoid about things :) If you're concerned about a mapState being accidentally broken, then write tests for it and move on.)

Now, if your wrapped component then renders other connected components inside of it, and especially if you're doing full rendering instead of shallow rendering, then it may very well be simpler to test things by doing const wrapper = mount(<Provider store={testStore}><MyConnectedComponent /></Provider>. But, overall, my take is that most of the time you should be able to test your mapState function and the "plain" component separately, and you really shouldn't be trying to test the connected version just to verify that the output of mapState is being passed to the plain component.

For reference, you may want to look at the miniature version of connect that Dan wrote a while back to illustrate what it does internally: https://gist.github.com/gaearon/1d19088790e70ac32ea636c025ba424e .

[dschinkel]

Yea, I mean if you export mapStateToProps and dispatchStateToProps, that would be better. I do not want to test that connect() (the collaborator in my test) works, definitely not. So that would be one clear way to do it, export those two methods:

example-container-spec.js

describe('testing mapPropsToState directly', () => {
    it.only('returns expected state', () => {
        const state = {firstName: 'Dave'};
        const output = mapStateToProps(state);

        expect(output.firstName).to.equal('Dave');
    });
});

Container

import React from 'react';
import { connect } from 'react-redux'
import Example from './Example';

export const mapStateToProps = (state) => ({
    firstName: state.firstName
})

export default connect(mapStateToProps)(Example);

Presentation

import React, { Component } from 'react';

export default class Example extends Component {
    render(){
        return (
            <div className='example'>
                {this.props.firstName}
            </div>
        )
    }
}

Then again, you could also do something like this with shallow rendering and still dive into the child component:

example-container-spec.js

it('persists firstName to presentation component', () => {
        var state = {firstName: "Dave"};
        const container = mount(<ExampleContainer store={fakeStore(state)}/>),
            example = container.find(Example);

        expect(example.length).to.equal(1);
        expect(example.text()).to.equal(state.firstName);
    });

Container

import React from 'react';
import { connect } from 'react-redux'
import Example from './Example';

const mapStateToProps = (state) => ({
    name: state.firstName
})

export default connect(mapStateToProps)(Example);

Presentation

import React, { Component } from 'react';

export default class Example extends Component {
    render(){
        return (
            <div className='example'>
                {this.props.firstName}
            </div>
        )
    }
}

yet another way to do this shallowly, is to do a double shallow. Shallow on the shallowed parent will shallow the child component that its wrapping, something like this:

it('get child component by double shallow()', () => {
        const container = shallow(<ExampleContainer store={fakeStore({})}/>),
            presentationalChildComponent = container.find(Example).shallow().find('.example');

        expect(presentationalChildComponent).to.have.length(1);
    });

So it's just another way, you're checking the outcome of your presentation component.

Either way would work...however like you said mapStateToProps being exported is probably best, as that's all I care about testing in terms of setting prop state.

I realize you are absolutely the expert on testing and I am not, but it does seem like you're getting a wee bit paranoid about things :)

no I'm not, testing right meaning having good tests that are not brittle and are providing value is important. First off nobody is an expert. Everyone is learning continually, some just don't want to admit it due to their egos. It's called Software Craftsmanship (aka Professionalism).

Testing only behavior and not testing collaborators (connect() itself) like you said is important. Figuring that out for the first time and testing well is important. Making sure you're not testing collaborators is important and that sometimes requires discussion amongst fellow devs.

You can have brittle tests, and it's important not to have brittle tests. Testing should be taken seriously, and that means continual checking if you're approaching it right. I'm simply trying to figure out my flow. I TDD, so how I start out, is important, and good tests is important with that. So nobody should feel like they are "paranoid" when they seek to understand different ways to test React components.

New React Test Repo Coming...
In fact I'm going to share a repo soon that shows various styles and combinations and approaches to testing react-redux. I think it'll help people because I am not the only ones thinking about this. Time and time again you see people asking the same questions around testing redux container and other components. The Redux docs nor do the react-redux docs do justice on that, they're lacking documentation in the testing area IMO. It only scratches the surface, and we need a nice repo that shows various styles about approaching React testing so I'll be putting that up soon.

If anyone would like to contribute to that repo, please get a hold of me so I can add you ass a collaborator on it. I'd love to have people add examples. I'd like to see examples with straight up React Test Utils + mocha, with Enzyme, Ava, Jest, tape, etc.

Conclusion:

I think both approaches we've discussed are fine. Directly test the method, or test it like I did above, dive into it. Sometimes you don't want to base your tests on a specific method because you can get fragile tests...hence why people got stung in the past with testing. So whether or not to test around one function, whether that tests the "unit" depends. Sometimes you don't want to make a method public, and it might be better to test the contract above that and keep some of it private. So it's always important to think about what your'e doing often when you write tests. Nothing wrong with that. Writing good tests is not easy.

That's exactly what TDD promotes, doing that often. So that you end up with leaner better less fragile code. TDD forces you to think about testing up front, not later. That's the difference and why people like me want to understand different flows and approaches because I have to when I TDD, it forces me to reassess design in little chunks often, and that means thinking about my testing approach often.

@markerikson thanks for your inputs I love talking testing. Good stuff man. I wasn't questioning your expertise, just that I hadn't actually tried testing mapStateToProps directly myself! You're not doing bad for being a non-tester ;). I'm simply new to redux, simple as that so I will have questions that don't just "touch the surface". We should discuss testing at a lower-level so that people understand what they're doing, how they can do it, and if they're benefitting from their approach. To do that you have to understand connect, react-redux, provider at a lower level...so that you know how to "only test behavior". I haven't seen many people exporting mapStateToProps which is interesting to me also...and it makes me wonder why not.

WeDoTDD.com
BTW for anyone interested, I started WeDoTDD.com last year (written in React btw). If a specific team (all devs on a particular team) TDD's, or your entire company (some companies do have devs where all TDD especially consulting companies), please reach out to me on slack.wedotdd.com

[dschinkel]

Update.

after playing more with testing connected components and reflecting on what's working great for me, I 100% agree now with @markerikson's statement here:

if you do actually feel like you need to test a connected component, you should be able to render without needing to worry about Provider or context or anything.

I haven't seen any need to introduce <Provider /> into my tests and it seems like you're testing a collaborator if you do that when the point isn't to test that <Provider /> is working. You should test that the behavior in your component works and it doesn't matter whether your container is getting the store via context. Just pass the store as a prop..connect just needs some way to get at the store, it doesn't care how (context or props)...get rid of provider altogether in your tests.

I also don't see a need to use enzyme's context option which is yet another way you could persist the store through React's context. Forget react's context altogether in your tests, it's totally unnecessary bloat and makes your tests more complex and much harder to maintain and read.

Test your mapStateToProps pure function directly by exporting it in your container file. I'm using a combo of testing mapStateToProps + also a couple tests that test my container shallowly, verifying that the presentational component at least is receiving the props I expect and then I stop there with the props testing. I've also decided not to do any double shallowing for my container component tests. TDD is giving me feedback that by trying to do so, the tests get too complicated and you're probably doing something wrong. That "wrong" is "don't double shallow stuff for your container component tests". Instead create a new suite of presentational component tests that test the output of your presentational components independently.

[markerikson]

Glad to hear you've got things worked out. (FWIW, my comments were largely in response to your questions about "seams" and "digging into props", which really seemed like going to unnecessary levels of detail for little to no benefit.)

Using a <Provider>/context is going to be necessary if you're doing a full render of a component that renders other connected components, as those connected children will be looking for the store in context.

If you've got suggestions for improving Redux's current docs on testing, by all means file a PR.

[dschinkel]

@markerikson good point on the mount. I'd only do a mount though if I needed to test the lifecycle of the component I'm testing. I wouldn't use it to deep dive into child components...that would no longer qualify as a unit test, and is basically integration testing and you'd be coupling that test to implementation details when you should test those child components on their own in isolation, not through a parent component.

Anyway good stuff, thanks!

[dschinkel]

ah ok got it! yea didn't think about that...

[danny-andrews]

Thanks for all the comments here, @markerikson and @dschinkel! They are very helpful. I've now decided to export my unconnected component and test it as I would any other, and also test my mapStateToProps and mapDispatchToProps separately. However, there is one case these tests don't catch, and that is when map(State/Dispatch)ToProps don't actually get passed into the call to connect. Example:

import React from 'react';
import { connect } from 'react-redux';

export const mapStateToProps = ({ name }) => ({ name });

export const Example = ({ name }) => <h1>{name}</h1>;

export default connect({ name } => ({ name: firstName }))(Example); // Whoops, didn't actually pass in `mapStateToProps`.

This probably won't ever happen in practice, but it could, especially since linters wouldn't report mapStateToProps as an unused variable since it's being exported.

[leizard]

@danny-andrews Could you be more specific about how to test your mapDispatchToProps function ?

[dschinkel]

@leizard

I don't test mapDispatchToProps or mapStateToProps directly anymore. I've changed my mind since this thread and advise not to do that.

Also testing them you'll run into @danny-andrews problem but the issue is more about good tests and trying to expose those two functions is not a good idea. It's over testing. You should test those two methods indirectly by testing the behavior or by testing the props through your shallowed container instead. I found there is no reason to try to expose those private methods and I now realized that it was also a bad testing practice to try to do so.

So I'm disagreeing with @markerikson, test through your connected component.

[dschinkel]

Finding the right API/contract to test at and not to over test, that's the key baby :).

[danny-andrews]

@dschinkel

You should test those two methods indirectly by testing the behavior...
So I'm disagreeing with @markerikson, test through your connected component.

Are you suggesting that you should forego exporting the unconnected component entirely and only test the connected component? I think that is "over testing." This ties your tests unnecessarily to the implementation details of the component it is testing. Because the functionality of the unconnected component (if it has any, that's a whole other can of worms) is completely unrelated to where it receives its props from. Maybe you want to turn that component into a purely presentational component in the future. If you test the connected component, you'll have to change all your tests to not inject a store anymore but pass the props in directly. If you tested the unconnected component, you don't have to do anything except blow away the connected-component-specific tests (more on that below).

I do agree that you should not directly test mapStateToProps/mapDispatchToProps, though. Exposing those private methods simply for testing purposes always felt like a code smell to me. In fact, I think this is the only time you should test the connected component. So, to summarize:

1. Export unconnected component and connected component
2. DO NOT export mapStateToPropsor mapDispatchToProps
3. Test all component logic via the unconnected component
4. Only test connected component when testing interaction with redux (data props are passed in from the proper place in the store/action props are assigned to the proper action creators, etc.).

The only extra overhead with doing number 4 is that you have to stub out the redux store to pass it into your connected component. This is pretty easy, though, as it's API is just three methods.

MapStateToProps Method

TestContainer.jsx:

import { connect } from 'react-redux';

export const TestContainer = ({ permissions }) => (permissions['view-message'] ? 'Hello!' : null);

export const mapStateToProps = ({ auth }) => ({ permissions: auth.userPermissions });

export default connect(mapStateToProps)(TestContainer); // Ewww, exposing private methods just for testing

TestContainer.spec.jsx:

import React from 'react';
import { shallow } from 'enzyme';

import TestContainer, { mapStateToProps } from './TestContainer';

it('maps auth.userPermissions to permissions', () => {
  const userPermissions = {};

  const { permissions: actual } = mapStateToProps({
    auth: { userPermissions },
  });

  expect(actual).toBe(userPermissions);
});

New Method

TestContainer.jsx:

import { connect } from 'react-redux';

export const TestContainer = ({ permissions }) => (permissions['view-message'] ? 'Hello!' : null);

const mapStateToProps = ({ auth }) => ({ permissions: auth.userPermissions });

export default connect(mapStateToProps)(TestContainer);

TestContainer.spec.jsx:

import React from 'react';
import { shallow } from 'enzyme';

import TestContainer, { TestComponent } from './TestContainer';

it('renders TestComponent with approriate props from store', () => {
  const userPermissions = {};
  // Stubbing out store. Would normally use a helper method. Did it inline for simplicity.
  const store = {
    getState: () => ({
      auth: { userPermissions },
    }),
    dispatch: () => {},
    subscribe: () => {},
  };
  const subject = shallow(<TestContainer store={store} />).find(TestComponent);

  const actual = subject.prop('permissions');
  expect(actual).toBe(userPermissions);
});

[dschinkel]

yea that's what I'm saying I don't export and test mapStateToProps, it's over testing. Test through the API. The API here is the Container.

[dschinkel]

@danny-andrews

Test all component logic via the unconnected component

can you site some examples?

[danny-andrews]

I mean the same thing as you when you said: "You should test those two methods indirectly by testing the behavior"

It might help to clarify terms: when I say "unconnected component," I mean the raw component which is wrapped in the call to connect and when I say "connected component" I mean the resulting component which is returned from the call to connect. "Connected component" and "container" I believe are synonymous. From my above:

// Unconnected component. Test all component logic by importing this guy.
export const TestComponent = ({ permissions }) => (permissions['view-message'] ? 'Hello!' : null);

const mapStateToProps = ({ auth }) => ({ permissions: auth.userPermissions });

// Connected component (container). Only test interaction with redux by importing this guy.
export default connect(mapStateToProps)(TestComponent);

Some people would prefer to split these entirely and have their "containers" just be calls to connect which wrap a purely presentation component. In that case, number 1 and 3 on the list goes away and the only tests you need to write are ones which verify interaction with redux (number 4).

[dschinkel]

I plan on writing a HUGE blog post on this. I'm busy at the moment in code but will reply later

[corinadev]

After reading this thread several times, I reached the conclusion that there are 3 approaches recommended:

1. Export mapDispatchToProps and mapStateToProps and test them separately
2. Shallow render the Container component and test the wiring is correct
3. Use selectors instead of mapStateToProps and action creators instead of mapDispatchToProps and test those separately; write tests using the action creators, reducers and selectors together to make sure the whole flow works.

In the end, I guess all options are valid, but have their own pros and cons. The "purest" is probably the 2nd one, but also involves most work. My personal preference would actually go towards option 3.
I wrote in more detail about this on my blog, where I also have some code samples.

[venikx]

@ucorina
In the current world of ES6 modules, each files is considered a module wrapped into it's own scope. Exporting is your API of the little module and it feels super wrong to export methods, just so you can test them. It's similar on how you would make private methods public, just to test them.

That's why I'm leaning towards @dschinkel approach and not export mapDispatch and mapState, because to me they are private implementation of the connected component.

However I'm not sure what to do with a component, which only wraps another component around a connect.

Did you ever write the blog post? @dschinkel would love to read it

[corinadev]

@AnaRobynn

Yeah, I agree with you that just exporting mapDispatch and mapState feels wrong, and I probably wouldn't use it myself, however it is a valid option, even though a less "pure" one. I left it there as a possible option also because Dan Abramov suggested this exact technique over here: reduxjs/react-redux#325 (comment).

To answer your queston of "what to do with a component, which only wraps another component around a connect", I would say just don't test it. There's no business logic related to your application and you don't want to test the connect implementation - that's already tested in the react-redux library.

If you still want to test anyway, to future proof your code, you can always follow the 2nd approach I describe here which is closer to what @dschinkel is describing.

[venikx]

@ucorina Exactly, I think option is the one I would go for as well! Thanks for explaining your answer more in depth. It's a nicely written blog post. Congratz!

[venikx]

@ucorina Sorry to bother again, but after sleeping on it for a while I'm not sure I fully agree with option 2 either. I'm not sure though. I'm also not sure about the benefit of testing mapStateToProps at all.

Do you think it's fine that "Option 2", indirectly also tests the action creators? And also indirectly testing selectors? Is that a good thing?
I'm not 100% conviced by that, but I'm also confused about all the (mis)information out there. There is so many different ideas to testing.

[dougbacelar]

1. Only use selectors inside mapStateToProps
   • mock them during container tests if needed
   • Test selectors separately.
2. Only use action creators inside mapDispatchToProps
   • mock them during container tests if needed.
   • Test action creators separately.
3. Shallow the connected component
   • assert it is rendered
   • test any extra logic that had to be added to mapStateToProps, mapDispatchToProps or mergeProps and isn't already tested elsewhere. (conditional dispatches, selectors that take args from ownProps, etc)
     • in this case you are only testing the mocks were called the correct amount of times and with the correct args.

Alternative:

• Mock store state to test container+selectors together
• Mock outside api calls to test container+actions together

[philihp]

@AnaRobynn I believe @ucorina's 2nd approach is the correct one.

A lot of what I hear suggests:

1. You should expose mapDispatchToProps, and test it directly
2. You don't need to test your call to connect() because connect() is already tested

Regarding 1, I don't think it's a good practice to expose internals in order to test. Your tests A: now assume internal structure, and B: should tests the thing how they're going to actually be used.

Regarding 2, you absolutely should test something that calls an external library. This is one of the weak points in any application for breaking. With any library it's still possible for a breaking change to be introduced in a minor/patch version bump, and you want your tests to fail fast.

Do you think it's fine that "Option 2", indirectly also tests the action creators? And also indirectly testing selectors? Is that a good thing?

Yes, redundant test coverage is a good thing.

[venikx]

@philihp @dougbacelar
I believe I grown a bit related to testing React applications. My current believes and thoughts about the subject:

1. Don't expose mapStateToProps and mapDispatchToProps if you don't need those mappings in multiple components. Exporting them just to test is an anti-pattern.
2. I currently see containers as collaborators (functions that delegate other functions and those other functions perform the real logic).
   What does that mean?

• Containers don't perform any logic
• The logic for selecting a state are for selector functions (which can be purely unit tested)
• The logic for actions are hidden in action creators (which might be depending if you are using redux-thunk or not are collaborators by themselves)
• The rendered view is another function (that potentially has another collaborator function or purely renders stuff

=> The test setup for it actually pretty straightforward when you use the proper testing libraries (I recommend testdouble.js).
=> Mock out your selector and actions via testdouble.js (using td.when())
=> Shallow render your container component, dive() once to get access to the view component's methods
=> Given the set of rules by td.when() assert if the component behaves correctly

No need to inject some fakeStore library, it's fine to stub out the store.

Example:

/** @format */

import React from 'react';
import { shallow } from 'enzyme';

function setup(props) {
  const HasOrganization = require('./HasOrganization').default;
  const defaultProps = {
    store: {
      subscribe: Function.prototype,
      getState: Function.prototype,
      dispatch: Function.prototype,
    },
  };
  const container = shallow(<HasOrganization {...defaultProps} {...props} />);
  return {
    container,
    wrapper: container.dive(),
  };
}

describe('<HasOrganization /> collaborations', () => {
  let getCurrentOrganizationId;
  beforeEach(() => {
    getCurrentOrganizationId = td.replace('../containers/App/userSelectors')
      .selectCurrentOrganizationId;
  });

  test('not render anything when the id is not valid', () => {
    const Dummy = <div />;
    td.when(getCurrentOrganizationId()).thenReturn(null);
    const { wrapper } = setup({ children: Dummy });

    expect(wrapper.find('div').length).toBe(0);
  });

  test('render something when the id is valid', () => {
    const Dummy = <div />;
    td.when(getCurrentOrganizationId()).thenReturn(1);
    const { wrapper } = setup({ children: Dummy });

    expect(wrapper.find('div').length).toBe(1);
  });
});

Thoughts?

[dougbacelar]

@AnaRobynn Strongly agree with points 1 and 2!

My containers are usually very straight forward:

// imports hidden

const mapStateToProps = (state, { userId }) => ({
  isLoading: isLoading(state),
  hasError: hasError(state),
  placeholders: getPlaceholders(userId)(state), // a list of some sort
  shouldDoStuff: shouldDoStuff(state),
});

const mapDispatchToProps = {
  asyncActionPlaceholder,
};

export const mergeProps = (stateProps, dispatchProps, ownProps) => {
  return {
    ...stateProps,
    ...dispatchProps,
    ...ownProps,
    onMount: () => {
      if (stateProps.shouldDoStuff) {
        dispatchProps.asyncActionPlaceholder(ownProps.userId);
      }
    },
  };
};

export default compose(
  connect(
    mapStateToProps,
    mapDispatchToProps,
    mergeProps
  ),
  callOnMount(props => props.onMount())
)(SampleComponent);

So I would

1. spy all selectors and action creator
2. test getPlaceholders spy was called with the correct userId
3. assert SampleComponent was rendered with the correct props
4. call the onMount prop from SampleComponent and verify it dispatches the mocked action when shouldDoStuff===true

• Things like conditional rendering of stuff depending on props I would test on a separate test for SampleComponent
• I also prefer to use redux-mock-store to mock the store like configureMockStore([thunk])() but think its fine to not do so...

TL;DR I basically only test selectors were called with correct arguments, mocked actions were dispatched successfully(if they depend on any logic) and that the child component is rendered with the correct props

@philihp

• I am very against testing external libraries, I prefer to mock everything and test things in isolation.
• I feel safe trusting that end-to-end tests will pick up any major problems caused by library upgrades.
• The reason I dont like to test containers+selectors+action creators all together is that you will have to retest the same selectors and action creators for every other container that reuses them. At that point you are just better off adding more test cases to the unit tests themselves.

[venikx]

@dougbacelar Can you give some sample test code to test the component you showed. You are using enzyme? Something else? Shallow? Mount?

[philihp]

@dougbacelar

I am very against testing external libraries, I prefer to mock everything and test things in isolation.

Mock when you need to, but if you can use the real thing, why not? Mocking is useful when certain calls are slow, or have side effects like network requests.

The reason I dont like to test containers+selectors+action creators all together is that you will have to retest the same selectors and action creators for every other container that reuses them. At that point you are just better off adding more test cases to the unit tests themselves.

Yes, test your selectors, action creators independently. Thoroughly test them with lots of expected inputs.

Yes, also test your containers. If that means they provide overlapping coverage on selectors and action creators, that's not a bad thing.

Here's what I mean...

import React from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { saveColor } from '../actions/save';
import { selectColors } from '../reducers/colors.js';
import ColorButtons from '../components/ColorButtons';
const ColorButtons = ({ colors, onClick }) => (
  <div>
    {colors.map(color => {
      <button type="button" key={color} onClick={onClick(color)}>{color}</button>
    })}
  </div>
);
ColorButtons.propTypes = {
  colors: PropTypes.arrayOf(PropTypes.string),
  onClick: PropTypes.func.isRequired,
};
const mapStateToProps = state => ({
  colors: selectColors(state.colors),
});
const mapDispatchToProps = dispatch => ({
  onClickColor: color => () => {
    dispatch(saveColor({ color }));
  },
});
export default connect(
  mapStateToProps,
  mapDispatchToProps
)(ColorButtons);

import React from 'react';
import { shallow } from 'enzyme';
import configureStore from 'redux-mock-store';
import ColorButtons from '../ColorButtons';
import { saveColor } from '../../actions/save';
import { selectColors } from '../../reducers/colors.js';
const buildStore = configureStore();
describe('<ColorButtons />', () => {
  let store;
  let wrapper;
  const initialState = { colors: ['red', 'blue'] };
  beforeEach(() => {
    store = buildStore(initialState);
    wrapper = shallow();
  });
  it('passes colors from state', () => {
    expect(wrapper.props().colors).toEqual(selectColors(initialState.colors));
  });
  it('can click yellow', () => {
    const color = 'yellow';
    wrapper.props().onClick(color)();
    expect(store.getActions()).toContainEqual(saveColor({ color }));
  });
});

Here, this isn't testing all the different params that selectColors and saveColor could get; it's testing that when <ColorButtons /> is created, it gets the properties we expect it to get. Absolutely test those in another test.

import { selectColors } from '../colors.js';
describe('selectColors', () => {
  it('returns the colors', state => {
    expect(selectColors(['red'])).toEqual(['red'])
  })
  it('returns an empty array if null', state => {
    expect(selectColors(null)).toEqual([])
  })
  it('returns a flattened array', state => {
    expect(selectColors(['red', ['blue', 'cyan']])).toEqual(['red','blue','cyan'])
  })
})

[venikx]

I am very against testing external libraries, I prefer to mock everything and test things in isolation.

Mock when you need to, but if you can use the real thing, why not? Mocking is useful when certain calls are slow, or have side effects like network requests.

Disagree, partly.
I never mock external libraries (don't mock what you don't own), but I do write a wrapper around for example axios. In my collaboration test I can just mock out all the functions and make sure everything is wired up correctly.

In my collaboration tests I always mock modules, functions, etc
These functions called by your collaborator can be easily unit tested.

Here's what I mean...

Also disagree.
You are implicitly testing your reducers and selectors here. If that's what you want to do fine, but I prefer to mock these functions out and just check if they are called correctly. The rest is handled by the reducers/selectors.

[philihp]

This thread is rehashing points made in https://martinfowler.com/articles/mocksArentStubs.html, Classical and Mockist Testing

[markerikson]

Agreed. Locking.