Sinon: callFake with callThrough override

[henit]

I need a function to be stubbed with a fake call, so tests usually use the fake. But then when I am going to test that function itself, I would need it to call the original code instead of the fake. I would think that could be done with something like:

sinon.stub(Foo, 'bar').callsFake(() => 'Fake res');

// Various tests that use the fake

Foo.bar.onFirstCall().callThrough();
// One test that need to call the original

// Various other tests that use the fake

But the fake is always called, even on the first call after .onFirstCall().callThrough(). Any reason why that is not possible? Fake as default, callThrough as override for first call...

[fatso83]

No idea - without consulting the source code. Fully possible not all code paths are covered when it comes to splitting up behavior over multiple calls like you do, but not sure whether this is intended or unintentional. I never assert more than one thing per test, so I seldom use multi-step setups like that.

[henit]

Same happens if I try to have a default result and override it in one place.

sinon.stub(Foo, 'bar').resolves(42);
Foo.bar.onFirstCall().callThrough(); // Resolves with 42, does not call original

[fatso83]

Any chance you could look into the behavior? It fairs a better chance of being fixed if someone with interest in getting the problem fixed and has some domain knowledge looks at it.

[henit]

I can see if I have a chance. But I have not yet looked at the sinon source, so I don't know how much time it will take to get to understand the architecture

[mroderick]

But I have not yet looked at the sinon source, so I don't know how much time it will take to get to understand the architecture

Feel free to ask questions here :)

[lucasfcosta]

Hi @henit, thanks for the issue and for wanting to help.

Something that has helped me a lot getting started with open source was having clear instructions on what to do. IMO we should incentivize people to contribute as much as possible when they're willing to instead of just sending PRs ourselves. This way we get more people on-board and in the long-term things go faster 😄

Anyway, enough open-source romance, here goes a few guidelines for this change:

• The default-behaviors file is what sets the flags we need on the stub object in order for them to behave properly when we call them or when we retrieve their values. The methods here are responsible for defining what happens when each behavior is added to a stub.
• The callThrough behavior, as you can see here
• The onFirstCall method on the stub adds a new behavior for the call with index 0. When invoking a stubbed function, it well look for behaviors for that call index and run them if they exist, as you can see here.
• That call invokes this function in the stub, which invokes a behavior if it exists and if isPresent returns true.
• However, as you can see here. The flag callThrough is not taken into account to see if a behavior is present or not, which is why no behavior is returned and therefore it is not called on the first time the stubbed function is invoked.

So, in order to solve that you just need to add that criteria to isPresent, like this:

    isPresent: function isPresent() {
        return (typeof this.callArgAt === "number" ||
                this.exception ||
                typeof this.returnArgAt === "number" ||
                this.returnThis ||
                typeof this.throwArgAt === "number" ||
                this.fakeFn ||
                this.callThrough || // Take a look at this line
                this.returnValueDefined);
    },

This also opens a precedent for us to take into account other flags as resolves and rejects when deciding whether or not a behavior for a call index exist. The fix for them is the same as this one. All you've gotta do is add that to this check.

I hope you are able to understand how and why this fix works and send us a PR with tests. Basically the same thing you've demonstrated in this issue would be amazing as a test.

And finally I also highly encourage anyone reading this to also explain issues instead of solving them yourselves, this brings more people in and makes it easier for them to get started. Let's keep the OSS community growing! ❤️

[mroderick]

Is this issue dead?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[rugk]

Is this issue dead?

Yes. :thinking: