Create examples & tests for tasks

[mjrussell]

This does not have to be merged, just using it as a tool for discussion

First of all, I hope its ok to "abuse" a PR like this. I thought about just making issues but I wanted to code up some of the difficulties I was facing in easy to understand examples. Also using the inline comments should be useful, and maybe it can merged as a fork/task example at some point.

Anyway...I really like redux-saga and Im trying to do some examples and get more familiar with using it (and es7 generators).

Recently I was playing around with the fork/cancel support and found it really useful for a "sync task", like a polling fetch of an item. You might want to stop the fetch at any time, or switch to polling a new item. And you shouldn't really poll more than an item at a time.

I was able to implement this somewhat easily, but ran into some snags when I began writing tests. I found it difficult to write "spec" tests for the fork/task and ended up giving up and using runSaga mainly. I've created an example for tasks that shows the gist of what I was doing and the tests. The Tests include a *** where I thought something could be improved or I found confusing. This was also done for 0.4.1, I recently tried 0.5.0 but my tests no longer pass (regression?).

Some general thoughts:

• Export delay since its a common pattern - Sure its trivial but at first I tried to import it from saga and did call(delay, 5) which ended up just sending an undefined action. Could the input validation be improved on call?
• Add easy cancellation to runSaga - You'll see I rolled my own for the tests

[mjrussell]

FYI the main difficulties in the test start to happen here: https://github.com/yelouafi/redux-saga/pull/45/files#diff-fe2186b9207791d7c78cdf4eb20515feR25

[mjrussell]

Also I havent used tape before so I think the "integration test" never ends for some reason, my mocha/chai test has a clean exit

[yelouafi]

@mjrussell Thanks for raising this issue.

If I understand the main problem is: how to resume the generator with a dummy result of the fork. Put in other words how to mock the result of a fork task. right ?

[mjrussell]

@yelouafi Thanks for the reply. Yeah I think the core of the issue is on testing the fork task, and I like your proposed syntax on #47.

I am also curious as to why this passes on 0.4.1 and not on 0.5.0, was there a breaking change in how call works?

In general would you tell people to avoid using the runSaga as a testing method and stay to the behavior driven? Otherwise it does seem like we are testing redux-saga as much as our own code.

[mjrussell]

Here's the output running against master (same for 0.5.0) and the branch

  behavior spec

    ✔ Waits for start
    ✔ Starts the task sync for an item

  fully running

anonymous: uncaught [Error: call/cps/fork first argument must be a function, an array [context, function] or an object {context, fn}]

    ✖ Error: call/cps/fork first argument must be a function, an array [context, function] or an object {context, fn}
    ------------------------------------------------------------------------------------------------------------------
      operator: fail
      at: run (/Users/matt/Hacking/redux-saga/node_modules/babel-polyfill/node_modules/core-js/modules/es6.promise.js:104:47)




  Failed Tests: There was 1 failure

    fully running

      ✖ Error: call/cps/fork first argument must be a function, an array [context, function] or an object {context, fn}


  total:     3
  passing:   2
  failing:   1
  duration:  7.1s

[yelouafi]

am also curious as to why this passes on 0.4.1 and not on 0.5.0, was there a breaking change in how call works?

I think the issue starts from here

https://github.com/yelouafi/redux-saga/pull/45/files#diff-fe2186b9207791d7c78cdf4eb20515feR83

In 0.4.1, you can call fork directly on an iterator (yield fork( saga() )). In 0.5.0 it was removed, and fork must be called as call and cps (sorry, forgot to mention this in the releases notes). so it should be (syncSaga without ())

 const task = runSaga(cancellableSaga(syncSaga), storeIO(store));

So this call wont fail

https://github.com/yelouafi/redux-saga/pull/45/files#diff-fe2186b9207791d7c78cdf4eb20515feR42

This is to provide an uniform support to all those function for providing context (this). Also it's much simpler to test equality on fork(saga, a1, a2).

In general would you tell people to avoid using the runSaga as a testing method and stay to the behavior driven? Otherwise it does seem like we are testing redux-saga as much as our own code.

In unit testing yes, IMO, it's more reliable and predictable.

[ghost]

Just some thoughts as I work through these problems myself...

I'm not sure that a put(..).should.equal(put(..)) or fork(...).should.eql(fork(...)) approach is the simplest path forward. It's only a marginal advantage/convenience to not have to mock out a service to return some data object, but it introduces weird implementation burdens on the testers - having to use some sort of library mocking service, for example.

I'm in a pinch where my ./some-feature-sagas.js exports only the root sagas for that feature, and a ./root-saga.js later scoops up these exports and forks all the processes. If we are going to test the "full" generator in a behavioral style - which may be many forks/calls/puts/takes, it seems very inconvenient to have to say something like:

        it('should dispatch a fetch request', function() {
            const mock = { id: 'foo' } 
            sut.next() // take the first action to start the gen
            sut.next(mock) // feed back some mock data present on the first action
            sut.next() // move into a fork or a call - not really testing anything here, just moving along
            sut.next().value.should.eql(io.put(creators.fetchRequest())) // testing the correct dispatch
            ... etc.
        })

On the other hand, if we want a unit-style test (which I am not a fan of ... I don't care that y was invoked and z were arguments, I want to know that invoking x returned the appropriate thing in the appropriate shape, or failed and threw the correct error, etc), we have to export all the sub-step/nested generators along the way. Exporting those generators places a burden on the app code - whether it's something cheesy like "if this feature-saga exports functions like _function, don't fork it from the root reducer", or something more complicated/orthogonal.

edits:
sorry, I got a new keyboard and keep missing keys and hitting tab -> update comment.

[mjrussell]

In 0.4.1, you can call fork directly on an iterator (yield fork( saga() )). In 0.5.0 it was removed, and fork must be called as call and cps (sorry, forgot to mention this in the releases notes). so it should be (syncSaga without ())

Aha! Thanks, updated and working now.

[yelouafi]

FYI. 0.6.0 added an utility functin createMockTask to mock fork results. see release notes for usage example

[mjrussell]

@yelouafi awesome thanks, I can close this then unless it would be worth cleaning it up with the examples and new way for mocking forks

[yelouafi]

I can close this then unless it would be worth cleaning it up with the examples and new way for mocking forks

I think it'd be util to include. But maybe we should change the title tasks to something more informative (kind of saga-test-sample)

[mjrussell]

maybe we should change the title tasks to something more informative (kind of saga-test-sample)

Will do!

[mjrussell]

Sorry I didn't get around to this sooner, been busy. I think theres a benefit to add some advanced testing examples to the docs/examples and will open a new PR when I find a little more time.

[yelouafi]

No worry :)