Sinon: stub#returnsArg supercedes stub#returns even if returns is called later

[platinumazure]

I'm unfortunately stuck with sinon 1.3.x (yes, I know, way old), so please forgive me if this is in fact addressed in a later release. I found nothing when searching the issues archive, at least.

It looks like calls to returns does not supercede calls to returnsArg. Since (outside of the new onCall(n) interface) calls to returns or returnsArg overwrite earlier calls to the same functions, it seems to me that the returns call should clobber the returnsArg information if it occurs later. This keeps the behavior consistent-- in all cases, whether you're calling returns or returnsArg, you're changing what value you expect to be returned from the stub, so it makes sense to me simply to clobber all return-related state on the stub when calling any of those functions.

Here's a test case (written in QUnit):

test("returns should clobber returnsArg if called later", function (assert) {
    QUnit.expect(1);

    // arrange
    var stub = sinon.stub.returnsArg(0);
    stub.returns("new value");

    // act
    var result = stub("first argument");

    // assert
    assert.strictEqual(result, "new value", "New return value is returned since returns was called last");
});

[cjohansen]

Please verify if this is a problem on master before opening issues.

[mAiNiNfEcTiOn]

I think this issue should be re-opened as I did a fresh install with the 1.17.2 and I still have the same problem.

The quick-fix for this problem is to re-do the stubbing.

stub.restore();
sandbox.stub(MyClass, 'myMethod').returns(myReturnValue);

And yes, I am using sandboxes. If that's not reproducible in the sinon object, it is for sure in the sandbox one.

EDIT: (had a typo on my example (wrote resolve instead of restore -.-')

[Cinamonas]

@cjohansen This is still an issue, which I bumped into just now. See a fiddle: http://jsbin.com/dobopohete/1/edit?html,js,console

The issue should be reopened.

[fatso83]

@Cinamonas : tried your fiddle. does not work. That version of sinon has no stub property.

[Cinamonas]

@fatso83 Sorry about that, apparently JS Bin didn't save my last changes! I have updated the fiddle: http://jsbin.com/dobopohete/1/edit?html,js,console

[fatso83]

That seems legit, @Cinamonas . Reopened.

[tfrommen]

I also just ran into this. There didn't happen anything for more than four months. Will this be fixed somewhen?

The problem is a combination of the invoke method:

invoke: function invoke(context, args) {
    callCallback(this, args);

    if (this.exception) {
        throw this.exception;
    } else if (typeof this.returnArgAt === "number") {
        return args[this.returnArgAt];
    } else if (this.returnThis) {
        return context;
    }

    return this.returnValue;
}

and that calling any of the returns* methods doesn't unset the according value(s) of the others:

returns: function returns(value) {
    this.returnValue = value;
    this.returnValueDefined = true;
    this.exception = undefined;

    return this;
},

returnsArg: function returnsArg(pos) {
    if (typeof pos !== "number") {
        throw new TypeError("argument index is not number");
    }

    this.returnArgAt = pos;

    return this;
},

returnsThis: function returnsThis() {
    this.returnThis = true;

    return this;
}

If you're interested, I could provide a PR for this...?

[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.

[kgregory]

According to the documentation:

Defining stub behavior on consecutive calls
Calling behavior defining methods like returns or throws multiple times overrides the behavior of the stub.

It doesn't work that way with the various methods that allow users to specify what a stub returns. It isn't very intuitive that using returns after returnsArg doesn't override the behavior. For example:

  const stubbedFunction = sandbox.stub();
  stubbedFunction.returnsArg(0);
  stubbedFunction.returns('foo');

  // returns 'bar'
  stubbedFunction('bar');

Since returns was used after returnsArg, the expectation would be that the stub would return 'foo', but this isn't the case because the default behavior is to determine the return value by considering the possibilities in priority order (and they are only cleared when reset or resetBehavior is used).

If reuse of stubs is promoted and altering the behavior is meant to override previous behaviors, returns, returnsArg, and returnsThis should undo what has been configured.

This issue should be reopened.

cc: @mroderick @cjohansen @fatso83

[mroderick]

Re-opening this issue