Skip to content

Commit

Permalink
Avoid return and callArg* clearing each other's state (#2593)
Browse files Browse the repository at this point in the history
* Add regression test for #2572

* Partially revert "fix returns does not override call through (#2567)"

This partially reverts commit 5fde5ae:
- we keep the tests
- revert to the old manual clearing of props

The clearing of callThrough has then been manually added to what I
deem are the relevant spots. Tests are unaltered.
  • Loading branch information
fatso83 committed Apr 25, 2024
1 parent ed068a8 commit 5025d00
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 71 deletions.
145 changes: 74 additions & 71 deletions lib/sinon/default-behaviors.js
Expand Up @@ -30,150 +30,138 @@ function throwsException(fake, error, message) {
}
}

const SKIP_OPTIONS_FOR_YIELDS = {
skipReturn: true,
skipThrows: true,
};

function clear(fake, options) {
fake.fakeFn = undefined;

fake.callsThrough = undefined;
fake.callsThroughWithNew = undefined;

if (!options || !options.skipThrows) {
fake.exception = undefined;
fake.exceptionCreator = undefined;
fake.throwArgAt = undefined;
}

fake.callArgAt = undefined;
fake.callbackArguments = undefined;
fake.callbackContext = undefined;
fake.callArgProp = undefined;
fake.callbackAsync = undefined;

if (!options || !options.skipReturn) {
fake.returnValue = undefined;
fake.returnValueDefined = undefined;
fake.returnArgAt = undefined;
fake.returnThis = undefined;
}

fake.resolve = undefined;
fake.resolveThis = undefined;
fake.resolveArgAt = undefined;

fake.reject = undefined;
}

const defaultBehaviors = {
callsFake: function callsFake(fake, fn) {
clear(fake);

fake.fakeFn = fn;
fake.exception = undefined;
fake.exceptionCreator = undefined;
fake.callsThrough = false;
},

callsArg: function callsArg(fake, index) {
if (typeof index !== "number") {
throw new TypeError("argument index is not number");
}
clear(fake);

fake.callArgAt = index;
fake.callbackArguments = [];
fake.callbackContext = undefined;
fake.callArgProp = undefined;
fake.callbackAsync = false;
fake.callsThrough = false;
},

callsArgOn: function callsArgOn(fake, index, context) {
if (typeof index !== "number") {
throw new TypeError("argument index is not number");
}
clear(fake);

fake.callArgAt = index;
fake.callbackArguments = [];
fake.callbackContext = context;
fake.callArgProp = undefined;
fake.callbackAsync = false;
fake.callsThrough = false;
},

callsArgWith: function callsArgWith(fake, index) {
if (typeof index !== "number") {
throw new TypeError("argument index is not number");
}
clear(fake);

fake.callArgAt = index;
fake.callbackArguments = slice(arguments, 2);
fake.callbackContext = undefined;
fake.callArgProp = undefined;
fake.callbackAsync = false;
fake.callsThrough = false;
},

callsArgOnWith: function callsArgWith(fake, index, context) {
if (typeof index !== "number") {
throw new TypeError("argument index is not number");
}
clear(fake);

fake.callArgAt = index;
fake.callbackArguments = slice(arguments, 3);
fake.callbackContext = context;
fake.callArgProp = undefined;
fake.callbackAsync = false;
fake.callsThrough = false;
},

usingPromise: function usingPromise(fake, promiseLibrary) {
fake.promiseLibrary = promiseLibrary;
},

yields: function (fake) {
clear(fake, SKIP_OPTIONS_FOR_YIELDS);

fake.callArgAt = useLeftMostCallback;
fake.callbackArguments = slice(arguments, 1);
fake.callbackContext = undefined;
fake.callArgProp = undefined;
fake.callbackAsync = false;
fake.fakeFn = undefined;
fake.callsThrough = false;
},

yieldsRight: function (fake) {
clear(fake, SKIP_OPTIONS_FOR_YIELDS);

fake.callArgAt = useRightMostCallback;
fake.callbackArguments = slice(arguments, 1);
fake.callbackContext = undefined;
fake.callArgProp = undefined;
fake.callbackAsync = false;
fake.callsThrough = false;
fake.fakeFn = undefined;
},

yieldsOn: function (fake, context) {
clear(fake, SKIP_OPTIONS_FOR_YIELDS);

fake.callArgAt = useLeftMostCallback;
fake.callbackArguments = slice(arguments, 2);
fake.callbackContext = context;
fake.callArgProp = undefined;
fake.callbackAsync = false;
fake.callsThrough = false;
fake.fakeFn = undefined;
},

yieldsTo: function (fake, prop) {
clear(fake, SKIP_OPTIONS_FOR_YIELDS);

fake.callArgAt = useLeftMostCallback;
fake.callbackArguments = slice(arguments, 2);
fake.callbackContext = undefined;
fake.callArgProp = prop;
fake.callbackAsync = false;
fake.callsThrough = false;
fake.fakeFn = undefined;
},

yieldsToOn: function (fake, prop, context) {
clear(fake, SKIP_OPTIONS_FOR_YIELDS);

fake.callArgAt = useLeftMostCallback;
fake.callbackArguments = slice(arguments, 3);
fake.callbackContext = context;
fake.callArgProp = prop;
fake.callbackAsync = false;
fake.fakeFn = undefined;
},

throws: throwsException,
throwsException: throwsException,

returns: function returns(fake, value) {
clear(fake);

fake.callsThrough = false;
fake.returnValue = value;
fake.resolve = false;
fake.reject = false;
fake.returnValueDefined = true;
fake.exception = undefined;
fake.exceptionCreator = undefined;
fake.fakeFn = undefined;
},

returnsArg: function returnsArg(fake, index) {
if (typeof index !== "number") {
throw new TypeError("argument index is not number");
}
clear(fake);
fake.callsThrough = false;

fake.returnArgAt = index;
},
Expand All @@ -182,33 +170,42 @@ const defaultBehaviors = {
if (typeof index !== "number") {
throw new TypeError("argument index is not number");
}
clear(fake);
fake.callsThrough = false;

fake.throwArgAt = index;
},

returnsThis: function returnsThis(fake) {
clear(fake);

fake.returnThis = true;
fake.callsThrough = false;
},

resolves: function resolves(fake, value) {
clear(fake);

fake.returnValue = value;
fake.resolve = true;
fake.resolveThis = false;
fake.reject = false;
fake.returnValueDefined = true;
fake.exception = undefined;
fake.exceptionCreator = undefined;
fake.fakeFn = undefined;
fake.callsThrough = false;
},

resolvesArg: function resolvesArg(fake, index) {
if (typeof index !== "number") {
throw new TypeError("argument index is not number");
}
clear(fake);

fake.resolveArgAt = index;
fake.returnValue = undefined;
fake.resolve = true;
fake.resolveThis = false;
fake.reject = false;
fake.returnValueDefined = false;
fake.exception = undefined;
fake.exceptionCreator = undefined;
fake.fakeFn = undefined;
fake.callsThrough = false;
},

rejects: function rejects(fake, error, message) {
Expand All @@ -221,30 +218,36 @@ const defaultBehaviors = {
} else {
reason = error;
}
clear(fake);

fake.returnValue = reason;
fake.resolve = false;
fake.resolveThis = false;
fake.reject = true;
fake.returnValueDefined = true;
fake.exception = undefined;
fake.exceptionCreator = undefined;
fake.fakeFn = undefined;
fake.callsThrough = false;

return fake;
},

resolvesThis: function resolvesThis(fake) {
clear(fake);

fake.returnValue = undefined;
fake.resolve = false;
fake.resolveThis = true;
fake.reject = false;
fake.returnValueDefined = false;
fake.exception = undefined;
fake.exceptionCreator = undefined;
fake.fakeFn = undefined;
fake.callsThrough = false;
},

callThrough: function callThrough(fake) {
clear(fake);

fake.callsThrough = true;
},

callThroughWithNew: function callThroughWithNew(fake) {
clear(fake);

fake.callsThroughWithNew = true;
},

Expand Down
24 changes: 24 additions & 0 deletions test/issues/issues-test.js
Expand Up @@ -914,4 +914,28 @@ describe("issues", function () {
object.prop;
assert.equals(spy.get.callCount, 1); // should remain unchanged
});

it("#2572 - returns should not clear callsArgWith", function () {
const stub = sinon.stub();
stub.callsArgWith(0, "Hello").returns("World");

const cb = sinon.stub();
const ret = stub(cb);

assert.equals(ret, "World");
assert(cb.calledOnce);
assert.equals(cb.firstCall.args, ["Hello"]);
});

it("#2572 - callsArgWith should not clear returns", function () {
const stub = sinon.stub();
stub.returns("World").callsArgWith(0, "Hello");

const cb = sinon.stub();
const ret = stub(cb);

assert.equals(ret, "World");
assert(cb.calledOnce);
assert.equals(cb.firstCall.args, ["Hello"]);
});
});

0 comments on commit 5025d00

Please sign in to comment.