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

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

Closed
cefn opened this issue Oct 18, 2019 · 1 comment
Closed

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

cefn opened this issue Oct 18, 2019 · 1 comment

Comments

@cefn
Copy link
Contributor

cefn commented Oct 18, 2019

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...

onReturn(result.value);

...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
Copy link
Contributor Author

cefn commented Oct 19, 2019

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.

@cefn cefn closed this as completed Oct 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant