From 09b4da57f3f685fa41fc8681b71d90668ab41250 Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Thu, 25 Apr 2024 15:18:27 +0200 Subject: [PATCH 1/3] Add regression test for #2572 --- test/issues/issues-test.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/issues/issues-test.js b/test/issues/issues-test.js index 64eed2c53..1e8dd7101 100644 --- a/test/issues/issues-test.js +++ b/test/issues/issues-test.js @@ -914,4 +914,16 @@ describe("issues", function () { object.prop; assert.equals(spy.get.callCount, 1); // should remain unchanged }); + + it("#2572 - returns clear callArgAt", 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"]); + }); }); From f4fb595b32f8cdbc9bc97857dc94a10281cbd223 Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Thu, 25 Apr 2024 15:12:06 +0200 Subject: [PATCH 2/3] Partially revert "fix returns does not override call through (#2567)" This partially reverts commit 5fde5aebc74dec12bacd84d00a2f22906a7ebcc0: - 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. --- lib/sinon/default-behaviors.js | 145 +++++++++++++++++---------------- 1 file changed, 74 insertions(+), 71 deletions(-) diff --git a/lib/sinon/default-behaviors.js b/lib/sinon/default-behaviors.js index dff8c9d81..b4530b67b 100644 --- a/lib/sinon/default-behaviors.js +++ b/lib/sinon/default-behaviors.js @@ -30,90 +30,64 @@ 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) { @@ -121,59 +95,73 @@ const defaultBehaviors = { }, 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; }, @@ -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) { @@ -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; }, From 90f80a5c71261cd2719cec327333d571d83092bc Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Thu, 25 Apr 2024 15:26:06 +0200 Subject: [PATCH 3/3] Add reflected test and fixup test titles --- test/issues/issues-test.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/test/issues/issues-test.js b/test/issues/issues-test.js index 1e8dd7101..bda2d3a6a 100644 --- a/test/issues/issues-test.js +++ b/test/issues/issues-test.js @@ -915,7 +915,7 @@ describe("issues", function () { assert.equals(spy.get.callCount, 1); // should remain unchanged }); - it("#2572 - returns clear callArgAt", function () { + it("#2572 - returns should not clear callsArgWith", function () { const stub = sinon.stub(); stub.callsArgWith(0, "Hello").returns("World"); @@ -926,4 +926,16 @@ describe("issues", function () { 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"]); + }); });