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

expectSaga test failure on missing assertions #99

Open
adamterlson opened this issue Apr 27, 2017 · 2 comments
Open

expectSaga test failure on missing assertions #99

adamterlson opened this issue Apr 27, 2017 · 2 comments

Comments

@adamterlson
Copy link

adamterlson commented Apr 27, 2017

Feature request: it'd be wonderful to be able to assert that a given saga yields exactly the specified effects--no more, and (critically for this ticket) no less.

The following example demonstrates the issue:

function* mainSaga(x, y) {
  yield take('HELLO');
  yield put({ type: 'ADD', payload: x + y });
  yield take('WORLD');
  yield put({ type: 'DONE' });
}

it('PARTIALLY handles dispatching actions', () => {
  return expectSaga(mainSaga, 40, 2)
    .put({ type: 'DONE' })
    // Put assertion on "ADD" is missing so the test does not break
    .run();
});

So perhaps a new helper which ensures that all side effects have an assertion would be nice to help developers be sure that their saga is appropriately covered:

it('EXPLICITLY handles dispatching actions', () => {
  return expectSaga(mainSaga, 40, 2)
    .put({ type: 'DONE' })
    .runExactly();
   // Put assertion on "ADD" is missing, test breaks!
});

I'm just making up a crappy API that probably won't work. Welcome your thoughts!

@adamterlson adamterlson changed the title expectSaga exactly assertion expectSaga test failure on missing assertions Apr 27, 2017
@jfairbank
Copy link
Owner

Hi, @adamterlson!

I like the spirit behind this idea, but requiring exact effects sounds like it couples the test to the implementation too much. If we required exact effects, then you'd also have to assert that the example saga yields take('HELLO') and take('WORLD').

If the desire is to require exact effects of a certain type, then we have a couple options. We could add a method like .put.exactly() as shown below:

it('EXPLICITLY handles dispatching actions', () => {
  return expectSaga(mainSaga, 40, 2)
    .put.exactly([
      { type: 'DONE' },
      { type: 'ADD', payload: 42 },
    ])
    .run();
});

Or we could potentially expose all yielded effects in the fulfilled value of the promise, as you and I discussed in #72.

it('EXPLICITLY handles dispatching actions', async () => {
  const effects = await expectSaga(mainSaga, 40, 2)
    .put({ type: 'ADD', payload: 42 })
    .put({ type: 'DONE' })
    .run();

  expect(effects.put.length).toBe(2);
});

What are your thoughts?

@adamterlson
Copy link
Author

adamterlson commented May 1, 2017

I like the spirit behind this idea, but requiring exact effects sounds like it couples the test to the implementation too much. If we required exact effects, then you'd also have to assert that the example saga yields take('HELLO') and take('WORLD').

You're right, I would indeed need to assert that the saga yields those takes if my runExactly were implemented. I copied from one of the doc samples and forgot to remove that bit from the saga as I did from the test! Whoops!

As for implementation coupling, I assume you don't feel that way toward the put.exactly example you gave? I certainly see your concern applying to .runExactly, but not put.exactly as it's just a more powerful assertion.

My thought right now is: why not both? Having put.exactly as shown would be quite nice (does it also give the ability to make assertions on effect order?), while having all effects available would make possible some very powerful checks that are not otherwise possible with the put.exactly helper.

I would also want a way to get all effects, of all types, in the order they were yielded. That is, expect(effects.length).toBe(2) and expect(effects[1].type).toBe('PUT')

Thanks for the discussion!

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

No branches or pull requests

2 participants