sinonjs / sinon

Test spies, stubs and mocks for JavaScript.
https://sinonjs.org/
Other
9.63k stars 770 forks source link

Feature Request: Chaining sinon.stub() with multiple .withArgs() #2419

Closed jelazos7 closed 2 years ago

jelazos7 commented 2 years ago

This is in response to https://github.com/sinonjs/sinon/issues/176

The above issue was closed as a wont-fix because it was argued that chaining is less clear than assigning a variable. So for example:

sinon.stub(obj, 'function')
  .withArgs('a').resolves('b')
  .withArgs('c').resolves('d')

was argued to be less clear than

const objFuncStub = sinon.stub(obj, 'function')
objFuncStub.withArgs('a').resolves('b')
objFuncStub.withArgs('c').resolves('d')

I stumbled into this issue because I attempted to do chaining first, assuming that chaining was the natural way to use the sinon API, as it is with chai. In my case, I was stubbing a function but not verifying the calls on it, so i do not need to keep a variable to the stub. In my example above i feel that the chaining option is more crisp and just as easy to understand, especially since the variable is not necessary.

Would it be a breaking change or infeasible to add chaining?

fatso83 commented 2 years ago

Hi, John and thanks for getting in touch with us!

It would of course be a breaking change, but that would be nothing new and it is not what is stopping us. This whole discussion is quite old by now, but AFAIK we have gone down this route multiple times, changing the API back and forth . In the end, I think it was decided that it was really hard to support the chained calls while not breaking some other bit of functionality (involuntarily). A quite significant piece of all our issues relates to withArgs.

AFAIK, this is one of the reasons we created the much simpler Fakes API that is much simpler to reason about and makes for simpler tests and code.

I haven't touched the old API in years for that reason, and sinon.fake(func); covers what I need when the existing API is too minimal:

sinon.fake((arg) => {
  if(arg == 'a') return 'b';
  if(arg == 'c') return 'd';
  return 'fallback';
});

So I think it just comes down to a lack of interest in maintaining the old Stubs API, since the chaining bit comes with quite a bit of mental overhead. Since there is quite limited maintainer resources available, and even less interest in this, I do not see a new breaking change to this old API as a good decision right now.