.calledWith() on a stub broke in sinon 4.4.10

[tomi]

• Sinon version : 4.4.10
• Environment : Chrome 62
• Example URL : -
• Other libraries you are using: mocha, testem

What did you expect to happen?
Tests to work as before

What actually happens
Tests break with '.calledWith is not a function' exception.

How to reproduce

1. Create a stub

const stub = sinon
  .stub()
  .onCall(0)
  .resolves(createResponse)
  .onCall(1)
  .resolves(validateResponse)
  .onCall(2)
  .resolves(downloadResponse)

2. Call calledWith on that stub

const stub.calledWith(...)

Downgrading to 4.4.9 solves the issue.

[fatso83]

@nfriedly, this is most probably related to merging #1738. Could you shine some light on this?

Runnable example

const sinon = require("./lib/sinon.js");
function createResponse() {
    return 42;
}
function validateResponse() {
    return true;
}
function downloadResponse() {}

const stub = sinon
    .stub()
    .onCall(0)
    .resolves(createResponse)
    .onCall(1)
    .resolves(validateResponse)
    .onCall(2)
    .resolves(downloadResponse);

stub(100);
console.log("calledWith", stub.calledWith(100)); // fails with '... is not a function'

[nfriedly]

Yea, I think this variation will work as desired:

const stub = sinon.stub();

stub.onCall(0)
 .resolves(createResponse) 
 .onCall(1)
 .resolves(validateResponse)
 .onCall(2)
 .resolves(downloadResponse);

[fatso83]

After first thinking this is real regression (I marked 4.4.10 transiently as deprecated), I now think this actually just shines some light on an undocumented "feature"/behaviour, as I recognize the current behaviour as how things have used to work in the past.

I think this might as well be addressed by adding an explicit test in our test arsenal that asserts that the behaviour adding methods (such as onCall, resolves, etc) does not return the stub. I think the provided example from @nfriedly is a reasonable cost for those being affected by this.

What do you think, @mroderick?

[fatso83]

Closing this as a non-bug, as our official stance has been that using inlined stubs is not supported: #1748 (comment)

I'll try to open a new feature request issue to see if we can improve the situation.

[mroderick]

I think this might as well be addressed by adding an explicit test in our test arsenal that asserts that the behaviour adding methods (such as onCall, resolves, etc) does not return the stub. I think the provided example from @nfriedly is a reasonable cost for those being affected by this.

What do you think, @mroderick?

I think that would be a great idea, as it makes it clear what the intention is, without having to resort to using git bisect to figure it out.

[fatso83]

I have done a lot git bisectiing on this now, and it seems explicit support for chaining behaviour methods was merged in @zuzusik's #1227 PR, meaning it was (a badly documented) part of the API.

A problem is that the feature description was not fully covered by the tests in that same PR. The tests explicitly just tested that onCall().returns() chaining worked, not the general behaviour concept. Another problem is of course that I didn't understand all the implications this would have, as our tests are very focused and avoid the intricacies of long method chains (where the problems of fluent API really comes to the surface).

Fluent APIs always gets these problem at some point, when you start returning a different object than you originally started with 🤢

As a temporary solution I have deprecated 4.4.10, but we will need to discuss the next step.

[mroderick]

Please continue this discussion in #1757