substantial / sinon-stub-promise

Synchronous Promise stubbing for Sinon.JS
MIT License
85 stars 11 forks source link

finally does not return anything, so chaining another then/reject on it is not possible #22

Closed jhilden closed 7 years ago

jhilden commented 8 years ago

The finally implementation of this library does not return anything. Therefore it is not possible to chain additional .then() or .reject() on it.

Here is some pseudocode to demonstrate the problem:

app code:

functionUnderTest(ajax) {
  ...
  return ajax()
    .then( ... )
    .catch( ... )
    .finally(stopLoadingAnimation)
}

test code:

desribe('functionUnderTest', () => {
  let ajaxStub = sinon.stub().returnsPromise().resolves(...):
  it('should work', function() {
    functionUnderTest(ajaxStub).then(() => {
      // some assertion
    });
  });
});

this will lead to an error cannot call method then on undefined.

I think the solution is simply to add a return this; at the end of the finally implementation, but I'm not sure if that would have any additional consequences.

amay commented 7 years ago

I believe you are correct that changing the return here https://github.com/substantial/sinon-stub-promise/blob/master/index.js#L70 to return this; would do the trick. I don't have time to make an update on this project right now, but if you want to open up a PR that would be great.

jhilden commented 7 years ago

I tried doing that change locally and it succeeded in returning a promise object in the end (to chain additional calls on it).

However, then I had a new issue where the promise that was rejected inside the functionUnderTest() was suddendly getting resolved in the context of the test code (that was the promise that was now returned by finally).

Because of these issues we decided to simply move on with a different solution. So unfortunately I won't be able to submit a PR for this.