Am I testing connected components correclty?

[renanvalentin]

Hey folks!

Would like to know if I'm testing correclty my connected component, I've made some search, but still not clear for me and the way I'm doing right now doesn't sounds correclty, thats why I'm asking helping for the jedi council hahaha

The problem:
I want to know if action.fetchPurveyors is being called on fetchData that is passed on mapDispatchToProps:

const mapDispatchToProps = (dispatch) => {
  return {
    fetchData: () => {
      dispatch(actions.fetchPurveyors());
    }
  };
};

export const PurveyorsViewContainer = connect(
  mapStateToProps,
  mapDispatchToProps
)(PurveyorsView);

So, on my unit test I'm doing this:

describe('Map Dispatch To Props', () => {
    it('should call fetch data action', () => {
      const dispatchSpy = sinon.spy();

      // I'm replacing store's dispatch by an spy
      store.dispatch = dispatchSpy; 

      // I'm justing passing the store as props instead of wrapping it in a Provider
      const tree = sd.shallowRender(<PurveyorsViewContainer store={store}/>); 
      const vdom = tree.getRenderOutput();

      // Here I'm calling fetchData that was passed as props by mapDispatchToProps 
      vdom.props.fetchData();

      // And finally I'm checking if the dispatch was called with the action's parameters
      const expectedAction = actions.fetchPurveyors();
      const spyLastCall = dispatchSpy.args[0][0];
      expect(spyLastCall.types).to.be.eql(expectedAction.types);
      expect(spyLastCall.callAPI).to.be.ok;
    });
  });

Is there a better way of doing this? Am I doing an integration test instead of a simple unit test?

Thanks in advance!

[gaearon]

I would just test mapStateToProps and mapDispatchToProps themselves to be honest. Otherwise you seem to be testing what React Redux already tests—that it works 😄 .

You can find a discussion on this here: reduxjs/redux#1534.

Hope it helps!

[renanvalentin]

BOOM! 💥 👯

Thanks @gaearon!

[flushentitypacket]

@gaearon How do you test that the state tree looks like you expect?

e.g. I change the structure of the state tree by changing the reducer and make sure to change the tests there. Now all my tests pass, but the connected component is broken since it's not mapping the state correctly, anymore.

[atkinchris]

@flushentitypacket; write selectors and use them in your mapStateToProps. They'll allow you to get parts of state in your components, without the component needing to know the structure of state.

// Bad
const mapStateToProps = state => ({
  items: state.data.items
})

// Good
import { selectItems } from './store/selectors'

const mapStateToProps = state => ({
  items: selectItems(state),
})

Then, test your reducers, actions and selectors together - so your test starts with dispatching an action, and ends with expectations about the selector's output.

[v-Zer0]

@flushentitypacket @gaearon Question though... so what if you have mapDispatchToProps and you want to check that those methods ARE called in one of your component instance methods. So:

const mapDispatchToProps = (dispatch) => {
  return {
    fetchPaymentDetails: () => dispatch(fetchPaymentDetails()),
    updatePaymentDetails: (payload) => dispatch(updatePaymentDetails(payload)),
    clearServerErrors: () => dispatch(clearServerErrors())
  }
}

Then in a method within the component:

 submit(e) {
    this.props.clearServerErrors();
 }

So we can trigger the submit function but then we want to know that this.props.clearServerErrors was indeed called.

So far I have tried many ways to test this involving sinon spys and stubs but I haven't been able to verify that the props method actually gets called.

It seems to me that the method spies that I pass in when mounting get overwritten by the mapDispatchToProps

Any suggestions on how to check that prop methods inside of mapDispatchToProps get called?

[flushentitypacket]

@cclifford3 I typically export both the connected and unconnected classes for my components.

export class MyComponent {
  render() {
  }
}

export connect(...)(MyComponent)

Then I only test the unconnected component. So using say jest and enzyme we can do something like

it('calls myFnProp on submit', () => {
  const myFnProp = jest.fn()
  const component = mounted(<MyComponent myFnProp={myFnProp} />)
  component.simulate('submit')
  expect(myFnProp).toHaveBeenCalled()
})

^ I might've gotten some details off there, but I think that gives the general idea.

If you really want to test the connected component, I'd say that all you need to do is to test that the props are set correctly, but then you'd be testing react-redux!

[pokorson]

@gaearon and what about short syntax for mapDispatchToProps where you only pass object with action creators?
export default connect(null, { addPost })(PostForm);
I really like this syntax but in such case it is impossible to test mapDispatchToProps separately. Totally agree that in terms of unit tests it's redundant and in fact it tests react-redux responsibility. But doesn't such test overlaps a bit with e2e or integration tests? Can testing connected components, and action creators reduce number of integration tests?

[markerikson]

@pokorson : in that case, there's no need to test mapDispatchToProps, because there isn't even an actual mapDispatchToProps function to test. Just test your action creators separately if desired. You can safely assume that your component will receive a props.addPost function.

[pokorson]

@markerikson but my thought about such tests is to ensure that container play specific role in my UI. I can safely assume react-redux do it's job, (converts and passes action creators to components) and now I want to test if I connected given part of UI with some action.
I'm trying to compare such approach with testing with selenium. So instead of writing a script that traverse full rendered page, I wanted to create isolated tests for specific parts of UI.

I see it something like this:

1. Test dumb components for correct render.
2. Test reducers for correct state updates.
3. Test action creators to return correct action type and payload (or dispatch another actions).
4. And finally test if containers are returning correct state slice and actionCreators.

It feels like it should be enough to ensure that whole application work correctly? Does it make sense? Or selenium, nightwatch or something similiar is the way to go here?

[markerikson]

Right, but there's a difference between "testing that connected components work as part of the rest of the application", and "testing mapDispatch". If you use the object shorthand, there's no actual mapDispatch function to test.

Beyond that, I don't have a whole lot of advice to offer here :)

[doomsbuster]

Through the use of redux-mock-store and sinon i have been able to test the connected component without unwanted exports and side effects. Check it out here https://github.com/doomsbuster/express-react-boilerlpate/blob/master/ui/tests/containers/Header.spec.js

[danny-andrews-snap]

@flushentitypacket

I typically export both the connected and unconnected classes for my components.

Couldn't agree more! Testing component logic through connected component results in unnecessary test overhead.

If you really want to test the connected component, I'd say that all you need to do is to test that the props are set correctly, but then you'd be testing react-redux!

This isn't quite true because you need to verify the correct part of the state/correct action is mapped to the correct prop. e.g.

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

export default connect(({ auth }) => ({ permissions: auth.userPermissions }))(TestComponent);

If I don't test the connected component, then if I make a typo in my mapStateToProps param to export default connect(({ auth }) => ({ permissions: auth.userPermisions }))(TestComponent); it wouldn't be caught. It's a fairly mundane thing to test, but important none-the-less.

p.s. Here's how I would test the props are wired properly:

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

[flushentitypacket]

@danny-andrews-snap

If I don't test the connected component, then if I make a typo in my mapStateToProps param to export default connect(({ auth }) => ({ permissions: auth.userPermisions }))(TestComponent); it wouldn't be caught.

That's a fair nuance to point out. I don't typically find it helpful in the practical sense since it's so easy to catch and identify these sorts of things during "integration tests" (i.e. testing it in a browser, which realistically must be done for any frontend code changes).

As far as I can tell, there are only a couple places this could go wrong:

• There's a problem in mapStateToProps or mapDispatchToProps. You can export and test these methods directly super easily if you're concerned, but I'm not entirely sold on the value of that kind of testing. If your map methods are complex enough to warrant testing, maybe the logic needs to be abstracted out elsewhere (e.g. a selector) and that extracted logic to be tested directly.
• There's a problem in connect. Maybe you forgot to pass in your mapDispatchToProps, in which case I argue that the mental overhead of testing that it was passed in is much greater than simply checking your connect call (again, seems like an easily identified and solved issue during integration testing). Or maybe you have a typo, in which case I argue that your linter or a thrown syntax error again makes this trivial to identify and resolve.

The reason it's nice to unit test a component is that the component may be complex, and so unit tests can streamline the development and especially future development (i.e. code changes) process. I don't see the same value in testing connectedness, since that should always be extremely simple and easy to identify errors for, caught in the manual browser tests you're doing anyway.

[sirrodgepodge]

in an attempt to make the decorator syntax (which I'm a big fan of lol) more testable I made this:
https://www.npmjs.com/package/babel-plugin-undecorate

input:

@anyOldClassDecorator
export class AnyOldClass {
  @anyOldMethodDecorator
  method() {
    console.log('hello');  	
  }
}

output:

@anyOldClassDecorator
export class AnyOldClass {
  @anyOldMethodDecorator
  method() {
    console.log('hello');  	
  }
}

export class __undecorated__AnyOldClass {
  method() {
    console.log('hello');  	
  }
}