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

stub().onCall().returns() should return stub function #1100

Closed
zuzusik opened this issue Jul 13, 2016 · 7 comments
Closed

stub().onCall().returns() should return stub function #1100

zuzusik opened this issue Jul 13, 2016 · 7 comments

Comments

@zuzusik
Copy link
Contributor

zuzusik commented Jul 13, 2016

let a = sinon.stub().returns(12); // a is a stub function
let b = sinon.stub().onCall(0).returns(12); // b is not a stub function (but some object)
  • Sinon version : 1.17.4
@fatso83
Copy link
Contributor

fatso83 commented Jul 14, 2016

Not sure what this is. It certainly is not a bug, because those chainable methods have never returned a stub AFAIK, so perhaps this is a feature request? If so, I would suggest you try modifying this yourself and submit a PR that keeps the current API.

Maybe some of the other maintainers know something about the history of this api, and if doing so is a bad idea.

You basically want to avoid having to write the first line in this code:

let b = sinon.stub();
b.onCall(0).returns(12).onCall(1).returns(3);

Not sure if this is worth keeping open just to save you a line, so I am going to close this until further.

@fatso83 fatso83 closed this as completed Jul 14, 2016
@mvcds
Copy link

mvcds commented Dec 19, 2016

But I'm sure it has worked in the past, I've use it in another project.

@fatso83
Copy link
Contributor

fatso83 commented Dec 19, 2016

If either @zuzusik or @mvcds can report a version of Sinon where a stub was being returned we have something to work with, as that means we have a regression on our hands. I'll keep it closed until so.

@fatso83
Copy link
Contributor

fatso83 commented Dec 19, 2016

This might be related to #817, #823 and the PRs #1207 and #1209.

@zuzusik
Copy link
Contributor Author

zuzusik commented Dec 19, 2016

@fatso83 just investigated a bit, seems like this behavior exists from the very beginning of the implementation of .onCall() (v 1.8)

Also I noticed that this code works as expected:

let c = sinon.stub().returns(12); // a is a stub function
let b = sinon.stub().onCall(0).returns(12).stub.onCall(1).returns(13).stub; // b is also a stub function

this means that probably returns (and other behavior methods) should return not this, but this.stub || this - as far as I see this will guarantee consistency of this methods when being called either directly from stub, or in .onCall chain

what do you think?

@mvcds
Copy link

mvcds commented Dec 19, 2016

@fatso83 I am using sinon@1.17.6 where it behaved as @zuzusik reported.

Separating the code into two lines did the trick though I personally think it does not make as much sense as chaining.

Let me know if I can help you guys with something, I'm new to open source and I'm kind of lost on what or how I can do.

@fatso83
Copy link
Contributor

fatso83 commented Dec 20, 2016

@mvcds, we can ALWAYS need a hand in fixing stuff. There are more issues than spare time ;-) The biggest obstacle is often getting to the next step, because getting stuck in discussions on finer technical points is easy. So if you want to take a look at the mentioned issues and see if you can see where to go from here that could help in this specific case.

Other than that, just take a gander at the issue list, see if there are any low hanging fruits that would be easy to fix, and then create a pulk request with a fix.

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

3 participants