jfairbank / redux-saga-test-plan

Test Redux Saga with an easy plan.
http://redux-saga-test-plan.jeremyfairbank.com
MIT License
1.25k stars 127 forks source link

returnValue (mistakenly?) set to Redux-Saga signalling value #317

Closed cefn closed 5 years ago

cefn commented 5 years ago

While trying to assert that a saga DID NOT complete and return a value in a particular test scenario, (assert that scenario is still active, waiting on a take()), I thought I could get the returnValue from the result promise of expectSaga(...).run(), then assert that the returnValue was undefined.

However, the returnValue of a saga which hasn't returned by the end of the scenario, resolves to the string "@@redux-saga/TASK_CANCEL".

Is this intended? In the self-contained MCVE example below, the first test fails, and the others succeed.

const { take } = require("redux-saga/effects")
const { expectSaga } = require("redux-saga-test-plan")
const { expect } = require("chai-jasmine")

describe("returnValue expected to be undefined", () => {

  const noopReducer = (state = [], action) => state

  function* giveUp() {
    yield take("*")
  }

  function* neverGonnaGiveYouUp() {
    while (true) {
      yield take("*")
    }
  }

  //this test fails with "AssertionError: expected '@@redux-saga/TASK_CANCEL' to be undefined"
  it("Continuous saga has undefined return value", async () => {
    const { returnValue } = await expectSaga(neverGonnaGiveYouUp)
      .withReducer(noopReducer)
      .dispatch({ type: "NEVER_GONNA_LET_YOU_DOWN" }).run()

    expect(returnValue).toBeUndefined()
  })

  //this test passes
  it("Instant saga returns undefined as expected", async () => {
    const { returnValue } = await expectSaga(giveUp)
      .withReducer(noopReducer)
      .dispatch({ type: "NEVER_GONNA_RUN_AROUND_AND_DESERT_YOU" }).run()

    expect(returnValue).toBeUndefined()
  })

  //this test passes
  it("Continuous saga has returnValue of special Redux-Saga string", async () => {
    const { returnValue } = await expectSaga(neverGonnaGiveYouUp)
      .withReducer(noopReducer)
      .dispatch({ type: "NEVER_GONNA_MAKE_YOU_CRY" }).run()

    expect(returnValue).toBe("@@redux-saga/TASK_CANCEL")
  })

})

I speculate that the complete() function at... https://github.com/jfairbank/redux-saga-test-plan/blob/47598c72addde3b11566cc203ad7b5d5d74089a9/src/expectSaga/sagaWrapper.js#L59 ...should check for and intercept redux-saga's task-cancellation return values, rather than faithfully setting the return value whenever next() has a done flag, regardless of whether it was the saga itself returning, or the redux-saga wrapper generating a cancellation.

Does this make sense?

Update

In my main test suite I am seeing the following log for all cases of saga scenarios which DON'T return...

[WARNING]: Saga exceeded async timeout of 250ms

It's possible there's something I'm doing wrong in the composition of non-returning scenarios which leads the sagas to be waited on, then finally cancelled.

If there is some more official way to assert scenarios over unfinished sagas, which doesn't trigger timeouts and (therefore) cancellations perhaps someone could point out how, so I can close this ticket.

cefn commented 5 years ago

I believe the documentation at... http://redux-saga-test-plan.jeremyfairbank.com/integration-testing/timeout.html ...clarifies the problem and indicates all the relevant options.

I had assumed timeouts weren't relevant to my case since there was no async logic at all in my test scenario. Given there was no promise to wait on between the last .dispatch() of my test scenario and the eventual 250ms timeout, I didn't expect it to be waiting at all, but rather just evaluating all the assertions after the last .dispatch() and returning immediately.

Since it WAS in fact waiting, then the cancellation signature makes sense.

I understand now that there's no way for the suite to know the scenario is fully synchronous. My original understanding was that no promise was returned from any generator or provided mock. I was therefore not expecting any async treatment or timeouts.

However, given it's legitimate to blend e.g. redux-thunk with redux-saga or just plain promises into Sagas, there could have been promises triggered without passing through a yield which are waiting to resolve 'out of band'. Neither redux-saga or redux-saga-test-plan can know about these or track them, hence the 250ms default timeout.