Making the library framework agnostic

[arnaudbenard]

I have noticed that there are multiple bugs related to how testing frameworks handle errors. I have built this library for mocha but it looks like we need to make it work with Jasmine and tape. I would love to get a conversation started on how we can make the library support multiple frameworks.

Do you have examples of framework agnostic testing libraries?
Or
Should we make forks for every testing framework?

Related issues

#12
#13
#16
#21

cc/ @marr @jdmunro @HowardLoTW @dara76

[erquhart]

To summarize my comment on #16: you shouldn't have to deal with supporting test frameworks at all.

What we really need is a store spy - it would simply collect actions as they came. This leaves developers to determine the optimal integration for a given test framework. In Jasmine, I'd expect to do something like this:

// mockStore.actions is an array of actions received by the store

expect(mockStore.actions).toContain({type: 'MY_ACTION', payload: undefined});

mockStore.reset();

This keeps your support surface nice and small, and when you get bug reports like "Doesnt Wrk with super awesome new test frmwrk HEELPZ!!1", you can be like ¯_(ツ)_/¯.

[arnaudbenard]

I like that idea a lot. We could have something like:

mockStore(getState, (err, actions) => {
   expect(actions).toContain({type: 'MY_ACTION', payload: undefined});
   done();
});

[erquhart]

My work with this lib is limited to a very specific use case, but it seems that the api shouldn't require the expect call to be passed into the mockStore. What are the drawbacks of the mock store simply providing an array of received calls to be consumed however a developer chooses?

[arnaudbenard]

We need to find a way to manage async behaviour as well. In the the previous example, I suggested returning the actions array in the callback (instead of mockStore.actions). I agree that the library should not have any expect calls.

I don't see any drawbacks yet, it will clean up the api for sure. :)

[marr]

Would love to get @gaearon's input here too. He suggested the use of this library here http://rackt.org/redux/docs/recipes/WritingTests.html#async-action-creators

[gaearon]

I agree it shouldn't be framework specific. As for the exact API you probably know better than me. It could be written as a store enhancer, I guess. But I haven't thought that through.

[arnaudbenard]

@gaearon I took a look at store enhancers https://github.com/rackt/redux/blob/master/docs/Glossary.md#store-enhancer , interesting concept. The library is small enough that I can play with a few implementations until I see which one fits the best. I have a big project that uses redux-mock-store in multiple scenario.

Thanks for the feedback! 👍

[gaearon]

Nice thing about enhancers is they're now a first class concept in Redux, and there is neat API sugar for applying them: reduxjs/redux#1294

[arnaudbenard]

I haven't followed the new features in redux for a while. How would store enhancers work for this library?

Braindump

Right now I thought that the library would look this way:

    const action = { type: 'ADD_ITEM' };
    const store = mockStore({});

    store.dispatch(action);

    store.done(dispatchedActions => {
       expect(dispatchedActions[0].type).to.be.equal(action.type)
    });

Having expected actions was super useful to know when the actions are done dispatching (when expectedActions.length === 0).

If I include the concept of expected actions again, I would need to use the expected actions in the mock store and write actions tests on top. It feels like a step back in developer UX from the previous version. Indeed, we only needed to included the expected actions.

const action = { type: 'ADD_ITEM' };
const store = mockStore({}, [action]); // expected actions here

store.dispatch(action);

store.done(dispatchedActions => {
  expect(dispatchedActions[0].type).to.be.equal(action.type) // more expected actions here 
});

I also like the suggestion from @erquhart, we could have get the actions directly the store. It could look like that:

// Sync
const store = mockStore({});

store.dispatch(action1);
store.dispatch(action2);
const dispatchedActions = store.getActions();
expect(dispatchedActions[0].type).to.be.equal(constants.ADD_TODO);

// Async
const store = mockStore({});

store.dispatch(asyncAction) // execute an async action (promise)
  .then(() => {
    const dispatchedActions = store.getActions();
    expect(dispatchedActions[0].type).to.be.equal(constants.ADD_TODO_ASYNC);
  });

What do you think?

[jdmunro]

Just to throw in my opinion I like the way this is shaping up!

[erquhart]

store.dispatch returning a promise for async actions is ideal. Love it.

[ccoffey]

@arnaudbenard I think it would be a really good exercise to test the Async Action Creators example here with the new syntax.

Can you provide any estimated release date for this framework agnostic redux-mock-store? I actually found this thread while trying to figure out how to use redux-mock-store with QUnit. It was starting to look like I had to roll my own version, if your planning to release soon then I won't have to. This would be really great!

[ccoffey]

@arnaudbenard one more thought.

Having expected actions was super useful to know when the actions are done dispatching (when expectedActions.length === 0).

I don't think this is a good idea, what happens if you expect two actions but 3 are actually dispatched? The two that you expected plus one that you didn't expect. Won't redux-mock-store hide this behaviour?

[erquhart]

@ccoffey based on @arnaudbenard's promise api example, why not use dispatchedActions.length?

[ccoffey]

@erquhart the promise api idea looks great and should work! My intension was to point out a failing in the current implementation and promote the new implementation.

[eXtreme]

What about situations when your action creator actually returns a promise? How to test that the promise is fulfilled/rejected AND test all actions were actually dispatched?

[ccoffey]

@eXtreme please take a look at this link where you can see @gaearon testing an async action creator fetchTodos, he tests that two actions are eventually dispatched FETCH_TODOS_REQUEST and FETCH_TODOS_SUCCESS.

Is this what you were looking for?

[eXtreme]

@ccoffey it's a example using redux-mock-store and you already know what's wrong with testing dispatched actions in that way, hence the discussion above. :)

I like the last proposal in #22 (comment) It looks like it would work great with both normal AC, and AC returing a Promise.

[arnaudbenard]

After reading all the comment, I think the last proposal is the most popular

// Sync
const store = mockStore({});

store.dispatch(action1);
store.dispatch(action2);
const dispatchedActions = store.getActions();
expect(dispatchedActions[0].type).to.be.equal(constants.ADD_TODO);

// Async
const store = mockStore({});

store.dispatch(asyncAction) // execute an async action (promise)
  .then(() => {
    const dispatchedActions = store.getActions();
    expect(dispatchedActions[0].type).to.be.equal(constants.ADD_TODO_ASYNC);
  });

/// Multiple async
const store = mockStore({});

Promise.all([store.dispatch(asyncAction1), store.dispatch(asyncAction2)])
.then(() => {
// do the tests
});

The library accumulates the actions in an array and then it's up to the developer to test them. Should we also implement a method to clear the array or are you creating a mockstore each time?

Once we settle on the last question, I think it will take me 2 weeks to ship it.

Thanks everyone for the feedback

[erquhart]

Being able to create a store in setup and then reuse with .clear() calls would be great - again, much like a standard spy implementation.

Looking forward to it!

[ccoffey]

Agreed, providing a .clear() is a good idea!

[arnaudbenard]

I made a beta version of the new lib, you can install it like this;
npm install redux-mock-store@beta

Checkout the readme and let me know if you have questions
https://github.com/arnaudbenard/redux-mock-store/blob/refactor/README.md

[eXtreme]

@arnaudbenard so far, working good! 👍

[marr]

@arnaudbenard Thanks for taking the time to put together the beta above. I think the readme is clear and what I expect when testing async action creators. There is one typo in that readme, namely the missing done in the async test it('should execute promise', () => { should be: it('should execute promise', (done) => {

I can look at testing that with node-tap and see how it behaves. That would let me close #21 and maybe I could submit a PR for adding a tap example.

[marr]

@arnaudbenard I threw together a quick repo illustrating a sample async action creator. I applied the thunk middleware to the mock store like the new beta README suggests, and things seem to be working well: https://github.com/marr/test-async-action-creators

[arnaudbenard]

@marr I will update the readme with done

There's an example here: https://github.com/arnaudbenard/redux-mock-store/blob/refactor/test/index.js#L54

Can you spot a difference?

EDIT: I think you didn't put done in the then