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

A better approach to writing redux tests? #2179

Closed
jhilden opened this issue Jan 4, 2017 · 13 comments
Closed

A better approach to writing redux tests? #2179

jhilden opened this issue Jan 4, 2017 · 13 comments

Comments

@jhilden
Copy link

jhilden commented Jan 4, 2017

I was wondering why you are recommending in the redux docs to test Action Creators and Reducers separately from each other instead of together and why you are recommending to use redux-mock-store instead of real redux store?

In my opinion Action Creators and Reducers form a strong unit or "module" together, with the Action Creators being the "public API", the reducer being the "private/internal implementation", and the state change on the store being the "output" of the module. My understanding of TDD/BDD is, that by default you should avoid testing private/internal logic (unless there are some strong/specific reasons for doing it). Instead the primary approach is to put only the public API of a module under test and make assertions on the output.

Documented approach: Unit testing creators/reducers separately

However, in the code examples from the redux documentation it is recommended to test the public API (the Action Creators) by only asserting that the correct internal methods were called with the correct arguments (expect(store.getActions()).toEqual(expectedActions)) and not by testing the intended output (= changes on the store). Because your are not doing that, you now need to write additional unit tests for your reducers. This not only requires more test code than necessary, but it also does not test the integration of Action Creators and Reducers. It is possible that both unit tests are still passing, but the combination of them does not produce the expected output anymore because of individual changes. Most importantly you need to rewrite a lot of test code when refactoring internals of the redux code, even if the behavior stays the same.

New approach: Testing redux from the outside

Based on these thoughts, I tried if it is possible to use a "real" redux store for the tests and test the redux logic "from the outside" without relying on any specific internals and by doing assertions on the changed store values. This is some pseudo-code on how such tests would look like:

// ---- actionCreators.js ----

function fetchTodosSuccess(body) {
  return {
    type: FETCH_TODO_SUCCESS,
    body
  }
}

export function fetchTodo() {
  return dispatch => {
    return fetch('http://example.com/todo')
      .then(res => res.json())
      .then(json => dispatch(fetchTodoSuccess(json.body)))
      .catch(ex => dispatch(fetchTodoFailure(ex)))
  }
}

// ---- reducers.js ----

export const initialState = [
  {
    text: 'Use Redux',
    completed: false,
    id: 0
  }
]

export default function todosReducer(state = initialState, action) {
  switch (action.type) {
    case FETCH_TODO_SUCCESS:
      return [
        {
          id: state.reduce((maxId, todo) => Math.max(todo.id, maxId), -1) + 1,
          completed: false,
          text: action.text
        },
        ...state
      ]

    default:
      return state
  }
}

// ---- test.js ----

import {createStore, applyMiddleware} from 'redux'
import thunk from 'redux-thunk'
import nock from 'nock'
import expect from 'expect' // You can use any testing library
import todosReducer, { initialState } from 'reducer'
import { fetchTodo } from 'actionCreators'

describe('fetchTodo()', () => {
  afterEach(() => {
    nock.cleanAll()
  })

  let store;
  beforeEach(() => {
    // create a new store instance for each test
    store = createStore(
      todosReducer,
      initialState,
      applyMiddleware(thunk)
    )
  })

  context('request succeeds', () => {
    it('should add the todo to the store', () => {
      nock('http://example.com/')
        .get('/todo')
        .reply(200, { body: { text: 'do something' }})

      return store.dispatch(actions.fetchTodo())
        .then(() => {
          expect(store.getState()).toEqual([
            {
              text: 'do something',
              completed: false,
              id: 1
            },
            {
              text: 'Use Redux',
              completed: false,
              id: 0
            }
          ])
        })
    })
  })
})

I think the advantages of this new approach are:

  • less test code
  • tested from an outside perspective (just like the code is used in your app)
  • internal logic can be refactored without a problem
  • integration of action creators and reducers is covered in the tests

Limitations

One limitation of not using redux-mock-store is, that with the exact approach above you are only doing assertions on the final state of the store. It is currently not yet possible to assert that there has been some intermediate state (e.g. a loading state) in between the action creator being dispatched and the final state being reached. However, I'm sure that there are some ways to test this with a real redux store as well.

I can imagine, that for certain much more complex use cases it could become cumbersome to do a lot of test setup to test all the different cases/branches of a single reducer. To easily handle that, and also to properly document such a complex reducer, it might make sense to resort to the old way of writing a separate unit test for the reducer. However, I think it is generally a good thing, that my recommended default approach encourages you to keep your action creators and reducers simple and only resort to other measures in special cases.

Feedback please

I hope my reasoning was comprehensible and I'm very much looking forward to hear what other people have to say about this subject. Maybe I'm overlooking some important things, but overall I'm pretty certain that my points are valid.

If people should agree with me, I would be happy to create a more comprehensive example and work on changing the redux docs accordingly.

@brigand
Copy link
Contributor

brigand commented Jan 5, 2017

I'd go a step further and create the full store, dispatch actions, and use selectors on the full store state in the tests. This allows you to also change the structure of the store/reducers but be sure that components accessing the state see the same thing. I've yet to try this.

@jhilden
Copy link
Author

jhilden commented Jan 5, 2017

@brigand thanks for your feedback. As far as I can tell, what you are describing should be exactly what I am also advocating. And it works well for me. Or am I missing any subtle differences?

@brigand
Copy link
Contributor

brigand commented Jan 5, 2017

@jhilden you're passing a single reducer instead of the combineReducers result, and asserting directly on the whole state instead of using selector functions (which may be plain functions or reselect). Perhaps that was just you simplifying the example.

@jhilden
Copy link
Author

jhilden commented Jan 5, 2017

@brigand Yes, this omission was just because of the simplified example. I would also definitely recommend to use all the combined reducers in the test (to make sure any interdependencies between reducers are also covered).
In the project where I tested the approach we are not using reselect, therefore it was not part of the example, but I also agree with you on that. One should be accessing the store in tests the same way one is accessing it in the app code.

@timdorr timdorr changed the title [Discussion, docs] A better approach to writing redux tests? A better approach to writing redux tests? Jan 5, 2017
@timdorr
Copy link
Member

timdorr commented Jan 5, 2017

One thing to watch out for are differences in how your store gets created in your app and your test suite. I think what might work best here is encouraging the use of a module dedicated to the creation of the store in both cases. That way you get a consistent store instance no matter what environment it's used in.

I also agree with the sentiment of dropping yet another dependency on users in the docs. You've already got a middleware, HTTP client, testing library, this "nock" thing, and a mock redux store in there. That can be pared down to something more simple and a better test that's looking for what happens, not how it happens.

@timdorr timdorr closed this as completed Jan 5, 2017
@timdorr timdorr reopened this Jan 5, 2017
@timdorr
Copy link
Member

timdorr commented Jan 5, 2017

Whoops, wrong button...

@jhilden
Copy link
Author

jhilden commented Jan 5, 2017

One thing to watch out for are differences in how your store gets created in your app and your test suite.

Yes @timdorr, that's another very good point that was not obvious from the simplified example. What we are doing in our app is that we export our composed reducers and middlewares from our store logic and use them in a storeFactory test helper for creating our test store instances. That way we make sure they are as close to our app as possible. The only thing that is different is the initial state passed into the store. In your tests you obviously want to prepare a certain initial state for most test cases.

@tf
Copy link

tf commented Jan 6, 2017

This absolutely matches my experience. Especially, as @brigand pointed out, in combination with selectors, this approach turns reducer/state shape into an implementation detail. I still think there can be a lot of value in writing more specific tests when things get complicated and it's one of Redux great merits that it lets us compose pieces that can be tested individually. But for verifying that everything integrates correctly this has been my approach of choice, especially when working without type checking (Flow/TypeScript etc).

Regarding testability of intermediate states, I think this is mostly a downside of asynchronous action creators. With Redux Saga these concerns can be handled independently. (off-topic: regarding your use of .then(...).catch(...) also see this tweet)

One related pattern that I have started to observe but haven't completely followed-up upon, is related to the concept of modules that you mention. I often find that my apps are composed of modules that each relate to a single domain concept or topic. Each of these modules exports action creators and selectors which form its public API. Furthermore they export a reducer, sagas and often some form of init function. We've been starting to export the last three inside a single object (that we refer to as "Redux module" in lack of a better name) and provide a createStore function that takes a list of these Redux modules and handles reducer combination and saga setup in a standard way.

I could imagine that a pattern like that would work especially well with this testing approach, as it allows creating a store that resembles the production store as much as possible, but does not have to use the full reducer function. Instead you can just create a store from a set of Redux modules and interact with them through their public API. This could easily be extended to include some form of simple dependency management, where one such module can provide a list of modules it depends on internally.

/cc @kioopi

@markerikson
Copy link
Contributor

Yeah, there's a bunch of experimentation going on around "pluggable Redux modules" and such. One interesting-looking example is https://github.com/brianneisler/duxtape (see duxtape issue #1 for discussion of intent and purpose ).

I've also got a number of other encapsulation/module-ish-related links to articles and libs here: https://github.com/markerikson/react-redux-links/blob/master/redux-architecture.md#encapsulation-and-reusability , https://github.com/markerikson/redux-ecosystem-links/blob/master/component-state.md .

@jhilden
Copy link
Author

jhilden commented Jan 9, 2017

Thanks for weighing in @tf

For our app we have been following the "ducks" pattern of collocating action creators and reducers that belong together to a common domain topic. It's not a full module solution, but it goes into the same direction, and it certainly drove us towards wanting a different approach for testing our little 🦆s.

Concerning the off-topic recommendation of not using .then(dispatch(...)), that code is taken right from the the redux testing docs. So one more reason to change those docs.

So, if I would create PR to change the docs into the direction of the suggested approach, is there a good chance that it would get merged? Or should anybody else be consulted first?

@markerikson
Copy link
Contributor

A couple thoughts.

First, yeah, it does seem that the .then(() => dispatch(SUCCESS)).catch(() => dispatch(ERROR)) pattern is flawed, because that will dispatch the error action if there was a problem during the success dispatch. What's really needed is the two-callback form of then(), so that we only dispatch the error action if there was a problem with the request itself. Seen this pointed out in a couple places recently, and it is something we should probably try to clean up.

Second, I'm not sure exactly what's being proposed here. It seems like you're talking about more of an integration-level testing than lower-level unit testing. Given that the "ducks" pattern is not specifically recommended by the Redux team, I'm not sure I agree with rewriting the docs to emphasize this approach.

That said, if someone wrote up a blog post that laid out this kind of approach in detail, we could definitely add a link to that post (and maybe a few others) to the "Testing" page.

@mica16
Copy link

mica16 commented May 10, 2017

@jhilden That's exactly what I do, avoiding testing the internal implementation.
Indeed, testing in isolation the reducers couple the tests to implementation details.
All websites recommend it, but sorry, I really dislike the approach.
A good TDD practicer tests a complete behaviour (public API), not method or non-atomic use-case.

Basically what I do is dispatching an action, and subscribing to the store to inspect the new resulting state.

Testing in isolation would distort the daily test report of the application.
Why ? Let's take this example:

_ You have 14 reducers.
_ You have 25 action creators of which 13 interact with a particular reducer.

If this reducer fails to achieve its job, you would get one failed unit test, its dedicated unit test, leading to this kind of feeling: "oh it's not a tragedy, one unit test that failed among 60, it's pretty good, we'll repair it; our application is good!".

This person would not notice at first glance that 13 use cases currently fail in production due to this specific reducer failure, and it might even be possible that the solution to repair is then far more tough than it would appear, since this solution will have to be satisfying for all those 13 use cases.

Testing from the outside (around the core logic, not internally) helps to trust the tests far better.

@markerikson It's not about an integration testing but still unit testing.
The following video well explains the idea and the advantage of a port-adapters architecture:
https://www.youtube.com/watch?v=HNjlJpuA5kQ
Integration testing is about testing "adapters".

With a port-adapters architecture (Hexagonal Architecture) actions creators (or Saga-triggering events) would represent the use cases of the app that I call, the "core logic".
Unit tests SHOULD and MUST be focused on the use cases, and thus should tackle the use cases API only.

Reducers are internal to the use case, they are implementation details since they are meant to be hidden from the client.

Client only dispatches some event to a saga, or rather calls an action creator directly.
Therefore, unit tests should only know about actions/events, store's state, and sagas if a middleware is set.
They should not be aware of any reducers or any other internal strategy.

TDD enables emergence and refactoring of reducers, while keeping tests unaltered.
It gives me a freedom for refactoring the internal.
The most simple example: I don't want to alter an existing unit test if I just renamed an action, or change some payloads in it. I'm just focused on the side effect => the update of the store.

I'm working like this for a long time, and it works very well in the Redux's world.

I even managed to involve a deepFreeze on the store's state, while being notified as soon as an internal reducer tried to mutate the previous state instead of returning a new one.
For that I needed redux-catch (a very simple middleware) coupled with the "done" attribute of a mocha test, in order to build my state's mutation listener and make the test (unfocused on any reducer so, but on the whole use case) aware of this forbidden attempt.

@tourman
Copy link

tourman commented Apr 13, 2020

Has someone come across the depicted approach of the testing whole store using the marble diagrams? It seems to me a convenient alternative way to the imperative paradigm. I should emphasize I'm about the coverage of no epic but the whole store. I've tried to find a realization but in vain. However the idea of declarative time-related expectation seems pretty attractive to me. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants