jfairbank / redux-saga-test-plan

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

[Proposal] Use the testing framework assertion library for better integration #262

Open mike1808 opened 5 years ago

mike1808 commented 5 years ago

The current behavior is unmet expectation is to throw an error with a formatted string of unmet expectations.

For small examples it is not an issue, however, when you have nested objects to assert (e.g. 2 level deep objects) or many effects to except the error message becomes very unreadable. It is hard to find what exactly didn't match.

If we use the testing framework, for example jest, assertion library (expect) errors will be printed using the library's reporter.

In my case, they will be formatted by WebStorm reporter which have a nice diff view which I missing when using redux-saga-test-plan.

What do you think about this proposal?

jp928 commented 5 years ago

@mike1808 Thanks for the proposal and pull request would be more than welcomed. Please let me know if you need any assistance.

fstoerkle commented 5 years ago

@mike1808 @jp928 here is a work-in-progress branch for always using jest-diff, but using the expect from the test framework would be even nicer!

borrascador commented 5 years ago

@fstoerkle Where can I find this expect method you mentioned? I'm interested in trying to make that change and open a PR

mike1808 commented 5 years ago

@borrascador it's the built-in expect in Jest. Try to use it in https://github.com/jfairbank/redux-saga-test-plan/blob/master/src/expectSaga/expectations.js file instead of throw

ivan-aksamentov commented 5 years ago

Can't wait this feature!

This would resolve the confusing assertions like:

SagaTestError: 
    Assertion 3 failed: put effects do not match

    Expected
    --------
    { '@@redux-saga/IO': true,
      combinator: false,
      type: 'PUT',
      payload: 
       { channel: undefined,
         action: 
          { type: 'FSA_TEST/REPLY_FAILED',
            payload: { params: [Object], error: [Object] },
            error: true } } }

    Actual
    ------
    { '@@redux-saga/IO': true,
      combinator: false,
      type: 'PUT',
      payload: 
       { channel: undefined,
         action: 
          { type: 'FSA_TEST/REPLY_FAILED',
            payload: { params: [Object], error: [Object] },
            error: true } } }

In this case payload.action.payload.error.message don't match, but you cannot figure it out from the message because the object serialization does not reach the necessary depth.

mike1808 commented 5 years ago

Hi @jp928 . I have started working on this feature and have the following steps to complete and wanted to discuss them with you:

Results so far:

In terminal when checking for PUT actions.

image

In WebStorm for the same test

image

With supported diff view

image

A comparison with the real project test:

After:

image

Before:

image

ivan-aksamentov commented 5 years ago

@mike1808 Looks great! Is it possible to also add diffs for partial matches?

mike1808 commented 5 years ago

@ivan-aksamentov what do you mean by partial match? Can you describe a use case? If that something that possible in Jest it'd be possible here because custom watchers have access to Jest API.

mike1808 commented 5 years ago

@ivan-aksamentov if you referring to your previous message, I think, that it's not so trivial, as redux-test-saga-plan doesn't know which particular effect you have expected (you may have the same action dispatched with different payloads). However, I think, the library can suggest you have a look at particular actions which were dispatched by comparing type then other values in payload.

ivan-aksamentov commented 5 years ago

@mike1808 Like in your "With supported diff view" screenshot, but in console https://pbs.twimg.com/media/D4UMeMUUwAIAfGD.png I think jest-diff is what implements this feature in jest.

Edit: Ah, I see. Are you saying that for integration testing mode the order in which actions are expected and emitted is not the same so the mapping between them is uncertain? Is there a way to make diffs work at least in unit testing mode, where the order of actions is enforced? The intended use-case is mostly comparing payloads, for both modes.

mike1808 commented 5 years ago

@ivan-aksamentov yes, currently I'm focused on integration tests. For unit testing that is 100% doable. Jest exposes deep comparison which will produce the same output as expect().isEqual().

I'll add unit testing support to my to-do list, thanks

dbartholomae commented 5 years ago

Any news here? Is there some way I could help? :)

mike1808 commented 5 years ago

@dbartholomae Ah, I joined a new company and started doing Go and kinda forgot about this PR. I'll try to bring to some workable state and commit this weekend. So everyone who's interested can continue the work.

dbartholomae commented 5 years ago

@mike1808 Thanks! If you don't find time to bring it in a workalbe state, please feel free to commit push as-is :)

mike1808 commented 5 years ago

For those who are interested. Unfortunately, I didn't have time to make the code more organized, however, Jest integration works perfectly. I tested my previous project which had a lot of saga tests using this library.

The code is available in my forked repo on this branch https://github.com/mike1808/redux-saga-test-plan/tree/beta-jest

You can try on your projects by installing jest-redux-saga-test-plan@4.0.0-beta.5

Few caveats:

  1. This branch is created from upgrade-to-babel7 as its new versions of babel was required to use the new Jest version.
  2. The compatibility for old Node.js might be broken because I upgraded Babel and removed some polyfills in the code (like Map), so my fork might not work on Node.js < 8
  3. The main code changes are in src/shared/serializeEffect.js file. There you can find that some effects like CANCELLED, FLUSH, CANCEL, JOIN are not implemented.
  4. The changes are only for expectSaga. I didn't touch the unit testing API.

If you have any question about code, feel free to write here, or, if you want to continue my work, we can set up a zoom/skype call and I'll share all context I have.

cc @dbartholomae @vov462 @Taguhi

jp928 commented 4 years ago

Mark this need to be merged before a stable release 4.0.0