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

Testing is Done Right #518

Open
3LOK opened this Issue Sep 1, 2016 · 41 comments

Comments

Projects
None yet
@3LOK

3LOK commented Sep 1, 2016

Hi,

My comment is about: https://redux-saga.github.io/redux-saga/docs/advanced/Testing.html

I think the way you see testing sagas is wrong and puts emphasis on exact 'implementation', and not 'result'.

The way you do tests: Check the call for each 'yield', and muck a return value to that yield. In this case if I change the implementation of my saga (in such a way that doesn't affect the results), I need to change me tests. Any refactoring of code will necessarily mean changing the tests.

My view is that your tests check that you implemented your saga in a very very specific way and not that the saga is doing what it needs to be doing.

The way we write test for our sagas is:

  1. Set up a test store with a specific scenario. Test store includes a reducer that interacts with actions that are fired by the saga.
  2. Run the saga using the standard redux-saga framework.
  3. See the side effects (actions that were sent).

I think this way of testing puts the emphasis where it needs to be. If we refactor the saga, and it still does exactly what it needs to do, the tests won't need to change (making the tests helpful for refactoring).

WDYT?

@Andarist

This comment has been minimized.

Member

Andarist commented Sep 1, 2016

I kinda feel the same. In my tests I wouldnt like to focus on underlaying implementation, just like in the 'normal' code. For that reason I'm not even testing my sagas right now. Dont want to have my tests broken if I rearrange 2 lines which dont even change the overall behaviour

@slorber

This comment has been minimized.

Contributor

slorber commented Sep 1, 2016

It's like other tests: do you want to unit-test the details of your code, or do you want to do integration tests of the behavior of the saga and your store, without having to deal implementation details.

I've always felt doing blackbox testing is a superior idea, as testing implementation details means that changing implementation details will likely break tests.

For example, do you want to test your classes that are used to build a REST API, or do you want to test the API directly? Personnally I think it's better to test the API directly, and don't care that much about unit tests, except rare cases where it's worth it because the function to test is likely complex and not going to change (like date/math utilities...)

I guess the subject is also quite related to new debates we've seen on twitter that say it's not really worth it to test React component outputs because when changing React component it will always break the test. That's probably why Jest's snapshot testing feature is gaining interest of the community

If redux-saga gives you good testability for yielded effects, it does not mean you have to test all your code in this fashion.

@3LOK

This comment has been minimized.

3LOK commented Sep 1, 2016

I'll make sure to open-source our redux-saga testing infrastructure soon.

Basically what we wrote is a component that you construct with full initial "state", an optional list of reducers (for more complex tests), and the sagas you need (including the tested one). Then our tests look like:

const tester = new SagaIntegrationTester({state, reducers, sagas}).
tester.put(someAction());
return tester.waitForAction(types.FINAL.ACTION).then(action => {
   expect(action.params).to.deep.equal(...);
   expect(tester.state).to.deep.equal(finalState);
})
@ms88privat

This comment has been minimized.

ms88privat commented Sep 2, 2016

@3LOK sounds awesome, please keep us informed!

@rpmonteiro

This comment has been minimized.

rpmonteiro commented Sep 3, 2016

@3LOK Patiently awaiting to hear back from you. I'm not pleased with the current way I have to test my sagas... :(

@3LOK

This comment has been minimized.

3LOK commented Sep 7, 2016

Hi all,

Here we go: https://www.npmjs.com/package/redux-saga-tester

Let me know what you think.

@neurosnap

This comment has been minimized.

Contributor

neurosnap commented Sep 7, 2016

I agree with @slorber, redux saga is providing you a means to unit test a saga whereas you want to test how the saga integrates with your actions, reducers, etc. At that point you might as well go for a full end to end test because that's effectively the goal you are attempting to accomplish.

@yelouafi yelouafi added the enhancement label Sep 9, 2016

@3LOK

This comment has been minimized.

3LOK commented Sep 12, 2016

@neurosnap I disagree - 'testing that you wrote specific lines of codes' is not unit-test. (by the way, I think that's also what @slorber meant, @slorber correct me if I'm mistaken).

Here's an example: Lets say I have a function:

function foo(params) {
    var a = params.a;
    var b = params.b;
    return a + b;
}

Now I have excellent unit test for this function. One day, I decide to change the order of the two 'var' lines - you'll agree with me that the unit test should not break - the function still does exactly what you'd expect it to do, right?

But if we see this as a saga:

var a = yield select(aSelector);
var b = yield select(bSelector);
yield put(actions.Result(a+b));

If I were to unit test your way, and now change the order of the two vars, the unit test will fail although the unit still does exactly what it's supposed to be doing (!). That's a clear sign of a bad unit test. This means that the unit test is specific for my specific implementation (implementation = line of codes) - that's not a good unit test. In your case - you might as well just write the same code twice, and use 'diff' - it is effectively the goal you are attempting to accomplish.

For redux-saga, the store and the actions are the inputs and outputs of the saga. To unit-test the saga is to give it different inputs (different stores), and to see its outputs (different actions). It's not the end-to-end test at all (eg. I'm not giving it a real store, or real reducers - just the ones that simulate the test).

My type of unit test allow me to refactor the code and still know it does exactly what it's supposed to do, without needing to rewrite the tests.

@neurosnap

This comment has been minimized.

Contributor

neurosnap commented Sep 13, 2016

You make a good point, @3LOK the order of selectors shouldn't change the outcome of the test. It seems more accurate to say that the conventional approach to unit testing sagas is actually to unit test the saga effects, rather each iteration of the generator. Basically with your library, you prepare a mock state, mock reducers, and dispatching a series of actions to determine if the saga dispatches the correct actions, is that correct?

@3LOK

This comment has been minimized.

3LOK commented Sep 13, 2016

@neurosnap yup, exactly.

@yelouafi

This comment has been minimized.

Member

yelouafi commented Sep 16, 2016

@3LOK How do you handle call effects? are call effects executing the real functions?

@3LOK

This comment has been minimized.

3LOK commented Sep 16, 2016

@yelouafi call effects are executed (since our testing infrastructure uses redux-saga to execute the saga), however we never call the 'real functions' in our unit-testing. All our sagas are initialized with the libraries they use, we never directly 'require' (or 'import') the libraries from the saga (as a rule for TDD):

Here's an example of our Feedback saga:

    export default function* feedbackRootSage(libraries) {
        yield [
            fork(waitForFeedback.bind(null, libraries))
        ];
    }

    function* waitForFeedback(libraries) {
        yield* takeEvery(types.FEEDBACK, handleFeedback.bind(null, libraries));
    }

    function* handleFeedback(libraries, action) {
        const {WixRestaurantsApi} = libraries;
        const {payload} = action;

        try {
            yield call(WixRestaurantsApi.sendFeedback, {
                accessToken:payload.accessToken,
                feedback:payload.feedbackRequest
             });
             yield put(actions.feedbackSent());
        } catch (e) {
             yield put(actions.feedbackError(e));
        }
    }

As you can see, during testing we can simply initialize the saga with mock libraries (for example a mock WixRestaurantsApi) and simulate different results.

@jfairbank

This comment has been minimized.

Contributor

jfairbank commented Oct 14, 2016

I guess I'll be the voice of dissension :)

While I admit that unit testing sagas necessarily ties the tests to the implementation, I can now make guarantees on the effects my saga deals with. I'm not only interested in the actions it dispatches. What if I care which API calls it makes? What if I care what arguments the call descriptor passes to my API functions?

Of course I love dependency injection (i.e. passing in the libraries that my saga will use), but I want to know that it uses the libraries correctly. Sure, I could create spies/mocks and assert that they're called with certain arguments, but then why bother using sagas? I can use spies with thunks. I want to test with as few spies and mocks as possible.

Reusing this example:

    function* handleFeedback(libraries, action) {
        const {WixRestaurantsApi} = libraries;
        const {payload} = action;

        try {
            yield call(WixRestaurantsApi.sendFeedback, {
                accessToken:payload.accessToken,
                feedback:payload.feedbackRequest
             });
             yield put(actions.feedbackSent());
        } catch (e) {
             yield put(actions.feedbackError(e));
        }
    }

Maybe it's personal preference or being overly anxious, but I would like to know that the sendFeedback function is called with the correct arguments. I also write some sagas intentionally such that ordering IS important. So for me, I want to test that ordering.

(Aside: Even my react tests take a hybrid unit testing approach along with snapshots because I'm not convinced snapshots are enough. If my component uses certain props, then I'd like to see those represented in the rendered output. If I'm writing stateless components with functions, then passing in props should result in a certain type of output based on those props. How is that any different from a normal pure function? Should I snapshot all my normal functions then too and just hope the initial snapshot is producing the correct result?)

I agree that refactoring a saga can be slightly annoying for unit testing, which is why I created redux-saga-test-plan. It has helped tremendously for testing and refactoring sagas, so fixing a test isn't that big of a deal for me personally.

Maybe I'm reading too much into it, but if I think about sagas as something similar to IO monads (#505), then they are similar-ish to compositions of function calls. I'd more than likely test the individual composed functions, so why should a saga be any different for testing?

So far, I haven't been burnt by this testing strategy for the project I've been working on. I have found more value and produced fewer bugs by just unit testing react, redux, and redux saga on this project, so I'm not convinced that I need integration testing.

Realistically, I think testing will be a subjective practice no matter how much we analyze it. Ultimately the testing strategy has to align with the goals of the business, the size of the team, and the pace of development of the product. For some teams, taking a more wholistic, integration testing approach might be better. For us, we've found unit testing more valuable. So maybe both testing approaches need to represented in the documentation along with the pros and cons and what each approach focuses on achieving.

@axelson

This comment has been minimized.

axelson commented Oct 17, 2016

I think a good step for me would be an easy way to only test the effects that I care about. So I generally don't care about when and in what order my saga is calling select, but I definitely do care for put and call. take is a little situational but I usually care about it.

@AsaAyers

This comment has been minimized.

AsaAyers commented Oct 17, 2016

I don't know if I even intend to continue the project on my own, but if anyone is interested here is my experiment: https://gitlab.com/AsaAyers/redux-saga-tester. I did also publish it on npm if you'd like to try it out.

The core concept is that it wraps around a specific saga and intercepts all calls. It has a real store like @3LOK 's redux-saga-tester, but it also gives you the ability to intercept calls like you do by examining the generator output. If you're testing the output of a generator, I think it's nice to be able to write your test as a generator.

Toy Example

This test is also available in the repo

export function* addSaga() {
    const a = yield select(selectA)
    const b = yield select(selectB)
    const total = yield call(sum, a, b)
    yield put({
        type: "RESULT",
        payload: total
    })
}

test("add-saga", () => {
    const tester = SagaTester({
        initialState: {
            a: 2,
            b: 3,
        },
        reducer,
    })
    tester.testAgainst(addSaga, function* ({ getState }) {
        let actual
        // yield gives you the next effect from your saga that you want to test.
        // This didn't care about the selectors, they execute normally.
        actual = yield
        expect(actual).toEqual(call(sum, 2, 3))
        const callSumResult = 5
        // You need to yield a result back to the saga to continue and pick up
        // the next significant effect.
        actual = yield callSumResult
        // END is a special value automatically emitted when/if your saga ends
        expect(actual).toBe(END)
        // Instead of verifying the action was fired, now verify the state.
        actual = selectResult(getState())
        expect(actual).toBe(callSumResult)
    })
})

The main problem I ran into was figuring out what to do with fork and call. Should I try to intercept fork / call effects, figure out if it's calling a generator and then also intercept that generator's calls?

@3LOK

This comment has been minimized.

3LOK commented Oct 18, 2016

@jfairbank @axelson @AsaAyers

For me, tests are relevant in the long run if I don't need to change them as long as I don't completely change the logic of the code. As long as the code should perform the same logic, as long as I fix bugs, refactor, and add new logic - the existing tests shouldn't change (just add new tests). Why? Because changing tests increases drastically the chances of introducing bugs in the tests which increases drastically the chances of introducing bugs in the code, and that beats the entire purpose for which I am writing tests (to make sure my code works the same in the long run).

Testing as described in the documentation simply doesn't follow that rule. Basically any change I do in the original code requires changing tests (since they are so binded together), and I think that's simply wrong in a continuous changing live code.

In the very very very rare cases where ordering of yields is important to you, that can be tested as well using our redux-saga-tester.

In regards to testing call effects, I do think our solution works best.

Here's an example of a small test (at the bottom you can see the partial saga I've tested - please disregard me not using selectors. I can share the entire saga and test if requested). See the test of api.sendDeleteRequest for an example of a call effect test (including testing the correct parameters were called):

The Test

describe('DeleteSaga', () => {

    let sagaTester = null;
    let apiMock = null;

    beforeEach(() => {

        apiMock = {
            sendDeleteRequest: sinon.promise()
        }

        const initialState = {
            session:{
                screen:'list'
            },
            list: {
                data:[
                    'mydata1',
                    'mydata2',
                    'mydata3'
                ]
            }
        };

        sagaTester = new SagaTester({
            initialState,
        });

        sagaTester.start(DeleteSaga.bind(null, {api: apiMock}));
    });

    it('should return delete list index and return to main screen when users selects "yes" in the yes/no screen', () => {
        // User pressed delete
        sagaTester.dispatch(DeleteActions.actions.request({itemIdx:1}));

        // screen should be 'yes/no'
        expect(sagaTester.getLastActionCalled()).to.deep.equal([SessionActions.actions.screenAreYouSure()]); 

        // user presses 'yes'
        sagaTester.dispatch(DeleteActions.actions.areYouSureYes());

        // screen should be thinking
        expect(sagaTester.getLastActionCalled()).to.deep.equal([SessionActions.actions.screenThinking()]); 

        // server returns success
        expect(apiMock.sendDeleteRequest.args[0][0]).to.deep.equal({itemIdx:1});
        apiMock.sendDeleteRequest.resolve();

        // list should be updated, and screen should go back to the main screen
        expect(sagaTester.getLastActionCalled(2)).to.deep.equal([
            ListActions.actions.update({data:['mydata1', 'mydata3']}),
            SessionActions.actions.screenList()
        ]);
    });
});

The Saga

function* onDelete(clients, action) {

    const {itemIdx} = action.payload;

    yield put(SessionActions.actions.screenAreYouSure());

    const answer = yield take([
        DeleteActions.types.ARE_YOU_SURE_NO, 
        DeleteActions.types.ARE_YOU_SURE_YES
    ]);

    if (answer.type === DeleteActions.types.ARE_YOU_SURE_NO) {
        yield put(SessionActions.actions.screenList());
        return;
    }

    yield put(SessionActions.actions.screenThinking());

    try {
        yield call(clients.api.sendDeleteRequest, {itemIdx});
    } catch(e) {
        yield put(SessionActions.actions.screenError({error:e.toString()}));
        return;
    }

    const list = _.cloneDeep(yield select(state => state.list.data));
    list.splice(itemIdx, 1);

    yield put(ListActions.actions.update({data:list}));
    yield put(SessionActions.actions.screenList());
}
@yelouafi

This comment has been minimized.

Member

yelouafi commented Oct 18, 2016

@3LOK Isn't the test above also coupled (in a different manner) to the implementation. I don't mean it's a bad thing. But that in the Saga example the ordering is still important except for the last 2 put effects.

If we change the Saga impl. and yield an array instead of the 2 last consecutive effects where the ordering doesn't matter.

function* onDelete(clients, action) {

    // ...
    yield ([
      put(ListActions.actions.update({data:list}));
      put(SessionActions.actions.screenList());
    ])
}

wouldn't the corresponding unit test be no so much different from the mock based version?

@3LOK

This comment has been minimized.

3LOK commented Oct 18, 2016

@yelouafi Indeed, I give this specifically as an example foe a call effect testing. Indeed since in this example every line is an output effect, order (flow) matters, and is tested. (and thank you for the improvement suggestion).

But also in this example, if you note, I can change the position of my select effect and the logic that comes with it, without needing to change my test at all. (i don't see a reason to do so, but I can :))

We have many 'logic sagas' (compared to this 'UX flow saga') that interact mainly interact with third party services and with the store, without importance to the order of the services (it gets X from Service1, Y from Service2, Z from the Store, does some calculation, and puts an action with f(X,Y,Z)). In these saga tests, the theories I've discussed come strongly into play and are very easily tested with the framework.

@jimbol

This comment has been minimized.

jimbol commented Oct 18, 2016

@axelson, I agree that put and call are the main items that should be tested. They are the items which have an effect on the application.

take should be tested as well seeing as how it is what triggers the saga. We want to test what triggers the saga.

select, then, is an input. i.e. Given select(myData) yields { foo: 'bar' }, I expect call to myApi with some payload.

@jimbol

This comment has been minimized.

jimbol commented Oct 18, 2016

What I would like to see a sort of saga spy, which gives us the ability to write a test like this.

let sagaSpy;
beforeEach(() => {
  sagaSpy = spyOnSaga({
    saga: mySaga,
    selectors: {
      myData: () => myDataStub,
      token: () => stubToken,
    }
  });
});

it('is triggered by MY_ACTION', () => {
  sagaSpy.dispatchAction(myAction());
  expect(sagaSpy.called).to.be.true();
});

context('when MY_ACTION is dispatched', () => {
  context('when myData is clean', () => {
    beforeEach(() => {
      sagaSpy = spyOnSaga({
        saga: mySaga,
        selectors: {
          myData: () => myCleanDataStub,
          token: () => stubToken,
        }
      });

      sagaSpy.dispatchAction(myAction());
    });

    it('dispatches UPDATE action', () => {
      const updateAction = put(createUpdateAction({ foo: 'bar'}));
      expect(sagaSpy.getCall(0).puts(updateAction)).to.be.true();
    });
  });

  context('when myData is dirty', () => {
    beforeEach(() => {
      sagaSpy = spyOnSaga({
        saga: mySaga,
        selectors: {
          myData: () => myDirtyDataStub,
          token: () => stubToken,
        }
      });

      sagaSpy.dispatchAction(myAction());
    });

    it('dispatches REFRESH action', () => {
      const refreshAction = put(createRefreshAction({ foo: 'bar'}));
      expect(sagaSpy.getCall(0).puts(refreshAction)).to.be.true();
    });

    it('calls refresh api', () => {
      const refreshAction = call(doRefresh, { token, foo: 'bar'});
      expect(sagaSpy.getCall(0).calls(refreshAction)).to.be.true();
    });
  });
});
@jimbol

This comment has been minimized.

jimbol commented Nov 8, 2016

Spent a little more time on this approach with the goal of having this api.

const spy = effectSpy(generator)
  .next('init')
  .next('getToken', token) // name and pass in yield values
  .next('getUpdatedFooBars', fooBars)
  .next('putFooBars');

// in beforeEach
mySpyRun = spy.run({
  getToken: 'invalid-token' // accepts overrides
});

// in `it` statement
expect(mySpyRun.getToken.value).to.deep.equal(select(getToken))
expect(mySpyRun.putFooBars.value)
  .to.deep.equal(put(fooBarsAction, fooBars))

Feel free to use, the implementation is simple, I could turn this into a module if there's interest.

https://gist.github.com/jimbol/fe660007487e57a6e172ba26c3ada23b

@jfairbank

This comment has been minimized.

Contributor

jfairbank commented Nov 21, 2016

Because there is a lot of interest in integration-type testing — and I do recognize some of the benefits too — I've started work on a new function in redux-saga-test-plan for allowing a more holistic testing approach. I'd appreciate ideas and comments over on the issue I'm using to track the feature: jfairbank/redux-saga-test-plan#37.

EDIT: adding example code sample

Minimal example:

// ES2015
import { expectSaga } from 'redux-saga-test-plan';

function identity(value) {
  return value;
}

function* mainSaga(x, y) {
  const action = yield take('HELLO');

  yield put({ type: 'ADD', payload: x + y });
  yield call(identity, action);
}

const saga = expectSaga(mainSaga, 40, 2);

saga
  // assert that the saga will eventually yield `put`
  // with the expected action
  .put({ type: 'ADD', payload: 42 })

  // start Redux Saga up with the saga
  .start()

  // dispatch any actions the saga will `take`
  .dispatch({ type: 'HELLO' })

  // stop the saga
  .stop();
@eloytoro

This comment has been minimized.

Contributor

eloytoro commented Dec 28, 2016

The main problem with some of these solutions is that when running the sagas using the middleware there's no way to prevent call effects to actually execute external logic. If we somehow had a way to mock the evaluation of these effects in the sagaMiddleware I think testing would be really simple

I don't think an external lib would be necessary. Right now redux-saga exposes a very useful runSaga function which allows you to monitor, but not alter, saga execution. If there were a middleware option for it that would allow us to return the resulting value of each effect (when needed) it should be possible

Example

function* mySaga() {
  yield take(ACTION_TYPE);
  const result = yield call(serverRequest);
  return `The server responded with ${result}`;
}

runSaga(mySaga(), {
  ...,
  middleware: (effect) => {
    // ideally check that the effect being yielded matches your criteria
    if (isEqual(effect, call(serverRequest)) {
      // when something is not an effect redux saga just returns whats yielded
      return 'mocked value';
    }
    // return original effect
    return effect;
  }
});
@jfairbank

This comment has been minimized.

Contributor

jfairbank commented Jan 10, 2017

redux-saga-test-plan now supports an integration, BDD-style form of testing via the new expectSaga function. As others have pointed out, though, a middleware feature is very useful and something I plan to add next to help with mocking certain effects like call.

I'd appreciate feedback and any issues people can find with expectSaga. Since it's newer than the unit-testing testSaga function, there might be some bugs I haven't encountered yet. I also welcome more use cases and feature ideas at the repo.

import { call, put, take } from 'redux-saga/effects';
import { expectSaga } from 'redux-saga-test-plan';

function identity(value) {
  return value;
}

function* mainSaga(x, y) {
  const action = yield take('HELLO');

  yield put({ type: 'ADD', payload: x + y });
  yield call(identity, action);
}

it('works!', () => {
  return expectSaga(mainSaga, 40, 2)
    // assert that the saga will eventually yield `put`
    // with the expected action
    .put({ type: 'ADD', payload: 42 })

    // dispatch any actions your saga will `take`
    .dispatch({ type: 'HELLO' })

    // run it
    .run();
});

EDIT:

You can learn more at the repo link above or the docs here

@Andarist

This comment has been minimized.

Member

Andarist commented Jan 10, 2017

It looks really cool and promising. However im wondering how do you determine .dispatch() timing? I mean - are you detecting takes and then dispatching what's been registered?

I also need to check out how do you handle forked tasks, as that's what the most tricky part of the whole thing I guess.

Providing withStore wold be also cool, as it would allow even broader 'inegration' testing here, I think its not possible now to change the state in .put()'s response, so that later select would be correct if it depends on that part of the state.

@jfairbank

This comment has been minimized.

Contributor

jfairbank commented Jan 10, 2017

Thanks @Andarist. This is probably not the cleanest, most optimal code yet, but it works for simple cases, including forks as you noticed.

However im wondering how do you determine .dispatch() timing? I mean - are you detecting takes and then dispatching what's been registered?

Right now, dispatches have to be in order, so that could get tricky with more complex sagas. Basically, it queues up actions from the dispatch method. Then, whenever it encounters a take it pops the first action in the queue and notifies listeners.

I also need to check out how do you handle forked tasks, as that's what the most tricky part of the whole thing I guess.

I'm using a custom IO with the sagaMonitor callbacks effectTriggered and effectResolved to store yielded effects as well as track outstanding fork effects by effect id. Once those forks resolve (i.e. we get a task in effectResolved), I push the task into an array and utilize Promise.all along with the main task's Promise and the forked tasks' Promises. There's a dirty check too in case a saga gets forked later on, so expectSaga can put off resolving and schedule resolution later on. It might not be the most elegant solution right now, but it works.

Providing withStore wold be also cool, as it would allow even broader 'inegration' testing here, I think its not possible now to change the state in .put()'s response, so that later select would be correct if it depends on that part of the state.

Yes, I already opened an issue for integrating with real Redux stores, so this would definitely be useful!

@jimbol

This comment has been minimized.

jimbol commented Jan 24, 2017

I have been working on this generator test runner for the past couple months that my team seems to enjoy for testing sagas/effects. I laid out the API earlier in this thread.

I just added a match method, which allows you to define catch-alls for yielded values.

// In this example, all yields which meet the conditions of the `detectSelector` fn
// Will use the `callSelector` result instead of the normal iterator flow (using `runner.next()`)

// Returns true if step meets desired conditions, otherwise false
const detectSelector = (step: YieldedStep): boolean => {
  return step.value && step.value.SELECT;
};

// Returns argument to be passed into `step.next`
const callSelector = (step: YieldedStep): * => {
  return step.value.select.selector(); // call spy (edit: "spy" not "promise")
};

sinon.stub(selectors, 'getToken').returns('fake-token');
sinon.stub(selectors, 'barIdsSelected').returns([ 1, 2, 3 ]);

run = genRunner(onArchiveThreads)
  .match('select', detectSelector, callSelector)
  .next('start')
  .next('finish')
  .run();

This way it doesn't matter the order that our selectors are called, but we can still test the order of things that we care about.

@Andarist

This comment has been minimized.

Member

Andarist commented Jan 24, 2017

this one return step.value && step.value.SELECT; is reaching for rather private API, which should be avoided

and also i dont get why here return step.value.select.selector(); // call promise you are mentioning promises

@jimbol

This comment has been minimized.

jimbol commented Jan 25, 2017

Whoops, spy, not promise.

The selector detector could be any function, whatever makes the most sense! So long as it returns a bool. I would appreciate some advice on what is the best way to detect a selector from the yielded value. Would you share a better way?

@Andarist

This comment has been minimized.

Member

Andarist commented Jan 25, 2017

I dont get why then the API is:
.match('select', detectSelector, callSelector)

I mean - what's the purpose of puting select string there? if it is not detected automatically under the head for the user is it smth more than a label? Is it used somethow?

@jimbol

This comment has been minimized.

jimbol commented Jan 25, 2017

Yes! We can get the steps off the run later for testing.

For match, you get an array of the yielded values

const SELECT = 'select';
type Step = { next: Function, done: Boolean, value: * };
const run = genRunner(generator)
  .match(SELECT, detect, callSelector)
  .run();

const steps: Array<Step> = run.get(SELECT);
expect(steps[0].value).to.deep.equal(select(getThing));

For next, you get a single value this way.

const step: Step = run.get('finish');
expect(step.done).to.be.true();
@jimbol

This comment has been minimized.

jimbol commented Apr 5, 2017

What are your thoughts on snapshot testing sagas and effects?

@leimonio

This comment has been minimized.

leimonio commented May 9, 2017

There are some cases I conditionally yield on saga effects, and so I want to test both cases. This leads to repetition of unit tests, in order to reach that specific generator step where you handle this condition. Is it possible to write your unit tests by removing this repeated steps?

@neurosnap

This comment has been minimized.

Contributor

neurosnap commented May 9, 2017

It is possible to do forking but I have found that forking a generator for testing purposes creates an unwieldy mess that is difficult to maintain. We created a utility to make those arduous duplicated tests easier to deal with by making testing sagas virtually painless with https://github.com/jimbol/expect-gen

Using this utility we have reduced saga testing to be as easy as testing reducers.

function* myGen({ userId }) {
  const users = yield select(getUsers);
  const user = users[userId];
  yield put(registerUser(user));

  if (user.isAdmin) {
    yield put(showAdminPanel());
  }
}
// it should not trigger admin panel
const user = { id: 5, isAdmin: false };
expectGen(myGen, { userId: 5 })
  .yields(select(getUsers), { 5: user }) // asserts the select as first arg and provides the dummy data as second
  .yields(put(registerUser(user)) // assert
  .finishes() // generator.next().done = true
  .run();

// it should trigger admin panel, not bothering to assert what we already asserted
const user = { id: 5, isAdmin: true };
expectGen(myGen, { userId: 5 })
  .next({ 5: user }) // doesn't assert just goes to the next step with dummy data
  .next() // doesn't assert
  .yields(put(showAdminPanel()))
  .finishes()
  .run();

If our action creators return JSON serializable objects we can make this even easier with snapshot testing. We don't need to worry about asserting that the correct actions are being dispatched if we create a snapshot.

const user = { id: 5, isAdmin: false };
const snapshot = expectGen(myGen, { userId: 5 })
  .next({ 5: user })
  .next()
  .finishes()
  .toJSON(); // converts results from generator to JSON serializable format

expect(snapshot).toMatchSnapshot();

const user = { id: 5, isAdmin: true };
const snapshot = expectGen(myGen, { userId: 5 })
  .next({ 5: user })
  .next()
  .next()
  .finishes()
  .toJSON();

expect(snapshot).toMatchSnapshot();
@jfairbank

This comment has been minimized.

Contributor

jfairbank commented May 9, 2017

@kwnccc Of course I'm biased, but you can easily write this with redux-saga-test-plan with little duplication. Reusing the example saga that @neurosnap provided:

import { put, select } from 'redux-saga/effects';
import { expectSaga } from 'redux-saga-test-plan';

function* myGen({ userId }) {
  const users = yield select(getUsers);
  const user = users[userId];
  yield put(registerUser(user));

  if (user.isAdmin) {
    yield put(showAdminPanel());
  }
}

it('regular users cannot access admin panel', () => {
  return expectSaga(myGen, { userId: 5 })
    .provide([
      [select(getUsers), { '5': { isAdmin: false } }]
    ])
    .not.put(showAdminPanel())
    .run();
});

it('admins can access admin panel', () => {
  return expectSaga(myGen, { userId: 5 })
    .provide([
      [select(getUsers), { '5': { isAdmin: true } }]
    ])
    .put(showAdminPanel())
    .run();
});
@leimonio

This comment has been minimized.

leimonio commented May 9, 2017

@neurosnap I really like the above example, it seems to make testing sagas extremely concise. I will try to integrate your library in my tests and let you know. Testing against snapshots is really nice as a feature, is it also available in intermediate steps other than the last one? Or does the .toJSON() keeps a tree of all the steps until the saga is completed?

@jimbol

This comment has been minimized.

jimbol commented May 9, 2017

@kwnccc @neurosnap yes you can do snapshots on partial generator runs.

@flushentitypacket

This comment has been minimized.

flushentitypacket commented Jun 6, 2017

What do you all think about a simplistic approach like this?

import deepEqual from 'deep-equal'

const nextUntil = (gen, untilValue, value) => {
  let result = gen.next(value)
  while (!result.done) {
    if (deepEqual(result.value, untilValue)) return result
    result = gen.next()
  }
  return null
}

You can use this to test generator functions directly, similar to how @neurosnap's lib does.

Here's a practical example:

it('uses token from store to call someAuthenticatedRequest', () => {
  const gen = mySaga(action)
  expect(nextUntil(gen, select(getToken))).toBeTruthy()
  const token = 'harrypotterandthesorcererstoken'
  expect(nextUntil(gen, call(someAuthenticatedRequest, token), token)).toBeTruthy()
})
@DzoQiEuoi

This comment has been minimized.

DzoQiEuoi commented Jul 13, 2017

I agree with those who say the testing recommended in the official documentation is pointless.

To make my tests implementation agnostic I'm using Jest's async/await capabilities along with a mock store which just keeps a log of actions. This might need some tweaking if your sagas trigger timers.

import { createStore, applyMiddleware } from 'redux';
import createSagaMiddleware from 'redux-saga';
import api from '.path/to/api/service';
import saga from './path/to/sagas';

function mockStore(rootSaga) {
  const sagaMiddleware = createSagaMiddleware();
  const actionReducer = (state, action) => 
    action.type === '@@redux/INIT' ? [] : [...state, action];
  const store = applyMiddleware(sagaMiddleware)(createStore)(actionReducer);
  sagaMiddleware.run(rootSaga);
  return store;
}

describe('suite', () => {
  it('test', async () => {
    const store = mockStore(saga);
    api.someCall = jest.fn(() => 'some response');

    store.dispatch({
      type: 'SOME_ACTION',
      id: 123
    });
    
    await Promise.resolve();

    const actual = store.getState();
    const expected = [
      { type: 'SOME_ACTION', id: 123 },
      { type: 'SOME_OTHER_ACTION', payload: 'some response' }
    ];

    expect(actual).toEqual(expected);
    expect(api.someCall).toHaveBeenCalledWith(123);
  });
});
@iwinux

This comment has been minimized.

iwinux commented Dec 19, 2017

A contrived mimic of the official testing example would be

function calc(a, b) {
    return a + b
}

test('calc', () => {
    const spy = mock('+') /* suppose we can do this */
    calc(1, 2)
    expect(spy).toBeCalledWith(1, 2)
})

while the actual test I want to perform is:

test('calc', () => {
    expect(calc(1, 1)).toEqual(2)
    expect(calc(1, 2)).toEqual(3)
})

IMO the first way of testing is just duplicating the same logic in tests.

@SelaO

This comment has been minimized.

SelaO commented Jul 12, 2018

@3LOK Do you have an example on how to properly test saga (if at all) without the silly overhead that was discussed here?

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