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

returnValue of calls is out of order when spying on recursive functions #1849

Open
justinlovinger opened this issue Jun 29, 2018 · 5 comments
Labels

Comments

@justinlovinger
Copy link

justinlovinger commented Jun 29, 2018

Describe the bug

The return value, as given by spy.getCall(n).returnValue, is in the wrong order when spying on a recursive function. The return values are in the order that calls returned, instead of the order of the calls themselves. For example, the returnValue of the first call is the value returned by the last call, instead of the value returned by the first call.

Note that this bug occurs with both sinon.spy and sinon.fake.

To Reproduce

recurse.js

function foo(num) {
    if (num < 2) {
        foo(num + 1);
    }

    return num;
}

recurse.test.js

const rewire = require('rewire');
const expect = require('chai').expect;
const sinon = require('sinon');

const recurse = rewire('../recurse');


describe('foo', function () {
    it('should recursively add 1 but return original number', function () {
        recurse.__with__({
            foo: sinon.fake(recurse.__get__('foo'))
        })(function () {
            recurse.__get__('foo')(0);

            expect(recurse.__get__('foo').getCall(0).returnValue).to.equal(0);
            expect(recurse.__get__('foo').getCall(1).returnValue).to.equal(1);
            expect(recurse.__get__('foo').getCall(2).returnValue).to.equal(2);
        });
    });
});

Expected behavior

spy.getCall(n).returnValue should be the return value of the nth call to spy.

Context:

  • Library version: 6.0.1
  • Environment: Windows
  • Other libraries you are using: mocha, chai, rewire
@fatso83
Copy link
Contributor

fatso83 commented Jun 30, 2018

Hmm ... any idea of how to fix this?

@mroderick
Copy link
Member

I edited the description to add language annotations to the the fenced blocks to allow GFM to do syntax highlighting

@fearphage
Copy link
Member

This doesn't work the way you think. The problem is the inner function has a closure so it never calls the (outer) fake in my testing. Is rewire fixing that somehow?

http://jsbin.com/sevesibibu/edit?js,console

I don't think this is a problem that sinon can resolve honestly.

@justinlovinger
Copy link
Author

@fearphage Your example is not actually replacing the function foo with a fake. You are instead creating a new function that wraps foo, and calling that.

Compare to this:

let count = 0;

function foo(num) {
    console.log(`foo called: ${++count}; num: ${num}`);
  
    if (num < 5) {
        return num + foo(num + 1);
    }

    return num;
}

foo = sinon.fake(foo);

const result = foo(2);

console.log(`fake call count: ${foo.callCount}, result: ${result}`);

@stale
Copy link

stale bot commented Sep 7, 2018

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.

@stale stale bot added the stale label Sep 7, 2018
@mroderick mroderick added pinned and removed stale labels Sep 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants